Hi Trond, On Tue, Jan 5, 2021 at 10:30 AM <trondmy@xxxxxxxxxx> wrote: > > From: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx> > > 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@xxxxxxxxxxxxxxx> > --- > 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 >