Messages in this thread Patch in this message | | | Date | Fri, 24 Dec 2021 08:14:05 +0900 | From | Dominique Martinet <> | Subject | Re: [PATCH] v9fs: handle async processing of F_SETLK with FL_SLEEP flag |
| |
Vasily Averin wrote on Thu, Dec 23, 2021 at 09:21:23PM +0300: > kernel export thread (nfsd/lockd/ksmbd) uses F_SETLK cmd with the FL_SLEEP > flag set to request asynchronous processing of blocking locks. > > Currently v9fs does not support such requests and calls blocking > locks_lock_file_wait() function.
There's two stages to 9p locks: the client first tries to get the lock locally on the client, then if it was obtained locally also tries to get it on the server. I believe the servers should just ignores flags like FL_SLEEP they don't know about, so we need to translate it as well if required.
> To work around the problem let's detect such request, drop FL_SLEEP > before execution of potentially blocking functions.
I'm not up to date with lock mechanisms, could you confirm I understand the flags right? - F_SETLK: tries to lock, on conflict return immediately with error - F_SETLKW|FL_SLEEP: tries to lock, on conflict wait for lock to become available - F_SETLKW: not possible through flock/fcntl setlk, can happen otherwise? but for 9p purpose same as above. - F_SETLK|FL_SLEEP: tries to lock, on conflict ????? you'd want it to return immediately but setup some callback to be woken up? how could that work without passing some wake up struct? or just behave as plain F_SETLK? but then FL_SLEEP has no purpose, I don't get it.
> > Dropped FL_SLEEP flag should be restored back because some calling > function (nfsd4_lock) require it. > > https://bugzilla.kernel.org/show_bug.cgi?id=215383 > Signed-off-by: Vasily Averin <vvs@virtuozzo.com> > --- > fs/9p/vfs_file.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/fs/9p/vfs_file.c b/fs/9p/vfs_file.c > index 612e297f3763..81c98afdbb32 100644 > --- a/fs/9p/vfs_file.c > +++ b/fs/9p/vfs_file.c > @@ -135,6 +135,7 @@ static int v9fs_file_do_lock(struct file *filp, int cmd, struct file_lock *fl) > int res = 0; > unsigned char fl_type; > struct v9fs_session_info *v9ses; > + bool async = false; > > fid = filp->private_data; > BUG_ON(fid == NULL); > @@ -142,6 +143,10 @@ static int v9fs_file_do_lock(struct file *filp, int cmd, struct file_lock *fl) > if ((fl->fl_flags & FL_POSIX) != FL_POSIX) > BUG(); > > + async = (fl->fl_flags & FL_SLEEP) && IS_SETLK(cmd); > + if (async) > + fl->fl_flags &= ~FL_SLEEP; > +
So clearing the flag makes the local lock not wait at all in case of SETLK|FL_SLEEP, and this errors instead.
I can't comment on this without understanding what's expected better
> res = locks_lock_file_wait(filp, fl); > if (res < 0) > goto out; > @@ -230,6 +235,8 @@ static int v9fs_file_do_lock(struct file *filp, int cmd, struct file_lock *fl) > if (flock.client_id != fid->clnt->name) > kfree(flock.client_id); > out: > + if (async) > + fl->fl_flags |= FL_SLEEP; > return res; > } >
-- Dominique
| |