Messages in this thread | | | Date | Wed, 11 Oct 2017 09:02:52 -0700 | From | "Paul E. McKenney" <> | 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@linux.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
| |