| Message ID | 20230814211116.3224759-5-aahringo@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fs: nfs: async lock request changes | expand |
On Mon, 2023-08-14 at 17:11 -0400, Alexander Aring wrote: > This patch updates the existing documentation regarding recent changes > to vfs_lock_file() and lm_grant() is set. In case of lm_grant() is set > we only handle FILE_LOCK_DEFERRED in case of FL_SLEEP in fl_flags is not > set. This is the case of an blocking lock request. Non-blocking lock > requests, when FL_SLEEP is not set, are handled in a synchronized way. > > Signed-off-by: Alexander Aring <aahringo@redhat.com> > --- > fs/locks.c | 28 ++++++++++++++-------------- > 1 file changed, 14 insertions(+), 14 deletions(-) > > diff --git a/fs/locks.c b/fs/locks.c > index df8b26a42524..a8e51f462b43 100644 > --- a/fs/locks.c > +++ b/fs/locks.c > @@ -2255,21 +2255,21 @@ int fcntl_getlk(struct file *filp, unsigned int cmd, struct flock *flock) > * To avoid blocking kernel daemons, such as lockd, that need to acquire POSIX > * locks, the ->lock() interface may return asynchronously, before the lock has > * been granted or denied by the underlying filesystem, if (and only if) > - * lm_grant is set. Callers expecting ->lock() to return asynchronously > - * will only use F_SETLK, not F_SETLKW; they will set FL_SLEEP if (and only if) > - * the request is for a blocking lock. When ->lock() does return asynchronously, > - * it must return FILE_LOCK_DEFERRED, and call ->lm_grant() when the lock > - * request completes. > - * If the request is for non-blocking lock the file system should return > - * FILE_LOCK_DEFERRED then try to get the lock and call the callback routine > - * with the result. If the request timed out the callback routine will return a > + * lm_grant and FL_SLEEP in fl_flags is set. Callers expecting ->lock() to return > + * asynchronously will only use F_SETLK, not F_SETLKW; When ->lock() does return Isn't the above backward? Shouldn't it say "Callers expecting ->lock() to return asynchronously will only use F_SETLKW, not F_SETLK" ? > + * asynchronously, it must return FILE_LOCK_DEFERRED, and call ->lm_grant() when > + * the lock request completes. The lm_grant() callback must be called in a > + * sleepable context. > + * > + * If the request timed out the ->lm_grant() callback routine will return a > * nonzero return code and the file system should release the lock. The file > - * system is also responsible to keep a corresponding posix lock when it > - * grants a lock so the VFS can find out which locks are locally held and do > - * the correct lock cleanup when required. > - * The underlying filesystem must not drop the kernel lock or call > - * ->lm_grant() before returning to the caller with a FILE_LOCK_DEFERRED > - * return code. > + * system is also responsible to keep a corresponding posix lock when it grants > + * a lock so the VFS can find out which locks are locally held and do the correct > + * lock cleanup when required. > + * > + * If the request is for non-blocking lock (when F_SETLK and FL_SLEEP in fl_flags is not set) > + * the file system should return -EAGAIN if failed to acquire or zero if acquiring was > + * successfully without calling the ->lm_grant() callback routine. > */ > int vfs_lock_file(struct file *filp, unsigned int cmd, struct file_lock *fl, struct file_lock *conf) > {
Hi, On Wed, Aug 16, 2023 at 8:01 AM Jeff Layton <jlayton@kernel.org> wrote: > > On Mon, 2023-08-14 at 17:11 -0400, Alexander Aring wrote: > > This patch updates the existing documentation regarding recent changes > > to vfs_lock_file() and lm_grant() is set. In case of lm_grant() is set > > we only handle FILE_LOCK_DEFERRED in case of FL_SLEEP in fl_flags is not > > set. This is the case of an blocking lock request. Non-blocking lock > > requests, when FL_SLEEP is not set, are handled in a synchronized way. > > > > Signed-off-by: Alexander Aring <aahringo@redhat.com> > > --- > > fs/locks.c | 28 ++++++++++++++-------------- > > 1 file changed, 14 insertions(+), 14 deletions(-) > > > > diff --git a/fs/locks.c b/fs/locks.c > > index df8b26a42524..a8e51f462b43 100644 > > --- a/fs/locks.c > > +++ b/fs/locks.c > > @@ -2255,21 +2255,21 @@ int fcntl_getlk(struct file *filp, unsigned int cmd, struct flock *flock) > > * To avoid blocking kernel daemons, such as lockd, that need to acquire POSIX > > * locks, the ->lock() interface may return asynchronously, before the lock has > > * been granted or denied by the underlying filesystem, if (and only if) > > - * lm_grant is set. Callers expecting ->lock() to return asynchronously > > - * will only use F_SETLK, not F_SETLKW; they will set FL_SLEEP if (and only if) > > - * the request is for a blocking lock. When ->lock() does return asynchronously, > > - * it must return FILE_LOCK_DEFERRED, and call ->lm_grant() when the lock > > - * request completes. > > - * If the request is for non-blocking lock the file system should return > > - * FILE_LOCK_DEFERRED then try to get the lock and call the callback routine > > - * with the result. If the request timed out the callback routine will return a > > + * lm_grant and FL_SLEEP in fl_flags is set. Callers expecting ->lock() to return > > + * asynchronously will only use F_SETLK, not F_SETLKW; When ->lock() does return > > Isn't the above backward? Shouldn't it say "Callers expecting ->lock() > to return asynchronously will only use F_SETLKW, not F_SETLK" ? > So far I know lockd will always use F_SETLK only, if it's a blocking or non-blocking request you need to evaluate FL_SLEEP. But if lm_grant() is not set we are using a check on cmd if it's F_SETLK or F_SETLKW to check if it's non-blocking or blocking. If lm_grant() is set and checking on F_SETLKW should never be the case, because it will never be true (speaking from lockd point of view). - Alex
diff --git a/fs/locks.c b/fs/locks.c index df8b26a42524..a8e51f462b43 100644 --- a/fs/locks.c +++ b/fs/locks.c @@ -2255,21 +2255,21 @@ int fcntl_getlk(struct file *filp, unsigned int cmd, struct flock *flock) * To avoid blocking kernel daemons, such as lockd, that need to acquire POSIX * locks, the ->lock() interface may return asynchronously, before the lock has * been granted or denied by the underlying filesystem, if (and only if) - * lm_grant is set. Callers expecting ->lock() to return asynchronously - * will only use F_SETLK, not F_SETLKW; they will set FL_SLEEP if (and only if) - * the request is for a blocking lock. When ->lock() does return asynchronously, - * it must return FILE_LOCK_DEFERRED, and call ->lm_grant() when the lock - * request completes. - * If the request is for non-blocking lock the file system should return - * FILE_LOCK_DEFERRED then try to get the lock and call the callback routine - * with the result. If the request timed out the callback routine will return a + * lm_grant and FL_SLEEP in fl_flags is set. Callers expecting ->lock() to return + * asynchronously will only use F_SETLK, not F_SETLKW; When ->lock() does return + * asynchronously, it must return FILE_LOCK_DEFERRED, and call ->lm_grant() when + * the lock request completes. The lm_grant() callback must be called in a + * sleepable context. + * + * If the request timed out the ->lm_grant() callback routine will return a * nonzero return code and the file system should release the lock. The file - * system is also responsible to keep a corresponding posix lock when it - * grants a lock so the VFS can find out which locks are locally held and do - * the correct lock cleanup when required. - * The underlying filesystem must not drop the kernel lock or call - * ->lm_grant() before returning to the caller with a FILE_LOCK_DEFERRED - * return code. + * system is also responsible to keep a corresponding posix lock when it grants + * a lock so the VFS can find out which locks are locally held and do the correct + * lock cleanup when required. + * + * If the request is for non-blocking lock (when F_SETLK and FL_SLEEP in fl_flags is not set) + * the file system should return -EAGAIN if failed to acquire or zero if acquiring was + * successfully without calling the ->lm_grant() callback routine. */ int vfs_lock_file(struct file *filp, unsigned int cmd, struct file_lock *fl, struct file_lock *conf) {
This patch updates the existing documentation regarding recent changes to vfs_lock_file() and lm_grant() is set. In case of lm_grant() is set we only handle FILE_LOCK_DEFERRED in case of FL_SLEEP in fl_flags is not set. This is the case of an blocking lock request. Non-blocking lock requests, when FL_SLEEP is not set, are handled in a synchronized way. Signed-off-by: Alexander Aring <aahringo@redhat.com> --- fs/locks.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-)