Re: [PATCH tip/core/rcu 09/12] fs: Substitute rcu_access_pointer() for rcu_dereference_raw()

["Paul E. McKenney" <paulmck@linux.vnet.ibm.com>]

On Mon, Feb 17, 2014 at 02:00:15PM -0800, Josh Triplett wrote:
> On Mon, Feb 17, 2014 at 01:35:56PM -0800, Paul E. McKenney wrote:
> > From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> > 
> > (Trivial patch.)
> > 
> > If the code is looking at the RCU-protected pointer itself, but not
> > dereferencing it, the rcu_dereference() functions can be downgraded to
> > rcu_access_pointer().  This commit makes this downgrade in __alloc_fd(),
> > which simply compares the RCU-protected pointer against NULL with no
> > dereferencing.
> > 
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > Cc: Alexander Viro <viro@zeniv.linux.org.uk>
> > Cc: linux-fsdevel@vger.kernel.org
> 
> I'm beginning to wonder if this common pattern ought to have an
> rcu_pointer_is_null(), which would not return the pointer, only the
> boolean.

Or perhaps an rcu_compare_pointer() to also handle the various cases like:

	if (rcu_dereference_raw(foop) == barp) ...

I added the problem to the RCU cleanup list on the OPW site, and
your solution or my elaboration of it might be the right thing to do.
(Inspected all 1300 uses of members of the rcu_dereference() family of
functions last week, and was feeling a bit buggy-eyed at the end...)

							Thanx, Paul

> Regardless, for this patch:
> Reviewed-by: Josh Triplett <josh@joshtriplett.org>
> 
> >  fs/file.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/fs/file.c b/fs/file.c
> > index db25c2bdfe46..18f7d27855c4 100644
> > --- a/fs/file.c
> > +++ b/fs/file.c
> > @@ -497,7 +497,7 @@ repeat:
> >  	error = fd;
> >  #if 1
> >  	/* Sanity check */
> > -	if (rcu_dereference_raw(fdt->fd[fd]) != NULL) {
> > +	if (rcu_access_pointer(fdt->fd[fd]) != NULL) {
> >  		printk(KERN_WARNING "alloc_fd: slot %d not NULL!\n", fd);
> >  		rcu_assign_pointer(fdt->fd[fd], NULL);
> >  	}
> > -- 
> > 1.8.1.5
> > 
>