Re: [PATCH 2/2] irq_work: Fix racy IRQ_WORK_BUSY flag setting

2012-10-31 Thread Steven Rostedt
On Wed, 2012-10-31 at 20:04 +0900, anish kumar wrote: > > This now does: > > > > CPU 1 CPU 2 > > - - > > (flags = 0) > > cmpxchg(flags, 0, IRQ_WORK_FLAGS) > > (flags = 3) > > [...] We can still ad

Re: [PATCH 2/2] irq_work: Fix racy IRQ_WORK_BUSY flag setting

2012-10-31 Thread Frederic Weisbecker
2012/10/31 Steven Rostedt : > On Wed, 2012-10-31 at 20:04 +0900, anish kumar wrote: >> nflags = 1 | 3 >> nflags = 2 | 3 >> In both cases the result would be same.If I am right then wouldn't this >> operation be redundant? > > Right. Actually we could change the new loop to: > > for (;;) { >

Re: [PATCH 2/2] irq_work: Fix racy IRQ_WORK_BUSY flag setting

2012-10-31 Thread Steven Rostedt
On Wed, 2012-10-31 at 20:04 +0900, anish kumar wrote: > > /* > > * Claim the entry so that no one else will poke at it. > > */ > > static bool irq_work_claim(struct irq_work *work) > > { > > unsigned long flags, nflags; > > > > for (;;) { > > flags = work->flags; > >

Re: [PATCH 2/2] irq_work: Fix racy IRQ_WORK_BUSY flag setting

2012-10-31 Thread anish kumar
On Tue, 2012-10-30 at 14:45 -0400, Steven Rostedt wrote: > On Wed, 2012-10-31 at 03:33 +0900, anish kumar wrote: > > > > CPU 0 CPU 1 > > > > > > data = something flags = IRQ_WORK_BUSY > > > smp_mb() (implicit with cmpxchg smp_mb() > > >

Re: [PATCH 2/2] irq_work: Fix racy IRQ_WORK_BUSY flag setting

2012-10-31 Thread anish kumar
On Tue, 2012-10-30 at 22:23 -0400, Steven Rostedt wrote: > On Wed, 2012-10-31 at 01:36 +0100, Frederic Weisbecker wrote: > > 2012/10/30 anish kumar : > > > As I understand without the memory barrier proposed by you the situation > > > would be as below: > > > CPU 0 C

Re: [PATCH 2/2] irq_work: Fix racy IRQ_WORK_BUSY flag setting

2012-10-30 Thread Frederic Weisbecker
2012/10/31 Steven Rostedt : > More confidence over what? The xchg()? They are equivalent (wrt memory > barriers). > > Here's the issue that currently exists. Let's look at the code: > > > /* > * Claim the entry so that no one else will poke at it. > */ > static bool irq_work_claim(struct irq_work

Re: [PATCH 2/2] irq_work: Fix racy IRQ_WORK_BUSY flag setting

2012-10-30 Thread Steven Rostedt
On Wed, 2012-10-31 at 01:36 +0100, Frederic Weisbecker wrote: > 2012/10/30 anish kumar : > > As I understand without the memory barrier proposed by you the situation > > would be as below: > > CPU 0 CPU 1 > > > > data = something flags = IRQ_WORK_

Re: [PATCH 2/2] irq_work: Fix racy IRQ_WORK_BUSY flag setting

2012-10-30 Thread Frederic Weisbecker
2012/10/30 anish kumar : > As I understand without the memory barrier proposed by you the situation > would be as below: > CPU 0 CPU 1 > > data = something flags = IRQ_WORK_BUSY > smp_mb() (implicit with cmpxchg execute_work (sees data from C

Re: [PATCH 2/2] irq_work: Fix racy IRQ_WORK_BUSY flag setting

2012-10-30 Thread Steven Rostedt
On Wed, 2012-10-31 at 03:33 +0900, anish kumar wrote: > > 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 > >

Re: [PATCH 2/2] irq_work: Fix racy IRQ_WORK_BUSY flag setting

2012-10-30 Thread anish kumar
> 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

Re: [PATCH 2/2] irq_work: Fix racy IRQ_WORK_BUSY flag setting

2012-10-30 Thread Steven Rostedt
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 e

[PATCH 2/2] irq_work: Fix racy IRQ_WORK_BUSY flag setting

2012-10-30 Thread Frederic Weisbecker
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 th