Re: [PATCH 1/2] NFSv4: Fix deadlock between nfs4_evict_inode() and nfs4_opendata_get_inode()

On Mon, 2021-06-07 at 15:49 +0800, zhangxiaoxu (A) wrote:

在 2021/6/2 1:36, trondmy@kernel.org 写道:

quoted

From: Trond Myklebust <redacted>

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).

quoted

Reported-by: zhangxiaoxu (A) <redacted>
Fixes: 5fcdfacc01f3 ("NFSv4: Return delegations synchronously in
evict_inode")
Signed-off-by: Trond Myklebust <redacted>
---
  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-

quoted

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-

quoted

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-

quoted

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-

quoted

res.seq_res, 1,

+                                  1);
+       else
+               nfs4_init_sequence(&data->args.seq_args, &data-

quoted

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@hammerspace.com