On 03.01.2022 22:40, J. Bruce Fields wrote: > On Wed, Dec 29, 2021 at 11:24:43AM +0300, Vasily Averin wrote: >> nfsd and lockd use F_SETLK cmd with the FL_SLEEP flag set to request >> asynchronous processing of blocking locks. >> >> Currently nfs4 use locks_lock_inode_wait() function which is blocked >> for such requests. To handle them correctly FL_SLEEP flag should be >> temporarily reset before executing the locks_lock_inode_wait() function. >> >> Additionally block flag is forced to set, to translate blocking lock to >> remote nfs server, expecting it supports async processing of the blocking >> locks too. > > Seems like an improvement, but is there some way to make this more > straightforward by just calling a function that doesn't sleep in the > first place? (posix_lock_inode(), maybe?) There are few problems: 1) posix_lock_inode() is static in fs/locks.c 2) exported posix_lock_file() used posix_lock_inode() inside requires file pointer, and I do not understand how to get it. 3) _nfs4_do_setlk() is called from do_setlk and handles flocks too, therefore any posix-only calls requires additional checks or branches. On the other hand all that is required to handle F_SETLK with FL_SLEEP correctly : to avoid blocking on exiting lock. We can reach this goal here by drop of FL_SLEEP flag before locks_lock_inode_wait() execution. Thank you, Vasily Averin PS I'm worry for a long delay with answer, in Russia we have long holidays after New Year, then I dealt with urgent tasks accumulated over the holidays then I forgot the context of this patch and I was need to spend some time to re-member the details. >> https://bugzilla.kernel.org/show_bug.cgi?id=215383 >> Signed-off-by: Vasily Averin <vvs@xxxxxxxxxxxxx> >> --- >> fs/nfs/nfs4proc.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c >> index ee3bc79f6ca3..9b1380c4223c 100644 >> --- a/fs/nfs/nfs4proc.c >> +++ b/fs/nfs/nfs4proc.c >> @@ -7094,7 +7094,7 @@ static int _nfs4_do_setlk(struct nfs4_state *state, int cmd, struct file_lock *f >> recovery_type == NFS_LOCK_NEW ? GFP_KERNEL : GFP_NOFS); >> if (data == NULL) >> return -ENOMEM; >> - if (IS_SETLKW(cmd)) >> + if (IS_SETLKW(cmd) || (fl->fl_flags & FL_SLEEP)) >> data->arg.block = 1; >> nfs4_init_sequence(&data->arg.seq_args, &data->res.seq_res, 1, >> recovery_type > NFS_LOCK_NEW); >> @@ -7200,6 +7200,9 @@ static int _nfs4_proc_setlk(struct nfs4_state *state, int cmd, struct file_lock >> int status; >> >> request->fl_flags |= FL_ACCESS; >> + if (((fl_flags & FL_SLEEP_POSIX) == FL_SLEEP_POSIX) && IS_SETLK(cmd)) >> + request->fl_flags &= ~FL_SLEEP; >> + >> status = locks_lock_inode_wait(state->inode, request); >> if (status < 0) >> goto out; >> -- >> 2.25.1
