| Message ID | 20241213035908.1789132-1-lilingfeng3@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | NFSv4: Fix deadlock during the running of state manager | expand |
On Fri, 2024-12-13 at 11:59 +0800, Li Lingfeng wrote: > Unlinking file may cause the following deadlock in state manager: > [root@localhost test]# cat /proc/2943/stack > [<0>] rpc_wait_bit_killable+0x1a/0x90 > [<0>] _nfs4_proc_delegreturn+0x60f/0x760 > [<0>] nfs4_proc_delegreturn+0x13d/0x2a0 > [<0>] nfs_do_return_delegation+0xba/0x110 > [<0>] nfs_end_delegation_return+0x32c/0x620 > [<0>] nfs_complete_unlink+0xc7/0x290 > [<0>] nfs_dentry_iput+0x36/0x50 > [<0>] __dentry_kill+0xaa/0x250 > [<0>] dput.part.0+0x26c/0x4d0 > [<0>] __put_nfs_open_context+0x1d9/0x260 > [<0>] nfs4_open_reclaim+0x77/0xa0 > [<0>] nfs4_do_reclaim+0x385/0xf40 > [<0>] nfs4_state_manager+0x762/0x14e0 > [<0>] nfs4_run_state_manager+0x181/0x330 > [<0>] kthread+0x1a7/0x1f0 > [<0>] ret_from_fork+0x34/0x60 > [<0>] ret_from_fork_asm+0x1a/0x30 > [root@localhost test]# > > It can be reproduced by following steps: > 1) client: open file > 2) client: unlink file > 3) server: service restart(trigger state manager in client) > 4) client: close file(in nfs4_open_reclaim, between > nfs4_do_open_reclaim > and put_nfs_open_context) > > Since the file has been open, unlinking will just set > DCACHE_NFSFS_RENAMED > for the dentry like this: > nfs_unlink > nfs_sillyrename > nfs_async_unlink > // set DCACHE_NFSFS_RENAMED > > Restarting service will trigger state manager in client. > (1) NFS4_SLOT_TBL_DRAINING will be set to nfs4_slot_table since > session > has been reset. > (2) DCACHE_NFSFS_RENAMED is detected in nfs_dentry_iput. Therefore, > nfs_complete_unlink is called to trigger delegation return. > (3) Due to the slot table being in draining state and sa_privileged > being > 0, the delegation return will be queued and wait. > nfs4_state_manager > nfs4_reset_session > nfs4_begin_drain_session > nfs4_drain_slot_tbl > // set NFS4_SLOT_TBL_DRAINING (1) > nfs4_do_reclaim > nfs4_open_reclaim > __put_nfs_open_context > __dentry_kill > nfs_dentry_iput // check DCACHE_NFSFS_RENAMED (2) > nfs_complete_unlink > nfs_end_delegation_return > nfs_do_return_delegation > nfs4_proc_delegreturn > _nfs4_proc_delegreturn > rpc_run_task > ... > nfs4_delegreturn_prepare > nfs4_setup_sequence > nfs4_slot_tbl_draining // check NFS4_SLOT_TBL_DRAINING > // and sa_privileged is 0 (3) > rpc_sleep_on // set queued and add to slot_tbl_waitq > // rpc_task is async and wait in __rpc_execute > rpc_wait_for_completion_task > __rpc_wait_for_completion_task > out_of_line_wait_on_bit > rpc_wait_bit_killable // wait for rpc_task to complete > <-------- can not get here to wake up rpc_task --------> > nfs4_end_drain_session > nfs4_end_drain_slot_table > nfs41_wake_slot_table > > On the one hand, the state manager is blocked by the unfinished > delegation > return. As a result, nfs4_end_drain_session cannot be invoked to > clear > NFS4_SLOT_TBL_DRAINING and wake up waiting tasks. > On the other hand, since NFS4_SLOT_TBL_DRAINING is not cleared, > delegation return can only wait in the queue, resulting in a > deadlock. > > Fix it by turning the delegation return into a privileged operation > for > the case where the nfs_client is in NFS4CLNT_RECLAIM_REBOOT state. > > Fixes: 977fcc2b0b41 ("NFS: Add a delegation return into > nfs4_proc_unlink_setup()") > Signed-off-by: Li Lingfeng <lilingfeng3@huawei.com> > --- > fs/nfs/nfs4proc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c > index 405f17e6e0b4..f3b1f2725c4e 100644 > --- a/fs/nfs/nfs4proc.c > +++ b/fs/nfs/nfs4proc.c > @@ -6878,7 +6878,7 @@ static int _nfs4_proc_delegreturn(struct inode > *inode, const struct cred *cred, > data->res.sattr_res = true; > } > > - if (!data->inode) > + if (!data->inode || test_bit(NFS4CLNT_RECLAIM_REBOOT, > &server->nfs_client->cl_state)) > nfs4_init_sequence(&data->args.seq_args, &data- > >res.seq_res, 1, > 1); > else Rather than make the delegreturn be privileged, it seems better to make that delegreturn be asynchronous, just like the unlink itself.
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 405f17e6e0b4..f3b1f2725c4e 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -6878,7 +6878,7 @@ static int _nfs4_proc_delegreturn(struct inode *inode, const struct cred *cred, data->res.sattr_res = true; } - if (!data->inode) + if (!data->inode || test_bit(NFS4CLNT_RECLAIM_REBOOT, &server->nfs_client->cl_state)) nfs4_init_sequence(&data->args.seq_args, &data->res.seq_res, 1, 1); else
Unlinking file may cause the following deadlock in state manager: [root@localhost test]# cat /proc/2943/stack [<0>] rpc_wait_bit_killable+0x1a/0x90 [<0>] _nfs4_proc_delegreturn+0x60f/0x760 [<0>] nfs4_proc_delegreturn+0x13d/0x2a0 [<0>] nfs_do_return_delegation+0xba/0x110 [<0>] nfs_end_delegation_return+0x32c/0x620 [<0>] nfs_complete_unlink+0xc7/0x290 [<0>] nfs_dentry_iput+0x36/0x50 [<0>] __dentry_kill+0xaa/0x250 [<0>] dput.part.0+0x26c/0x4d0 [<0>] __put_nfs_open_context+0x1d9/0x260 [<0>] nfs4_open_reclaim+0x77/0xa0 [<0>] nfs4_do_reclaim+0x385/0xf40 [<0>] nfs4_state_manager+0x762/0x14e0 [<0>] nfs4_run_state_manager+0x181/0x330 [<0>] kthread+0x1a7/0x1f0 [<0>] ret_from_fork+0x34/0x60 [<0>] ret_from_fork_asm+0x1a/0x30 [root@localhost test]# It can be reproduced by following steps: 1) client: open file 2) client: unlink file 3) server: service restart(trigger state manager in client) 4) client: close file(in nfs4_open_reclaim, between nfs4_do_open_reclaim and put_nfs_open_context) Since the file has been open, unlinking will just set DCACHE_NFSFS_RENAMED for the dentry like this: nfs_unlink nfs_sillyrename nfs_async_unlink // set DCACHE_NFSFS_RENAMED Restarting service will trigger state manager in client. (1) NFS4_SLOT_TBL_DRAINING will be set to nfs4_slot_table since session has been reset. (2) DCACHE_NFSFS_RENAMED is detected in nfs_dentry_iput. Therefore, nfs_complete_unlink is called to trigger delegation return. (3) Due to the slot table being in draining state and sa_privileged being 0, the delegation return will be queued and wait. nfs4_state_manager nfs4_reset_session nfs4_begin_drain_session nfs4_drain_slot_tbl // set NFS4_SLOT_TBL_DRAINING (1) nfs4_do_reclaim nfs4_open_reclaim __put_nfs_open_context __dentry_kill nfs_dentry_iput // check DCACHE_NFSFS_RENAMED (2) nfs_complete_unlink nfs_end_delegation_return nfs_do_return_delegation nfs4_proc_delegreturn _nfs4_proc_delegreturn rpc_run_task ... nfs4_delegreturn_prepare nfs4_setup_sequence nfs4_slot_tbl_draining // check NFS4_SLOT_TBL_DRAINING // and sa_privileged is 0 (3) rpc_sleep_on // set queued and add to slot_tbl_waitq // rpc_task is async and wait in __rpc_execute rpc_wait_for_completion_task __rpc_wait_for_completion_task out_of_line_wait_on_bit rpc_wait_bit_killable // wait for rpc_task to complete <-------- can not get here to wake up rpc_task --------> nfs4_end_drain_session nfs4_end_drain_slot_table nfs41_wake_slot_table On the one hand, the state manager is blocked by the unfinished delegation return. As a result, nfs4_end_drain_session cannot be invoked to clear NFS4_SLOT_TBL_DRAINING and wake up waiting tasks. On the other hand, since NFS4_SLOT_TBL_DRAINING is not cleared, delegation return can only wait in the queue, resulting in a deadlock. Fix it by turning the delegation return into a privileged operation for the case where the nfs_client is in NFS4CLNT_RECLAIM_REBOOT state. Fixes: 977fcc2b0b41 ("NFS: Add a delegation return into nfs4_proc_unlink_setup()") Signed-off-by: Li Lingfeng <lilingfeng3@huawei.com> --- fs/nfs/nfs4proc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)