linux-kernel - Re: [PATCH RFC tip/core/rcu 12/15] lib/assoc_array: Remove smp_read_barrier

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: <20171011125857.GY3521@linux.vnet.ibm.com>
Date:   Wed, 11 Oct 2017 05:58:57 -0700
From:   "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
To:     David Howells <dhowells@...hat.com>
Cc:     linux-kernel@...r.kernel.org, mingo@...nel.org,
        torvalds@...ux-foundation.org, mark.rutland@....com,
        linux-arch@...r.kernel.org, peterz@...radead.org,
        will.deacon@....com, Jonathan Corbet <corbet@....net>,
        Alexander Kuleshov <kuleshovmail@...il.com>
Subject: Re: [PATCH RFC tip/core/rcu 12/15] lib/assoc_array: Remove
 smp_read_barrier_depends()

On Wed, Oct 11, 2017 at 01:19:59PM +0100, David Howells wrote:
> Paul E. McKenney <paulmck@...ux.vnet.ibm.com> wrote:
> 
> > -	node = result.terminal_node.node;
> > -	smp_read_barrier_depends();
> > +	node = READ_ONCE(result.terminal_node.node); /* Address dependency. */
> 
> The main problem I have with this method of annotation is that it's not
> obvious there's a barrier there or which side the barrier is.
> 
> I think one of the trickiest issues is that a barrier is typically between two
> things and we're not making it clear what those two things actually are.
> 
> Also, I would say that the most natural interpretation of READ_ONCE() is that
> the implicit barrier comes after the read, e.g.:
> 
> 	f = READ_ONCE(stuff->foo);
> 	/* Implied barrier */
> 	look_at(f->a);
> 	look_at(f->b);
> 
> I.e. READ_ONCE() prevents stuff->foo from being reread whilst you access f and
> orders LOAD(stuff->foo) before LOAD(f->a) and LOAD(f->b).

Placing the comment on the same line makes it less likely that some
later change will move the comment away from the load that it applies to.
Which appears to have happened on some of the other instances of
smp_read_barrier_depends() in other parts of the kernel.  It is not at
all clear what load they go with, or if that load is even still present
in the kernel.  :-/

							Thanx, Paul