All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: Trond Myklebust <Trond.Myklebust@netapp.com>
Cc: David Howells <dhowells@redhat.com>,
	linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] NFS: Fix RCU warnings in nfs_inode_return_delegation_noreclaim()
Date: Tue, 16 Mar 2010 12:15:29 -0700	[thread overview]
Message-ID: <20100316191529.GQ6709@linux.vnet.ibm.com> (raw)
In-Reply-To: <1268764995.3098.23.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>

On Tue, Mar 16, 2010 at 02:43:15PM -0400, Trond Myklebust wrote:
> On Tue, 2010-03-16 at 11:10 -0700, Paul E. McKenney wrote: 
> > On Tue, Mar 16, 2010 at 01:35:54PM -0400, Trond Myklebust wrote:
> > > On Tue, 2010-03-16 at 10:17 -0700, Paul E. McKenney wrote: 
> > > > On Tue, Mar 16, 2010 at 11:51:30AM +0000, David Howells wrote:
> > > > > diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
> > > > > index 2563beb..a77c735 100644
> > > > > --- a/fs/nfs/delegation.c
> > > > > +++ b/fs/nfs/delegation.c
> > > > > @@ -37,8 +37,10 @@ static void nfs_free_delegation(struct nfs_delegation *delegation)
> > > > >  {
> > > > >  	struct rpc_cred *cred;
> > > > > 
> > > > > +	rcu_read_lock();
> > > > >  	cred = rcu_dereference(delegation->cred);
> > > > >  	rcu_assign_pointer(delegation->cred, NULL);
> > > > 
> > > > The lock is probably held here, in which case something like the
> > > > following would work well without needing the artificial rcu_read_lock()
> > > > and rcu_read_unlock():
> > > 
> > > No. The lock is not held here. At this point, the delegation has been
> > > detached from the inode that pointed to it, and so we can free up its
> > > contents.
> > 
> > OK.  Is there some reference counter or pointer that can be checked to
> > verify that this data structure really is in a state that prevents
> > RCU readers from finding it?
> 
> Yes. The RCU readers are supposed to grab the delegation->lock and then
> check the contents of the delegation->inode.

So would something like the following work, then?

	cred = rcu_dereference_check(delegation->cred,
				     delegation->inode == NULL);

Or would some other check condition be more appropriate?

							Thanx, Paul

WARNING: multiple messages have this Message-ID (diff)
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: Trond Myklebust <Trond.Myklebust@netapp.com>
Cc: David Howells <dhowells@redhat.com>,
	linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] NFS: Fix RCU warnings in nfs_inode_return_delegation_noreclaim()
Date: Tue, 16 Mar 2010 12:15:29 -0700	[thread overview]
Message-ID: <20100316191529.GQ6709@linux.vnet.ibm.com> (raw)
In-Reply-To: <1268764995.3098.23.camel@localhost.localdomain>

On Tue, Mar 16, 2010 at 02:43:15PM -0400, Trond Myklebust wrote:
> On Tue, 2010-03-16 at 11:10 -0700, Paul E. McKenney wrote: 
> > On Tue, Mar 16, 2010 at 01:35:54PM -0400, Trond Myklebust wrote:
> > > On Tue, 2010-03-16 at 10:17 -0700, Paul E. McKenney wrote: 
> > > > On Tue, Mar 16, 2010 at 11:51:30AM +0000, David Howells wrote:
> > > > > diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
> > > > > index 2563beb..a77c735 100644
> > > > > --- a/fs/nfs/delegation.c
> > > > > +++ b/fs/nfs/delegation.c
> > > > > @@ -37,8 +37,10 @@ static void nfs_free_delegation(struct nfs_delegation *delegation)
> > > > >  {
> > > > >  	struct rpc_cred *cred;
> > > > > 
> > > > > +	rcu_read_lock();
> > > > >  	cred = rcu_dereference(delegation->cred);
> > > > >  	rcu_assign_pointer(delegation->cred, NULL);
> > > > 
> > > > The lock is probably held here, in which case something like the
> > > > following would work well without needing the artificial rcu_read_lock()
> > > > and rcu_read_unlock():
> > > 
> > > No. The lock is not held here. At this point, the delegation has been
> > > detached from the inode that pointed to it, and so we can free up its
> > > contents.
> > 
> > OK.  Is there some reference counter or pointer that can be checked to
> > verify that this data structure really is in a state that prevents
> > RCU readers from finding it?
> 
> Yes. The RCU readers are supposed to grab the delegation->lock and then
> check the contents of the delegation->inode.

So would something like the following work, then?

	cred = rcu_dereference_check(delegation->cred,
				     delegation->inode == NULL);

Or would some other check condition be more appropriate?

							Thanx, Paul

  parent reply	other threads:[~2010-03-16 19:15 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-03-16 11:51 [PATCH] NFS: Fix RCU warnings in nfs_inode_return_delegation_noreclaim() David Howells
     [not found] ` <20100316115130.25642.17789.stgit-S6HVgzuS8uM4Awkfq6JHfwNdhmdF6hFW@public.gmane.org>
2010-03-16 13:07   ` Trond Myklebust
2010-03-16 13:07     ` Trond Myklebust
2010-03-16 17:17   ` Paul E. McKenney
2010-03-16 17:17     ` Paul E. McKenney
2010-03-16 17:35     ` Trond Myklebust
     [not found]     ` <1268760954.3098.17.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2010-03-16 17:40       ` David Howells
2010-03-16 17:40         ` David Howells
2010-03-16 18:41         ` Trond Myklebust
2010-03-16 18:10       ` Paul E. McKenney
2010-03-16 18:10         ` Paul E. McKenney
2010-03-16 18:43         ` Trond Myklebust
     [not found]           ` <1268764995.3098.23.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2010-03-16 19:15             ` Paul E. McKenney [this message]
2010-03-16 19:15               ` Paul E. McKenney
     [not found] ` <1268744823.3155.15.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2010-03-16 16:37   ` David Howells
2010-03-16 16:37     ` David Howells

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20100316191529.GQ6709@linux.vnet.ibm.com \
    --to=paulmck@linux.vnet.ibm.com \
    --cc=Trond.Myklebust@netapp.com \
    --cc=dhowells@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.