lkml.org 
[lkml]   [2018]   [Jun]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 1/2] genirq: Update code comments wrt recycled thread_mask
Lukas,

On Sun, 24 Jun 2018, Lukas Wunner wrote:

> Previously a race existed between __free_irq() and __setup_irq() wherein
> the thread_mask of a just removed action could be handed out to a newly
> added action and the freed irq thread would then tread on the oneshot
> mask bit of the newly added irq thread in irq_finalize_oneshot():
>
> time
> | __free_irq()
> | raw_spin_lock_irqsave(&desc->lock, flags);
> | <remove action from linked list>
> | raw_spin_unlock_irqrestore(&desc->lock, flags);
> |
> | __setup_irq()
> | raw_spin_lock_irqsave(&desc->lock, flags);
> | <traverse linked list to determine oneshot mask bit>
> | raw_spin_unlock_irqrestore(&desc->lock, flags);
> |
> | irq_thread() of freed irq (__free_irq() waits in synchronize_irq())
> | irq_thread_fn()
> | irq_finalize_oneshot()
> | raw_spin_lock_irq(&desc->lock);
> | desc->threads_oneshot &= ~action->thread_mask;
> | raw_spin_unlock_irq(&desc->lock);
> v
>
> The race was known at least since 2012 when it was documented in a code
> comment by commit e04268b0effc ("genirq: Remove paranoid warnons and
> bogus fixups").

The race was known, but it was also harmless as nothing would touch stuff
after synchronize_irq().

> But it wasn't until 2017 that it was fixed by commit 9114014cf4e6
> ("genirq: Add mutex to irq desc to serialize request/free_irq()"),
> apparently inadvertantly so because the race is neither mentioned in the
> commit message nor was the code comment updated. Make up for that.

Thanks for following up. This update is very well done.

tglx

\
 
 \ /
  Last update: 2018-06-24 11:48    [W:0.559 / U:0.028 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site