* Mathieu Desnoyers <mathieu.desnoy...@efficios.com> wrote:
> ----- Original Message -----
> > From: "Ingo Molnar" <mi...@kernel.org>
> > To: "Paul E. McKenney" <paul...@linux.vnet.ibm.com>
> > Cc: linux-kernel@vger.kernel.org, la...@cn.fujitsu.com,
> > dipan...@in.ibm.com, a...@linux-foundation.org, "mathieu
> > desnoyers" <mathieu.desnoy...@efficios.com>, j...@joshtriplett.org,
> > n...@us.ibm.com, t...@linutronix.de,
> > pet...@infradead.org, rost...@goodmis.org, dhowe...@redhat.com,
> > eduma...@google.com, dar...@dvhart.com,
> > fweis...@gmail.com, s...@mit.edu, "Oleg Nesterov" <o...@redhat.com>,
> > "Jonathan Corbet" <cor...@lwn.net>, "Rusty
> > Russell" <ru...@rustcorp.com.au>
> > Sent: Thursday, December 5, 2013 10:33:34 AM
> > Subject: Re: [PATCH tip/core/locking 4/4]
> > Documentation/memory-barriers.txt: Document ACCESS_ONCE()
> >
> >
> > * Paul E. McKenney <paul...@linux.vnet.ibm.com> wrote:
> >
> > > + (*) The compiler is within its rights to reorder memory accesses unless
> > > + you tell it not to. For example, consider the following interaction
> > > + between process-level code and an interrupt handler:
> > > +
> > > + void process_level(void)
> > > + {
> > > + msg = get_message();
> > > + flag = true;
> > > + }
> > > +
> > > + void interrupt_handler(void)
> > > + {
> > > + if (flag)
> > > + process_message(msg);
> > > + }
> > > +
> > > + There is nothing to prevent the the compiler from transforming
> > > + process_level() to the following, in fact, this might well be a
> > > + win for single-threaded code:
> > > +
> > > + void process_level(void)
> > > + {
> > > + flag = true;
> > > + msg = get_message();
> > > + }
> > > +
> > > + If the interrupt occurs between these two statement, then
> > > + interrupt_handler() might be passed a garbled msg. Use
> > > ACCESS_ONCE()
> > > + to prevent this as follows:
> > > +
> > > + void process_level(void)
> > > + {
> > > + ACCESS_ONCE(msg) = get_message();
> > > + ACCESS_ONCE(flag) = true;
> > > + }
> > > +
> > > + void interrupt_handler(void)
> > > + {
> > > + if (ACCESS_ONCE(flag))
> > > + process_message(ACCESS_ONCE(msg));
> > > + }
> >
> > Technically, if the interrupt handler is the innermost context, the
> > ACCESS_ONCE() is not needed in the interrupt_handler() code.
> >
> > Since for the vast majority of Linux code IRQ handlers are the most
> > atomic contexts (very few drivers deal with NMIs) I suspect we should
> > either remove that ACCESS_ONCE() from the example or add a comment
> > explaining that in many cases those are superfluous?
>
> It might be worthwhile to state clearly those two different use-cases:
>
> * no nesting (e.g. process vs process), where both sides of the access
> need ACCESS_ONCE().
>
> * nested access: the outer context needs ACCESS_ONCE(), but not the
> inner context.
>
> We don't actually care about IRQs being the "most atomic" context,
> and we should not care about NMIs in this example. Only the relative
> nesting of the execution contexts accessing the data matter.
I meant 'most atomic' in the sense of no NMI context having access to
those same shared variables, obviously.
We do actually have 'triple' nesting in Linux, NMI context code with
non-trivial complexity that accesses variables in different code paths
from IRQ codepaths and from process level code paths, so technically
the original example is valid - I just think misleading to most
readers.
Thanks,
Ingo
--
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/
