linux-kernel - Re: [PATCH] Input: pwm-beeper - defer pwm config if pwm can sleep

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:	Tue, 23 Feb 2016 09:46:02 +0100
From:	Manfred Schlaegl <manfred.schlaegl@....at>
To:	Dmitry Torokhov <dmitry.torokhov@...il.com>
Cc:	Manfred Schlaegl <manfred.schlaegl@...zinger.com>,
	Luis de Bethencourt <luis@...ethencourt.com>,
	Olivier Sobrie <olivier@...rie.be>,
	linux-input@...r.kernel.org, linux-kernel@...r.kernel.org,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: [PATCH] Input: pwm-beeper - defer pwm config if pwm can sleep

On 2016-02-22 20:46, Dmitry Torokhov wrote:
> On Wed, Feb 17, 2016 at 02:19:26PM +0100, Manfred Schlaegl wrote:
>> If the pwm can sleep defer actions to it using a worker.
>> A similar approach was used in leds-pwm (c971ff185)
>>
>> Trigger:
>> On a Freescale i.MX53 based board we ran into "BUG: scheduling while
>> atomic" because input_inject_event locks interrupts, but
>> imx_pwm_config_v2 sleeps.
>>
>> Tested on Freescale i.MX53 SoC with 4.5-rc1 and 4.1.
>>
>> Unmodified applicable to
>>  * 4.5-rc4
>>  * 4.4.1 (stable)
>>  * 4.3.5 (stable)
>>  * 4.1.18 (longterm)
>>
>> Modified applicable to
>>  * 3.18.27 (longterm)
>>
>> Signed-off-by: Manfred Schlaegl <manfred.schlaegl@....at>
>> ---
>>  drivers/input/misc/pwm-beeper.c | 62 +++++++++++++++++++++++++++++------------
>>  1 file changed, 44 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c
>> index f2261ab..c160b5e 100644
>> --- a/drivers/input/misc/pwm-beeper.c
>> +++ b/drivers/input/misc/pwm-beeper.c
>> @@ -20,21 +20,42 @@
>>  #include <linux/platform_device.h>
>>  #include <linux/pwm.h>
>>  #include <linux/slab.h>
>> +#include <linux/workqueue.h>
>>  
>>  struct pwm_beeper {
>>  	struct input_dev *input;
>>  	struct pwm_device *pwm;
>> +	struct work_struct work;
>>  	unsigned long period;
>> +	bool can_sleep;
> 
> I wonder if it is not better to always schedule work, regardless of
> whether PWM may sleep or not.
> 
> Thanks.
> 

In my opinion there is no real strong argument to do it this or that way.
I decided to do it this way because of following weaker arguments:
 1. If pwm can not sleep the behavior stays exactly the same as before
 2. The introduced conditions do not really add much complexity to the code
 3. It was successfully done the same way in leds-pwm

Best regards,
Manfred