Re: [Lse-tech] Subject: [RFC][PATCH] Fix for unsafe notifier chain mechanism

[Alan Stern <stern@rowland.harvard.edu>]

On Sat, 12 Nov 2005, Paul E. McKenney wrote:

> Any advice on how to change the documentation so as to make the intent
> more clear would of course be most welcome!

Well, I thought the intent _was_ clear -- but it turned out not to be what 
you really meant!

Actually in this particular case I don't think it matters either way.  The 
difference amounts to an unnecessary memory barrier on an 
infrequently-used code path.  If you want to change the comment, you 
could say that using rcu_assign_pointer helps document which pointers 
might be rcu-dereferenced by readers at the time the assignment is being 
made.  Right now it just says "will be dereferenced", which could refer to 
any time in the future.


There is one other aspect where more documentation would be welcome: the 
description of rcu_dereference in Documentation/RCU/checklist.txt and why 
it is necessary on the Alpha.  (On the whole, I think the kernel's 
documentation of read_barrier_depends could be improved.)  It would be 
good to point out that when evaluating "*p", an Alpha can speculatively 
access memory before it knows the value of p!  If p's value then turns out 
not to match the address that was read, the processor does another fetch.  

Intuitively we don't normally think of the need to evaluate p before
fetching *p, because it's hard to imagine doing them in the wrong order.
Furthermore the C language isn't conducive to putting a read-barrier
between the evaluation of p and the evaluation of *p, but on the Alpha
such a barrier is important when pointers are updated without locking (as
in RCU).  Hence the need for rcu_dereference, which essentially does
nothing more than add that barrier.

I don't know, this may be more detail than you want to add at that spot.  
Maybe a different location in the documentation would be better; I haven't 
read all of it.

Alan Stern

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