Messages in this thread | | | Date | Tue, 30 Oct 2012 17:25:41 +0100 | Subject | Re: [PATCH 0/2] irq_work: A couple fixes | From | Frederic Weisbecker <> |
| |
2012/10/30 Steven Rostedt <rostedt@goodmis.org>: > On Tue, 2012-10-30 at 16:34 +0100, Frederic Weisbecker wrote: >> Hi, > >> And I still wonder if cpu_relax() is enough to prevent the compiler >> from correctly reloading work->flags in irq_work_sync() loop. >> Do we need ACCESS_ONCE()? > > You mean this loop: > > flags = work->flags & ~IRQ_WORK_PENDING; > for (;;) { > nflags = flags | IRQ_WORK_FLAGS; > oflags = cmpxchg(&work->flags, flags, nflags); > if (oflags == flags) > break; > if (oflags & IRQ_WORK_PENDING) > return false; > flags = oflags; > cpu_relax(); > } > > After the first loading of work->flags, you are worried about the > &work->flags in the cmpxchg()? The cmpxchg() will handle that itself. I > don't see any place that a ACCESS_ONCE() is required here. The cmpxchg() > acts on the address of work->flags, the compiler isn't involved with the > value at that address.
No I was worried about the cpu_relax() in irq_work_sync()
| |