nfs: abort delegation in dying VE

Submitted by Stanislav Kinsburskiy on Nov. 15, 2017, 4:50 p.m.

Details

Message ID 20171115165054.29548.40238.stgit@localhost.localdomain
State New
Series "nfs: abort delegation in dying VE"
Headers show

Commit Message

Stanislav Kinsburskiy Nov. 15, 2017, 4:50 p.m.
Don't queue delegation request, if ve init is exiting.

https://jira.sw.ru/browse/PSBM-77061

Inspired-by: Kirill Tkhai <ktkhai@virtuozzo.com>
Signed-off-by: Stanislav Kinsburskiy <skinsbursky@virtuozzo.com>
---
 fs/nfs/delegation.c |   16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
index 66af497..2422754 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -189,15 +189,29 @@  void nfs_inode_reclaim_delegation(struct inode *inode, struct rpc_cred *cred,
 	nfs_inode_set_delegation(inode, cred, res);
 }
 
+static bool ve_abort_delegation(struct inode *inode)
+{
+	struct nfs_client *clp = NFS_SERVER(inode)->nfs_client;
+	struct rpc_xprt *xprt;
+
+	rcu_read_lock();
+	xprt = rcu_dereference(clp->cl_rpcclient->cl_xprt);
+	rcu_read_unlock();
+
+	return xprt->xprt_net->owner_ve->ve_netns == NULL;
+}
+
 static int nfs_do_return_delegation(struct inode *inode, struct nfs_delegation *delegation, int issync)
 {
 	int res = 0;
 
-	if (!test_bit(NFS_DELEGATION_REVOKED, &delegation->flags))
+	if (!test_bit(NFS_DELEGATION_REVOKED, &delegation->flags) &&
+	    !ve_abort_delegation(inode)) {
 		res = nfs4_proc_delegreturn(inode,
 				delegation->cred,
 				&delegation->stateid,
 				issync);
+	}
 	nfs_free_delegation(delegation);
 	return res;
 }

Comments

