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: <20171011160252.GF3521@linux.vnet.ibm.com>
Date:   Wed, 11 Oct 2017 09:02:52 -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 04:28:24PM +0100, David Howells wrote:
> Paul E. McKenney <paulmck@...ux.vnet.ibm.com> wrote:
> 
> > 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.
> 
> The problem with your 'address dep' comment is that it's not particularly
> useful.
> 
> Either your comment needs to say "dep between X and Y", but if the following is
> always the dep:
> 
> 	Y = READ_ONCE(X)
> 	access(*Y)
> 
> then the comment is superfluous.

In assoc_array.c, the access is often quite some distance from the
corresponding READ_ONCE().

> If it's not always true then your comment needs to indicate what the dependency
> is.

Given that most READ_ONCE() calls aren't heading dependency chains,
a comment indicating that a particular READ_ONCE() does head a dependency
chain does provide at least some information.  But, as you say below...

> The other thing your comment could/should say is where the other barrier is -
> barriers always have to be paired as a general rule.  (I know I haven't put
> these comments in here - but I've been doing that recently).

I would welcome a patch that added the comments or help with what
the comments should say.

							Thanx, Paul