On Mon, 2021-06-07 at 15:49 +0800, zhangxiaoxu (A) wrote:
>
>
> 在 2021/6/2 1:36, trondmy@xxxxxxxxxx 写道:
> > From: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
> >
> > If the inode is being evicted, but has to return a delegation
> > first,
> > then it can cause a deadlock in the corner case where the server
> > reboots
> > before the delegreturn completes, but while the call to
> > iget5_locked() in
> > nfs4_opendata_get_inode() is waiting for the inode free to
> > complete.
> > Since the open call still holds a session slot, the reboot recovery
> > cannot proceed.
> >
> > In order to break the logjam, we can turn the delegation return
> > into a
> > privileged operation for the case where we're evicting the inode.
> > We
> > know that in that case, there can be no other state recovery
> > operation
> > that conflicts.
> >
> it's looks good to me.
>
> but i have another confuse, how to ensure no writeback when evict nfs
> inode?
> because flush writes to server when close?
> but not all close will flush writes to server.
The struct nfs_open_context holds a reference to the dentry (which
holds a reference to the inode) and to the superblock. The struct
nfs_page that is tracking page dirtiness then holds a reference to the
nfs_open_context.
That mechanism ensures the inode cannot be evicted until all dirty
pages have been either flushed or cancelled. The only thing we need to
worry about is the delegation and the pNFS layout since neither one is
allowed to reference the inode in any way (because otherwise they would
prevent the memory reclaim mechanisms from working).
> > Reported-by: zhangxiaoxu (A) <zhangxiaoxu5@xxxxxxxxxx>
> > Fixes: 5fcdfacc01f3 ("NFSv4: Return delegations synchronously in
> > evict_inode")
> > Signed-off-by: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
> > ---
> > fs/nfs/nfs4_fs.h | 1 +
> > fs/nfs/nfs4proc.c | 12 +++++++++++-
> > 2 files changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
> > index 065cb04222a1..543d916f79ab 100644
> > --- a/fs/nfs/nfs4_fs.h
> > +++ b/fs/nfs/nfs4_fs.h
> > @@ -205,6 +205,7 @@ struct nfs4_exception {
> > struct inode *inode;
> > nfs4_stateid *stateid;
> > long timeout;
> > + unsigned char task_is_privileged : 1;
> > unsigned char delay : 1,
> > recovering : 1,
> > retry : 1;
> > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> > index d671b2884d5a..673809644981 100644
> > --- a/fs/nfs/nfs4proc.c
> > +++ b/fs/nfs/nfs4proc.c
> > @@ -589,6 +589,8 @@ int nfs4_handle_exception(struct nfs_server
> > *server, int errorcode, struct nfs4_
> > goto out_retry;
> > }
> > if (exception->recovering) {
> > + if (exception->task_is_privileged)
> > + return -EDEADLOCK;
> > ret = nfs4_wait_clnt_recover(clp);
> > if (test_bit(NFS_MIG_FAILED, &server->mig_status))
> > return -EIO;
> > @@ -614,6 +616,8 @@ nfs4_async_handle_exception(struct rpc_task
> > *task, struct nfs_server *server,
> > goto out_retry;
> > }
> > if (exception->recovering) {
> > + if (exception->task_is_privileged)
> > + return -EDEADLOCK;
> > rpc_sleep_on(&clp->cl_rpcwaitq, task, NULL);
> > if (test_bit(NFS4CLNT_MANAGER_RUNNING, &clp-
> > >cl_state) == 0)
> > rpc_wake_up_queued_task(&clp->cl_rpcwaitq,
> > task);
> > @@ -6417,6 +6421,7 @@ static void nfs4_delegreturn_done(struct
> > rpc_task *task, void *calldata)
> > struct nfs4_exception exception = {
> > .inode = data->inode,
> > .stateid = &data->stateid,
> > + .task_is_privileged = data-
> > >args.seq_args.sa_privileged,
> > };
> >
> > if (!nfs4_sequence_done(task, &data->res.seq_res))
> > @@ -6540,7 +6545,6 @@ static int _nfs4_proc_delegreturn(struct
> > inode *inode, const struct cred *cred,
> > data = kzalloc(sizeof(*data), GFP_NOFS);
> > if (data == NULL)
> > return -ENOMEM;
> > - nfs4_init_sequence(&data->args.seq_args, &data-
> > >res.seq_res, 1, 0);
> >
> > nfs4_state_protect(server->nfs_client,
> > NFS_SP4_MACH_CRED_CLEANUP,
> > @@ -6571,6 +6575,12 @@ static int _nfs4_proc_delegreturn(struct
> > inode *inode, const struct cred *cred,
> > }
> > }
> >
> > + if (!data->inode)
> > + nfs4_init_sequence(&data->args.seq_args, &data-
> > >res.seq_res, 1,
> > + 1);
> > + else
> > + nfs4_init_sequence(&data->args.seq_args, &data-
> > >res.seq_res, 1,
> > + 0);
> > task_setup_data.callback_data = data;
> > msg.rpc_argp = &data->args;
> > msg.rpc_resp = &data->res;
> >
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@xxxxxxxxxxxxxxx
