linux-kernel - Re: [Letux-kernel] [PATCH 5/5] input: twl6040-vibra: remove mutex

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 20 Apr 2016 10:49:51 -0700
From:	Dmitry Torokhov <dmitry.torokhov@...il.com>
To:	"H. Nikolaus Schaller" <hns@...delico.com>
Cc:	Fabio Estevam <fabio.estevam@...escale.com>,
	linux-input@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>,
	Peter Ujfalusi <peter.ujfalusi@...com>,
	kernel@...a-handheld.com,
	Discussions about the Letux Kernel 
	<letux-kernel@...nphoenux.org>
Subject: Re: [Letux-kernel] [PATCH 5/5] input: twl6040-vibra: remove mutex

On Wed, Apr 20, 2016 at 11:22:53AM +0200, H. Nikolaus Schaller wrote:
> 
> > Am 19.04.2016 um 10:08 schrieb H. Nikolaus Schaller <hns@...delico.com>:
> > 
> > 
> >> Am 19.04.2016 um 10:01 schrieb Dmitry Torokhov <dmitry.torokhov@...il.com>:
> >> 
> >> On Tue, Apr 19, 2016 at 09:49:01AM +0200, H. Nikolaus Schaller wrote:
> >>> 
> >>>> Am 18.04.2016 um 23:20 schrieb Dmitry Torokhov <dmitry.torokhov@...il.com>:
> >>>> 
> >>>> On Mon, Apr 18, 2016 at 09:55:41PM +0200, H. Nikolaus Schaller wrote:
> >>>>> The mutex does not seem to be needed.
> >>>> 
> >>>> twl6040_vibra_suspend() and vibra_play_work() may run concurrently, no?
> >>> 
> >>> Hm. I don't know about the rule that would give an answer to this question...
> >> 
> >> Sorry, that was actually a statement, not really a question.
> > 
> > Indeed. In doubt about the answer we should take measures for the worst case.
> > 
> >> It is
> >> possible (although very unlikely) that userspace posts play request and
> >> workqueue will not run until after suspend callback.
> >> 
> >> Thinking about it some more I wonder if we better do what
> >> twl6040_vibra_close() does and cancel the work before shutting off the
> >> device, so that there is no chance of work executing after suspend
> >> callback and reenabling the device. This way we can indeed remove the
> >> mutex.
> > 
> > Ok, I am fine with this.
> > 
> > Will post an update ASAP.
> 
> While doing testing I did run again into another locking related issue which I
> had not yet tried to address with my patch set. It is a:
> 
> 	BUG: scheduling while atomic
> 
> report which sometimes ends in a kernel panic.
> 
> I have attached such a log. vibra.py is here:
> 
> 	http://git.goldelico.com/?p=gta04-kernel.git;a=blob;f=Letux/root/vibra.py;hb=refs/heads/letux-4.6-rc4
> 
> Basically it does an ioctl(EVIOCSFF) which triggers vibra_play.
> 
> Maybe, can you decipher from the log what the reason could be?
> 
> I only conjecture that it happens when vibra_play tries to read the regmap
> of the twl6040 to get twl6040_get_vibralr_status (which has no pendant
> in the twl4030 driver).
> 
> Do we have to configure the twl6040 regmap differently?

Right, vibra_play is called with interrupts disabled (that's why we have
workqueue to enable/disable regulators, etc, when we start or stop
vibration), so twl6040_get_vibralr_status() should not sleep, but
apparently it does. Maybe the check for audio configuration should be
moved into vibra_play_work().

Thanks.

-- 
Dmitry