kthread: Mark timer used by delayed kthread works as IRQ safe

Commit Message

Petr Mladek Feb. 17, 2020, 12:07 p.m. UTC

The timer used by delayed kthread works are IRQ safe because the used
kthread_delayed_work_timer_fn() is IRQ safe.

It is properly marked when initialized by KTHREAD_DELAYED_WORK_INIT().
But TIMER_IRQSAFE flag is missing when initialized by
kthread_init_delayed_work().

The missing flag might trigger invalid warning from del_timer_sync()
when kthread_mod_delayed_work() is called with interrupts disabled.

Reported-by: Grygorii Strashko <[e-mail]>
Signed-off-by: Petr Mladek <[e-mail]>
Tested-by: Grygorii Strashko <[e-mail]>
---
This patch is result of a discussion about using the API, see
https://lkml.kernel.org/r/[e-mail]

 include/linux/kthread.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Tejun Heo Feb. 19, 2020, 3:22 p.m. UTC | #1

On Mon, Feb 17, 2020 at 01:07:09PM +0100, Petr Mladek wrote:
> The timer used by delayed kthread works are IRQ safe because the used
> kthread_delayed_work_timer_fn() is IRQ safe.
> 
> It is properly marked when initialized by KTHREAD_DELAYED_WORK_INIT().
> But TIMER_IRQSAFE flag is missing when initialized by
> kthread_init_delayed_work().
> 
> The missing flag might trigger invalid warning from del_timer_sync()
> when kthread_mod_delayed_work() is called with interrupts disabled.
> 
> Reported-by: Grygorii Strashko <[e-mail]>
> Signed-off-by: Petr Mladek <[e-mail]>
> Tested-by: Grygorii Strashko <[e-mail]>

Acked-by: Tejun Heo <[e-mail]>

Thanks.

Grygorii Strashko March 16, 2020, 12:23 p.m. UTC | #2

Hi Petr,

On 19/02/2020 17:22, Tejun Heo wrote:
> On Mon, Feb 17, 2020 at 01:07:09PM +0100, Petr Mladek wrote:
>> The timer used by delayed kthread works are IRQ safe because the used
>> kthread_delayed_work_timer_fn() is IRQ safe.
>>
>> It is properly marked when initialized by KTHREAD_DELAYED_WORK_INIT().
>> But TIMER_IRQSAFE flag is missing when initialized by
>> kthread_init_delayed_work().
>>
>> The missing flag might trigger invalid warning from del_timer_sync()
>> when kthread_mod_delayed_work() is called with interrupts disabled.
>>
>> Reported-by: Grygorii Strashko <[e-mail]>
>> Signed-off-by: Petr Mladek <[e-mail]>
>> Tested-by: Grygorii Strashko <[e-mail]>
> 
> Acked-by: Tejun Heo <[e-mail]>

I'm worry shouldn't this patch have "fixes" tag?

Grygorii Strashko March 24, 2020, 10:46 a.m. UTC | #3

Hi All,

On 16/03/2020 14:23, Grygorii Strashko wrote:
> Hi Petr,
> 
> On 19/02/2020 17:22, Tejun Heo wrote:
>> On Mon, Feb 17, 2020 at 01:07:09PM +0100, Petr Mladek wrote:
>>> The timer used by delayed kthread works are IRQ safe because the used
>>> kthread_delayed_work_timer_fn() is IRQ safe.
>>>
>>> It is properly marked when initialized by KTHREAD_DELAYED_WORK_INIT().
>>> But TIMER_IRQSAFE flag is missing when initialized by
>>> kthread_init_delayed_work().
>>>
>>> The missing flag might trigger invalid warning from del_timer_sync()
>>> when kthread_mod_delayed_work() is called with interrupts disabled.
>>>
>>> Reported-by: Grygorii Strashko <[e-mail]>
>>> Signed-off-by: Petr Mladek <[e-mail]>
>>> Tested-by: Grygorii Strashko <[e-mail]>
>>
>> Acked-by: Tejun Heo <[e-mail]>
> 
> I'm worry shouldn't this patch have "fixes" tag?
> 

Sorry, the I'm disturbing you, but I have dependency from this path [1].
I can see it in -next: commit d7c8c7de96de ("kthread: mark timer used by
delayed kthread works as IRQ safe"), but it does not present in net-next.

Any way this can be resolved?

[1] https://patchwork.ozlabs.org/cover/1259207/

Patch

diff --git a/include/linux/kthread.h b/include/linux/kthread.h
index 0f9da966934e..8bbcaad7ef0f 100644
--- a/include/linux/kthread.h
+++ b/include/linux/kthread.h
@@ -165,7 +165,8 @@  extern void __kthread_init_worker(struct kthread_worker *worker,
 	do {								\
 		kthread_init_work(&(dwork)->work, (fn));		\
 		timer_setup(&(dwork)->timer,				\
-			     kthread_delayed_work_timer_fn, 0);		\
+			     kthread_delayed_work_timer_fn,		\
+			     TIMER_IRQSAFE);				\
 	} while (0)
 
 int kthread_worker_fn(void *worker_ptr);