* [PATCH 1/4] pNFS: Mark layout for return if return-on-close was not sent
@ 2021-01-05 15:26 trondmy
2021-01-05 15:26 ` [PATCH 2/4] pNFS: We want return-on-close to complete when evicting the inode trondmy
0 siblings, 1 reply; 5+ messages in thread
From: trondmy @ 2021-01-05 15:26 UTC (permalink / raw)
To: linux-nfs
From: Trond Myklebust <trond.myklebust@hammerspace.com>
If the layout return-on-close failed because the layoutreturn was never
sent, then we should mark the layout for return again.
Fixes: 9c47b18cf722 ("pNFS: Ensure we do clear the return-on-close layout stateid on fatal errors")
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
fs/nfs/pnfs.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index 07f59dc8cb2e..ccc89fab1802 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -1560,12 +1560,18 @@ void pnfs_roc_release(struct nfs4_layoutreturn_args *args,
int ret)
{
struct pnfs_layout_hdr *lo = args->layout;
+ struct inode *inode = args->inode;
const nfs4_stateid *arg_stateid = NULL;
const nfs4_stateid *res_stateid = NULL;
struct nfs4_xdr_opaque_data *ld_private = args->ld_private;
switch (ret) {
case -NFS4ERR_NOMATCHING_LAYOUT:
+ spin_lock(&inode->i_lock);
+ if (pnfs_layout_is_valid(lo) &&
+ nfs4_stateid_match_other(&args->stateid, &lo->plh_stateid))
+ pnfs_set_plh_return_info(lo, args->range.iomode, 0);
+ spin_unlock(&inode->i_lock);
break;
case 0:
if (res->lrs_present)
--
2.29.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/4] pNFS: We want return-on-close to complete when evicting the inode 2021-01-05 15:26 [PATCH 1/4] pNFS: Mark layout for return if return-on-close was not sent trondmy @ 2021-01-05 15:26 ` trondmy 2021-01-05 15:26 ` [PATCH 3/4] pNFS: Clean up pnfs_layoutreturn_free_lsegs() trondmy 2021-01-06 15:25 ` [PATCH 2/4] pNFS: We want return-on-close to complete when evicting the inode Anna Schumaker 0 siblings, 2 replies; 5+ messages in thread From: trondmy @ 2021-01-05 15:26 UTC (permalink / raw) To: linux-nfs From: Trond Myklebust <trond.myklebust@hammerspace.com> If the inode is being evicted, it should be safe to run return-on-close, so we should do it to ensure we don't inadvertently leak layout segments. Fixes: 1c5bd76d17cc ("pNFS: Enable layoutreturn operation for return-on-close") Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- fs/nfs/nfs4proc.c | 26 ++++++++++---------------- fs/nfs/pnfs.c | 8 +++----- fs/nfs/pnfs.h | 6 ++---- 3 files changed, 15 insertions(+), 25 deletions(-) diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 0ce04e0e5d82..bbca5c46e400 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -3536,10 +3536,8 @@ static void nfs4_close_done(struct rpc_task *task, void *data) trace_nfs4_close(state, &calldata->arg, &calldata->res, task->tk_status); /* Handle Layoutreturn errors */ - if (pnfs_roc_done(task, calldata->inode, - &calldata->arg.lr_args, - &calldata->res.lr_res, - &calldata->res.lr_ret) == -EAGAIN) + if (pnfs_roc_done(task, &calldata->arg.lr_args, &calldata->res.lr_res, + &calldata->res.lr_ret) == -EAGAIN) goto out_restart; /* hmm. we are done with the inode, and in the process of freeing @@ -6384,10 +6382,8 @@ static void nfs4_delegreturn_done(struct rpc_task *task, void *calldata) trace_nfs4_delegreturn_exit(&data->args, &data->res, task->tk_status); /* Handle Layoutreturn errors */ - if (pnfs_roc_done(task, data->inode, - &data->args.lr_args, - &data->res.lr_res, - &data->res.lr_ret) == -EAGAIN) + if (pnfs_roc_done(task, &data->args.lr_args, &data->res.lr_res, + &data->res.lr_ret) == -EAGAIN) goto out_restart; switch (task->tk_status) { @@ -6441,10 +6437,10 @@ static void nfs4_delegreturn_release(void *calldata) struct nfs4_delegreturndata *data = calldata; struct inode *inode = data->inode; + if (data->lr.roc) + pnfs_roc_release(&data->lr.arg, &data->lr.res, + data->res.lr_ret); if (inode) { - if (data->lr.roc) - pnfs_roc_release(&data->lr.arg, &data->lr.res, - data->res.lr_ret); nfs_post_op_update_inode_force_wcc(inode, &data->fattr); nfs_iput_and_deactive(inode); } @@ -6520,16 +6516,14 @@ static int _nfs4_proc_delegreturn(struct inode *inode, const struct cred *cred, nfs_fattr_init(data->res.fattr); data->timestamp = jiffies; data->rpc_status = 0; - data->lr.roc = pnfs_roc(inode, &data->lr.arg, &data->lr.res, cred); data->inode = nfs_igrab_and_active(inode); - if (data->inode) { + if (data->inode || issync) { + data->lr.roc = pnfs_roc(inode, &data->lr.arg, &data->lr.res, + cred); if (data->lr.roc) { data->args.lr_args = &data->lr.arg; data->res.lr_res = &data->lr.res; } - } else if (data->lr.roc) { - pnfs_roc_release(&data->lr.arg, &data->lr.res, 0); - data->lr.roc = false; } task_setup_data.callback_data = data; diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c index ccc89fab1802..a18b1992b2fb 100644 --- a/fs/nfs/pnfs.c +++ b/fs/nfs/pnfs.c @@ -1509,10 +1509,8 @@ bool pnfs_roc(struct inode *ino, return false; } -int pnfs_roc_done(struct rpc_task *task, struct inode *inode, - struct nfs4_layoutreturn_args **argpp, - struct nfs4_layoutreturn_res **respp, - int *ret) +int pnfs_roc_done(struct rpc_task *task, struct nfs4_layoutreturn_args **argpp, + struct nfs4_layoutreturn_res **respp, int *ret) { struct nfs4_layoutreturn_args *arg = *argpp; int retval = -EAGAIN; @@ -1545,7 +1543,7 @@ int pnfs_roc_done(struct rpc_task *task, struct inode *inode, return 0; case -NFS4ERR_OLD_STATEID: if (!nfs4_layout_refresh_old_stateid(&arg->stateid, - &arg->range, inode)) + &arg->range, arg->inode)) break; *ret = -NFS4ERR_NOMATCHING_LAYOUT; return -EAGAIN; diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h index bbd3de1025f2..4d5ee568411d 100644 --- a/fs/nfs/pnfs.h +++ b/fs/nfs/pnfs.h @@ -297,10 +297,8 @@ bool pnfs_roc(struct inode *ino, struct nfs4_layoutreturn_args *args, struct nfs4_layoutreturn_res *res, const struct cred *cred); -int pnfs_roc_done(struct rpc_task *task, struct inode *inode, - struct nfs4_layoutreturn_args **argpp, - struct nfs4_layoutreturn_res **respp, - int *ret); +int pnfs_roc_done(struct rpc_task *task, struct nfs4_layoutreturn_args **argpp, + struct nfs4_layoutreturn_res **respp, int *ret); void pnfs_roc_release(struct nfs4_layoutreturn_args *args, struct nfs4_layoutreturn_res *res, int ret); -- 2.29.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 3/4] pNFS: Clean up pnfs_layoutreturn_free_lsegs() 2021-01-05 15:26 ` [PATCH 2/4] pNFS: We want return-on-close to complete when evicting the inode trondmy @ 2021-01-05 15:26 ` trondmy 2021-01-05 15:26 ` [PATCH 4/4] pNFS: Stricter ordering of layoutget and layoutreturn trondmy 2021-01-06 15:25 ` [PATCH 2/4] pNFS: We want return-on-close to complete when evicting the inode Anna Schumaker 1 sibling, 1 reply; 5+ messages in thread From: trondmy @ 2021-01-05 15:26 UTC (permalink / raw) To: linux-nfs From: Trond Myklebust <trond.myklebust@hammerspace.com> Remove the check for whether or not the stateid is NULL, and fix up the callers. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- fs/nfs/pnfs.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c index a18b1992b2fb..a0d8a67a4dd9 100644 --- a/fs/nfs/pnfs.c +++ b/fs/nfs/pnfs.c @@ -1152,7 +1152,7 @@ void pnfs_layoutreturn_free_lsegs(struct pnfs_layout_hdr *lo, LIST_HEAD(freeme); spin_lock(&inode->i_lock); - if (!pnfs_layout_is_valid(lo) || !arg_stateid || + if (!pnfs_layout_is_valid(lo) || !nfs4_stateid_match_other(&lo->plh_stateid, arg_stateid)) goto out_unlock; if (stateid) { @@ -1559,7 +1559,6 @@ void pnfs_roc_release(struct nfs4_layoutreturn_args *args, { struct pnfs_layout_hdr *lo = args->layout; struct inode *inode = args->inode; - const nfs4_stateid *arg_stateid = NULL; const nfs4_stateid *res_stateid = NULL; struct nfs4_xdr_opaque_data *ld_private = args->ld_private; @@ -1576,11 +1575,10 @@ void pnfs_roc_release(struct nfs4_layoutreturn_args *args, res_stateid = &res->stateid; fallthrough; default: - arg_stateid = &args->stateid; + pnfs_layoutreturn_free_lsegs(lo, &args->stateid, &args->range, + res_stateid); } trace_nfs4_layoutreturn_on_close(args->inode, &args->stateid, ret); - pnfs_layoutreturn_free_lsegs(lo, arg_stateid, &args->range, - res_stateid); if (ld_private && ld_private->ops && ld_private->ops->free) ld_private->ops->free(ld_private); pnfs_put_layout_hdr(lo); -- 2.29.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 4/4] pNFS: Stricter ordering of layoutget and layoutreturn 2021-01-05 15:26 ` [PATCH 3/4] pNFS: Clean up pnfs_layoutreturn_free_lsegs() trondmy @ 2021-01-05 15:26 ` trondmy 0 siblings, 0 replies; 5+ messages in thread From: trondmy @ 2021-01-05 15:26 UTC (permalink / raw) To: linux-nfs From: Trond Myklebust <trond.myklebust@hammerspace.com> If a layout return is in progress, we should wait for it to complete, in case the layout segment we are picking up gets returned too. Fixes: 30cb3ee299cb ("pNFS: Handle NFS4ERR_OLD_STATEID on layoutreturn by bumping the state seqid") Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- fs/nfs/pnfs.c | 43 +++++++++++++++++++++---------------------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c index a0d8a67a4dd9..e4a27afeee90 100644 --- a/fs/nfs/pnfs.c +++ b/fs/nfs/pnfs.c @@ -2017,6 +2017,27 @@ pnfs_update_layout(struct inode *ino, goto lookup_again; } + /* + * Because we free lsegs when sending LAYOUTRETURN, we need to wait + * for LAYOUTRETURN. + */ + if (test_bit(NFS_LAYOUT_RETURN, &lo->plh_flags)) { + spin_unlock(&ino->i_lock); + dprintk("%s wait for layoutreturn\n", __func__); + lseg = ERR_PTR(pnfs_prepare_to_retry_layoutget(lo)); + if (!IS_ERR(lseg)) { + pnfs_put_layout_hdr(lo); + dprintk("%s retrying\n", __func__); + trace_pnfs_update_layout(ino, pos, count, iomode, lo, + lseg, + PNFS_UPDATE_LAYOUT_RETRY); + goto lookup_again; + } + trace_pnfs_update_layout(ino, pos, count, iomode, lo, lseg, + PNFS_UPDATE_LAYOUT_RETURN); + goto out_put_layout_hdr; + } + lseg = pnfs_find_lseg(lo, &arg, strict_iomode); if (lseg) { trace_pnfs_update_layout(ino, pos, count, iomode, lo, lseg, @@ -2069,28 +2090,6 @@ pnfs_update_layout(struct inode *ino, nfs4_stateid_copy(&stateid, &lo->plh_stateid); } - /* - * Because we free lsegs before sending LAYOUTRETURN, we need to wait - * for LAYOUTRETURN even if first is true. - */ - if (test_bit(NFS_LAYOUT_RETURN, &lo->plh_flags)) { - spin_unlock(&ino->i_lock); - dprintk("%s wait for layoutreturn\n", __func__); - lseg = ERR_PTR(pnfs_prepare_to_retry_layoutget(lo)); - if (!IS_ERR(lseg)) { - if (first) - pnfs_clear_first_layoutget(lo); - pnfs_put_layout_hdr(lo); - dprintk("%s retrying\n", __func__); - trace_pnfs_update_layout(ino, pos, count, iomode, lo, - lseg, PNFS_UPDATE_LAYOUT_RETRY); - goto lookup_again; - } - trace_pnfs_update_layout(ino, pos, count, iomode, lo, lseg, - PNFS_UPDATE_LAYOUT_RETURN); - goto out_put_layout_hdr; - } - if (pnfs_layoutgets_blocked(lo)) { trace_pnfs_update_layout(ino, pos, count, iomode, lo, lseg, PNFS_UPDATE_LAYOUT_BLOCKED); -- 2.29.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/4] pNFS: We want return-on-close to complete when evicting the inode 2021-01-05 15:26 ` [PATCH 2/4] pNFS: We want return-on-close to complete when evicting the inode trondmy 2021-01-05 15:26 ` [PATCH 3/4] pNFS: Clean up pnfs_layoutreturn_free_lsegs() trondmy @ 2021-01-06 15:25 ` Anna Schumaker 1 sibling, 0 replies; 5+ messages in thread From: Anna Schumaker @ 2021-01-06 15:25 UTC (permalink / raw) To: trondmy; +Cc: Linux NFS Mailing List Hi Trond, On Tue, Jan 5, 2021 at 10:30 AM <trondmy@kernel.org> wrote: > > From: Trond Myklebust <trond.myklebust@hammerspace.com> > > If the inode is being evicted, it should be safe to run return-on-close, > so we should do it to ensure we don't inadvertently leak layout segments. > > Fixes: 1c5bd76d17cc ("pNFS: Enable layoutreturn operation for return-on-close") > Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> > --- > fs/nfs/nfs4proc.c | 26 ++++++++++---------------- > fs/nfs/pnfs.c | 8 +++----- > fs/nfs/pnfs.h | 6 ++---- > 3 files changed, 15 insertions(+), 25 deletions(-) > > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c > index 0ce04e0e5d82..bbca5c46e400 100644 > --- a/fs/nfs/nfs4proc.c > +++ b/fs/nfs/nfs4proc.c > @@ -3536,10 +3536,8 @@ static void nfs4_close_done(struct rpc_task *task, void *data) > trace_nfs4_close(state, &calldata->arg, &calldata->res, task->tk_status); > > /* Handle Layoutreturn errors */ > - if (pnfs_roc_done(task, calldata->inode, > - &calldata->arg.lr_args, > - &calldata->res.lr_res, > - &calldata->res.lr_ret) == -EAGAIN) > + if (pnfs_roc_done(task, &calldata->arg.lr_args, &calldata->res.lr_res, > + &calldata->res.lr_ret) == -EAGAIN) > goto out_restart; > > /* hmm. we are done with the inode, and in the process of freeing > @@ -6384,10 +6382,8 @@ static void nfs4_delegreturn_done(struct rpc_task *task, void *calldata) > trace_nfs4_delegreturn_exit(&data->args, &data->res, task->tk_status); > > /* Handle Layoutreturn errors */ > - if (pnfs_roc_done(task, data->inode, > - &data->args.lr_args, > - &data->res.lr_res, > - &data->res.lr_ret) == -EAGAIN) > + if (pnfs_roc_done(task, &data->args.lr_args, &data->res.lr_res, > + &data->res.lr_ret) == -EAGAIN) > goto out_restart; > > switch (task->tk_status) { > @@ -6441,10 +6437,10 @@ static void nfs4_delegreturn_release(void *calldata) > struct nfs4_delegreturndata *data = calldata; > struct inode *inode = data->inode; > > + if (data->lr.roc) > + pnfs_roc_release(&data->lr.arg, &data->lr.res, > + data->res.lr_ret); > if (inode) { > - if (data->lr.roc) > - pnfs_roc_release(&data->lr.arg, &data->lr.res, > - data->res.lr_ret); > nfs_post_op_update_inode_force_wcc(inode, &data->fattr); > nfs_iput_and_deactive(inode); > } > @@ -6520,16 +6516,14 @@ static int _nfs4_proc_delegreturn(struct inode *inode, const struct cred *cred, > nfs_fattr_init(data->res.fattr); > data->timestamp = jiffies; > data->rpc_status = 0; > - data->lr.roc = pnfs_roc(inode, &data->lr.arg, &data->lr.res, cred); > data->inode = nfs_igrab_and_active(inode); > - if (data->inode) { > + if (data->inode || issync) { > + data->lr.roc = pnfs_roc(inode, &data->lr.arg, &data->lr.res, > + cred); > if (data->lr.roc) { > data->args.lr_args = &data->lr.arg; > data->res.lr_res = &data->lr.res; > } > - } else if (data->lr.roc) { > - pnfs_roc_release(&data->lr.arg, &data->lr.res, 0); > - data->lr.roc = false; > } > > task_setup_data.callback_data = data; > diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c > index ccc89fab1802..a18b1992b2fb 100644 > --- a/fs/nfs/pnfs.c > +++ b/fs/nfs/pnfs.c > @@ -1509,10 +1509,8 @@ bool pnfs_roc(struct inode *ino, > return false; > } > > -int pnfs_roc_done(struct rpc_task *task, struct inode *inode, > - struct nfs4_layoutreturn_args **argpp, > - struct nfs4_layoutreturn_res **respp, > - int *ret) > +int pnfs_roc_done(struct rpc_task *task, struct nfs4_layoutreturn_args **argpp, > + struct nfs4_layoutreturn_res **respp, int *ret) > { > struct nfs4_layoutreturn_args *arg = *argpp; > int retval = -EAGAIN; > @@ -1545,7 +1543,7 @@ int pnfs_roc_done(struct rpc_task *task, struct inode *inode, > return 0; > case -NFS4ERR_OLD_STATEID: > if (!nfs4_layout_refresh_old_stateid(&arg->stateid, > - &arg->range, inode)) > + &arg->range, arg->inode)) > break; > *ret = -NFS4ERR_NOMATCHING_LAYOUT; > return -EAGAIN; > diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h > index bbd3de1025f2..4d5ee568411d 100644 > --- a/fs/nfs/pnfs.h > +++ b/fs/nfs/pnfs.h > @@ -297,10 +297,8 @@ bool pnfs_roc(struct inode *ino, > struct nfs4_layoutreturn_args *args, > struct nfs4_layoutreturn_res *res, > const struct cred *cred); > -int pnfs_roc_done(struct rpc_task *task, struct inode *inode, > - struct nfs4_layoutreturn_args **argpp, > - struct nfs4_layoutreturn_res **respp, > - int *ret); > +int pnfs_roc_done(struct rpc_task *task, struct nfs4_layoutreturn_args **argpp, > + struct nfs4_layoutreturn_res **respp, int *ret); I think you also need to make this change to the fallback definition (for CONFIG_NFS_V4=n) Thanks, Anna > void pnfs_roc_release(struct nfs4_layoutreturn_args *args, > struct nfs4_layoutreturn_res *res, > int ret); > -- > 2.29.2 > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-01-06 15:26 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-01-05 15:26 [PATCH 1/4] pNFS: Mark layout for return if return-on-close was not sent trondmy 2021-01-05 15:26 ` [PATCH 2/4] pNFS: We want return-on-close to complete when evicting the inode trondmy 2021-01-05 15:26 ` [PATCH 3/4] pNFS: Clean up pnfs_layoutreturn_free_lsegs() trondmy 2021-01-05 15:26 ` [PATCH 4/4] pNFS: Stricter ordering of layoutget and layoutreturn trondmy 2021-01-06 15:25 ` [PATCH 2/4] pNFS: We want return-on-close to complete when evicting the inode Anna Schumaker
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).