linux-kernel - Re: [PATCH tip/core/locking 4/4] Documentation/memory-barriers.txt: Document ACCESS

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20131205214406.GY15492@linux.vnet.ibm.com>
Date:	Thu, 5 Dec 2013 13:44:06 -0800
From:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
To:	Jonathan Corbet <corbet@....net>
Cc:	linux-kernel@...r.kernel.org, mingo@...nel.org,
	laijs@...fujitsu.com, dipankar@...ibm.com,
	akpm@...ux-foundation.org, mathieu.desnoyers@...icios.com,
	josh@...htriplett.org, niv@...ibm.com, tglx@...utronix.de,
	peterz@...radead.org, rostedt@...dmis.org, dhowells@...hat.com,
	edumazet@...gle.com, darren@...art.com, fweisbec@...il.com,
	sbw@....edu, Oleg Nesterov <oleg@...hat.com>,
	Rusty Russell <rusty@...tcorp.com.au>
Subject: Re: [PATCH tip/core/locking 4/4] Documentation/memory-barriers.txt:
 Document ACCESS_ONCE()

On Thu, Dec 05, 2013 at 01:21:01PM -0700, Jonathan Corbet wrote:
> On Wed,  4 Dec 2013 14:46:59 -0800
> "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com> wrote:
> 
> > From: "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
> > 
> > The situations in which ACCESS_ONCE() is required are not well documented,
> > so this commit adds some verbiage to memory-barriers.txt.
> 
> [...]
> 
> > +     But please note that the compiler is also closely watching what you
> > +     do with the value after the ACCESS_ONCE().  For example, suppose you
> > +     do the following and MAX is a preprocessor macro with the value 1:
> > +
> > +	for ((tmp = ACCESS_ONCE(a)) % MAX)
> > +		do_something_with(tmp);
> 
> That sure looks like it was meant to be "while" instead of "for"?

Good catch, fixed!

> [...]
> 
> > + (*) 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));
> > +	}
> 
> Looking at this, I find myself wondering why you couldn't just put a
> barrier() between the two statements in process_level()?  ACCESS_ONCE()
> seems like a heavy hammer to just avoid reordering of two assignments.
> What am I missing, and what could be added here to keep the other folks as
> dense as me from missing the same thing?

You could use barrier() from an ordering viewpoint.  However,
ACCESS_ONCE() is often lighter weight than barrier().  ACCESS_ONCE()
affects only that one access, while barrier() forces the compiler to
forget pretty much anything it previously gleaned from any region of
memory, including private locations that no one else touches.

I am adding a sentence saying that pure ordering can be provided
by barrier(), though often at higher cost.

							Thanx, Paul

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/