On Tue, 2012-10-30 at 16:35 +0100, Frederic Weisbecker wrote: > The IRQ_WORK_BUSY flag is set right before we execute the > work. Once this flag value is set, the work enters a > claimable state again. > > This is necessary because if we want to enqueue a work but we > fail the claim, we want to ensure that the CPU where that work > is still pending will see and handle the data we expected the > work to compute. > > This might not work as expected though because IRQ_WORK_BUSY > isn't set atomically. By the time a CPU fails a work claim, > this work may well have been already executed by the CPU where > it was previously pending. > > Due to the lack of appropriate memory barrier, the IRQ_WORK_BUSY > flag value may not be visible by the CPU trying to claim while > the work is executing, and that until we clear the busy bit in > the work flags using cmpxchg() that implies the full barrier. > > One solution could involve a full barrier between setting > IRQ_WORK_BUSY flag and the work execution. This way we > ensure that the work execution site sees the expected data > and the claim site sees the IRQ_WORK_BUSY: > > CPU 0 CPU 1 > > data = something flags = IRQ_WORK_BUSY > smp_mb() (implicit with cmpxchg smp_mb() > on flags in claim) execute_work (sees data from CPU 0) > try to claim > > As a shortcut, let's just use xchg() that implies a full memory > barrier. > > Signed-off-by: Frederic Weisbecker <fweis...@gmail.com> > Cc: Peter Zijlstra <pet...@infradead.org> > Cc: Ingo Molnar <mi...@kernel.org> > Cc: Thomas Gleixner <t...@linutronix.de> > Cc: Andrew Morton <a...@linux-foundation.org> > Cc: Steven Rostedt <rost...@goodmis.org> > Cc: Paul Gortmaker <paul.gortma...@windriver.com>
Reviewed-by: Steven Rostedt <rost...@goodmis.org> -- Steve > --- > kernel/irq_work.c | 7 +++++-- > 1 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/kernel/irq_work.c b/kernel/irq_work.c > index 764240a..ea79365 100644 > --- a/kernel/irq_work.c > +++ b/kernel/irq_work.c > @@ -130,9 +130,12 @@ void irq_work_run(void) > > /* > * Clear the PENDING bit, after this point the @work > - * can be re-used. > + * can be re-used. Use xchg to force ordering against > + * data to process, such that if claiming fails on > + * another CPU, we see and handle the data it wants > + * us to process on the work. > */ > - work->flags = IRQ_WORK_BUSY; > + xchg(&work->flags, IRQ_WORK_BUSY); > work->func(work); > /* > * Clear the BUSY bit and return to the free state if -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