Kirill Tkhai Nov. 15, 2017, 4:55 p.m.
On 15.11.2017 19:50, Stanislav Kinsburskiy wrote:
> Don't queue delegation request, if ve init is exiting.
> 
> https://jira.sw.ru/browse/PSBM-77061
> 
> Inspired-by: Kirill Tkhai <ktkhai@virtuozzo.com>
> Signed-off-by: Stanislav Kinsburskiy <skinsbursky@virtuozzo.com>
> ---
>  fs/nfs/delegation.c |   16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
> index 66af497..2422754 100644
> --- a/fs/nfs/delegation.c
> +++ b/fs/nfs/delegation.c
> @@ -189,15 +189,29 @@ void nfs_inode_reclaim_delegation(struct inode *inode, struct rpc_cred *cred,
>  	nfs_inode_set_delegation(inode, cred, res);
>  }
>  
> +static bool ve_abort_delegation(struct inode *inode)
> +{
> +	struct nfs_client *clp = NFS_SERVER(inode)->nfs_client;
> +	struct rpc_xprt *xprt;
> +
> +	rcu_read_lock();
> +	xprt = rcu_dereference(clp->cl_rpcclient->cl_xprt);
> +	rcu_read_unlock();
> +
> +	return xprt->xprt_net->owner_ve->ve_netns == NULL;

Usually, memory pointed by a pointer, which was obtained via rcu, has to be used
in rcu_read_* brackets:

rcu_read_lock();
xprt = rcu_dereference(clp->cl_rpcclient->cl_xprt);
ret = (xprt->xprt_net->owner_ve->ve_netns == NULL);
rcu_read_unlock();

return ret;

If there is no an exception, we have to do something like above.

> +}
> +
>  static int nfs_do_return_delegation(struct inode *inode, struct nfs_delegation *delegation, int issync)
>  {
>  	int res = 0;
>  
> -	if (!test_bit(NFS_DELEGATION_REVOKED, &delegation->flags))
> +	if (!test_bit(NFS_DELEGATION_REVOKED, &delegation->flags) &&
> +	    !ve_abort_delegation(inode)) {
>  		res = nfs4_proc_delegreturn(inode,
>  				delegation->cred,
>  				&delegation->stateid,
>  				issync);
> +	}
>  	nfs_free_delegation(delegation);
>  	return res;
>  }
>
Andrey Vagin Nov. 15, 2017, 7:41 p.m.
On Wed, Nov 15, 2017 at 07:55:02PM +0300, Kirill Tkhai wrote:
> On 15.11.2017 19:50, Stanislav Kinsburskiy wrote:
> > Don't queue delegation request, if ve init is exiting.
> > 
> > https://jira.sw.ru/browse/PSBM-77061
> > 
> > Inspired-by: Kirill Tkhai <ktkhai@virtuozzo.com>
> > Signed-off-by: Stanislav Kinsburskiy <skinsbursky@virtuozzo.com>
> > ---
> >  fs/nfs/delegation.c |   16 +++++++++++++++-
> >  1 file changed, 15 insertions(+), 1 deletion(-)
> > 
> > diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
> > index 66af497..2422754 100644
> > --- a/fs/nfs/delegation.c
> > +++ b/fs/nfs/delegation.c
> > @@ -189,15 +189,29 @@ void nfs_inode_reclaim_delegation(struct inode *inode, struct rpc_cred *cred,
> >  	nfs_inode_set_delegation(inode, cred, res);
> >  }
> >  
> > +static bool ve_abort_delegation(struct inode *inode)
> > +{
> > +	struct nfs_client *clp = NFS_SERVER(inode)->nfs_client;
> > +	struct rpc_xprt *xprt;
> > +
> > +	rcu_read_lock();
> > +	xprt = rcu_dereference(clp->cl_rpcclient->cl_xprt);
> > +	rcu_read_unlock();
> > +
> > +	return xprt->xprt_net->owner_ve->ve_netns == NULL;
> 
> Usually, memory pointed by a pointer, which was obtained via rcu, has to be used
> in rcu_read_* brackets:
> 
> rcu_read_lock();
> xprt = rcu_dereference(clp->cl_rpcclient->cl_xprt);
> ret = (xprt->xprt_net->owner_ve->ve_netns == NULL);
> rcu_read_unlock();
> 
> return ret;
> 
> If there is no an exception, we have to do something like above.

It is true when you want to dereference this pointer, otherwise I don't
see any reason to take rcu_read_lock().

> 
> > +}
> > +
> >  static int nfs_do_return_delegation(struct inode *inode, struct nfs_delegation *delegation, int issync)
> >  {
> >  	int res = 0;
> >  
> > -	if (!test_bit(NFS_DELEGATION_REVOKED, &delegation->flags))
> > +	if (!test_bit(NFS_DELEGATION_REVOKED, &delegation->flags) &&
> > +	    !ve_abort_delegation(inode)) {
> >  		res = nfs4_proc_delegreturn(inode,
> >  				delegation->cred,
> >  				&delegation->stateid,
> >  				issync);
> > +	}
> >  	nfs_free_delegation(delegation);
> >  	return res;
> >  }
> > 
> _______________________________________________
> Devel mailing list
> Devel@openvz.org
> https://lists.openvz.org/mailman/listinfo/devel
Andrey Vagin Nov. 15, 2017, 7:47 p.m.
On Wed, Nov 15, 2017 at 11:41:00AM -0800, Andrei Vagin wrote:
> On Wed, Nov 15, 2017 at 07:55:02PM +0300, Kirill Tkhai wrote:
> > On 15.11.2017 19:50, Stanislav Kinsburskiy wrote:
> > > Don't queue delegation request, if ve init is exiting.
> > > 
> > > https://jira.sw.ru/browse/PSBM-77061
> > > 
> > > Inspired-by: Kirill Tkhai <ktkhai@virtuozzo.com>
> > > Signed-off-by: Stanislav Kinsburskiy <skinsbursky@virtuozzo.com>
> > > ---
> > >  fs/nfs/delegation.c |   16 +++++++++++++++-
> > >  1 file changed, 15 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
> > > index 66af497..2422754 100644
> > > --- a/fs/nfs/delegation.c
> > > +++ b/fs/nfs/delegation.c
> > > @@ -189,15 +189,29 @@ void nfs_inode_reclaim_delegation(struct inode *inode, struct rpc_cred *cred,
> > >  	nfs_inode_set_delegation(inode, cred, res);
> > >  }
> > >  
> > > +static bool ve_abort_delegation(struct inode *inode)
> > > +{
> > > +	struct nfs_client *clp = NFS_SERVER(inode)->nfs_client;
> > > +	struct rpc_xprt *xprt;
> > > +
> > > +	rcu_read_lock();
> > > +	xprt = rcu_dereference(clp->cl_rpcclient->cl_xprt);
> > > +	rcu_read_unlock();
> > > +
> > > +	return xprt->xprt_net->owner_ve->ve_netns == NULL;
> > 
> > Usually, memory pointed by a pointer, which was obtained via rcu, has to be used
> > in rcu_read_* brackets:
> > 
> > rcu_read_lock();
> > xprt = rcu_dereference(clp->cl_rpcclient->cl_xprt);
> > ret = (xprt->xprt_net->owner_ve->ve_netns == NULL);
> > rcu_read_unlock();
> > 
> > return ret;
> > 
> > If there is no an exception, we have to do something like above.
> 
> It is true when you want to dereference this pointer, otherwise I don't
> see any reason to take rcu_read_lock().

Oops, here is exectly this case. I didn't read the proposed code. Sorry.
> 
> > 
> > > +}
> > > +
> > >  static int nfs_do_return_delegation(struct inode *inode, struct nfs_delegation *delegation, int issync)
> > >  {
> > >  	int res = 0;
> > >  
> > > -	if (!test_bit(NFS_DELEGATION_REVOKED, &delegation->flags))
> > > +	if (!test_bit(NFS_DELEGATION_REVOKED, &delegation->flags) &&
> > > +	    !ve_abort_delegation(inode)) {
> > >  		res = nfs4_proc_delegreturn(inode,
> > >  				delegation->cred,
> > >  				&delegation->stateid,
> > >  				issync);
> > > +	}
> > >  	nfs_free_delegation(delegation);
> > >  	return res;
> > >  }
> > > 
> > _______________________________________________
> > Devel mailing list
> > Devel@openvz.org
> > https://lists.openvz.org/mailman/listinfo/devel
> _______________________________________________
> Devel mailing list
> Devel@openvz.org
> https://lists.openvz.org/mailman/listinfo/devel