| Message ID | 20230823213352.1971009-4-aahringo@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | lockd: dlm: async lock request changes | expand |
On Wed, Aug 23, 2023 at 05:33:48PM -0400, Alexander Aring wrote: > This patch fixes a race in async lock request handling between adding > the relevant struct nlm_block to nlm_blocked list after the request was > sent by vfs_lock_file() and nlmsvc_grant_deferred() does a lookup of the > nlm_block in the nlm_blocked list. It could be that the async request is > completed before the nlm_block was added to the list. This would end > in a -ENOENT and a kernel log message of "lockd: grant for unknown > block". > > To solve this issue we add the nlm_block before the vfs_lock_file() call > to be sure it has been added when a possible nlmsvc_grant_deferred() is > called. If the vfs_lock_file() results in an case when it wouldn't be > added to nlm_blocked list, the nlm_block struct will be removed from > this list again. > > The introducing of the new B_PENDING_CALLBACK nlm_block flag will handle > async lock requests on a pending lock requests as a retry on the caller > level to hit the -EAGAIN case. This last paragraph might be obsolete? I don't see a B_PENDING_CALLBACK flag in the kernel or in this patch series. > Signed-off-by: Alexander Aring <aahringo@redhat.com> > --- > fs/lockd/svclock.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c > index aa4174fbaf5b..3b158446203b 100644 > --- a/fs/lockd/svclock.c > +++ b/fs/lockd/svclock.c > @@ -546,6 +546,9 @@ nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file, > ret = nlm_lck_blocked; > goto out; > } > + > + /* Append to list of blocked */ > + nlmsvc_insert_block_locked(block, NLM_NEVER); > spin_unlock(&nlm_blocked_lock); > > if (!wait) > @@ -557,9 +560,12 @@ nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file, > dprintk("lockd: vfs_lock_file returned %d\n", error); > switch (error) { > case 0: > + nlmsvc_remove_block(block); > ret = nlm_granted; > goto out; > case -EAGAIN: > + if (!wait) > + nlmsvc_remove_block(block); > ret = async_block ? nlm_lck_blocked : nlm_lck_denied; > goto out; > case FILE_LOCK_DEFERRED: > @@ -570,17 +576,16 @@ nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file, > ret = nlmsvc_defer_lock_rqst(rqstp, block); > goto out; > case -EDEADLK: > + nlmsvc_remove_block(block); > ret = nlm_deadlock; > goto out; > default: /* includes ENOLCK */ > + nlmsvc_remove_block(block); > ret = nlm_lck_denied_nolocks; > goto out; > } > > ret = nlm_lck_blocked; > - > - /* Append to list of blocked */ > - nlmsvc_insert_block(block, NLM_NEVER); > out: > mutex_unlock(&file->f_mutex); > nlmsvc_release_block(block); > -- > 2.31.1 >
On Wed, 2023-08-23 at 17:33 -0400, Alexander Aring wrote: > This patch fixes a race in async lock request handling between adding > the relevant struct nlm_block to nlm_blocked list after the request was > sent by vfs_lock_file() and nlmsvc_grant_deferred() does a lookup of the > nlm_block in the nlm_blocked list. It could be that the async request is > completed before the nlm_block was added to the list. This would end > in a -ENOENT and a kernel log message of "lockd: grant for unknown > block". > > To solve this issue we add the nlm_block before the vfs_lock_file() call > to be sure it has been added when a possible nlmsvc_grant_deferred() is > called. If the vfs_lock_file() results in an case when it wouldn't be > added to nlm_blocked list, the nlm_block struct will be removed from > this list again. > > The introducing of the new B_PENDING_CALLBACK nlm_block flag will handle > async lock requests on a pending lock requests as a retry on the caller > level to hit the -EAGAIN case. > > Signed-off-by: Alexander Aring <aahringo@redhat.com> > --- > fs/lockd/svclock.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c > index aa4174fbaf5b..3b158446203b 100644 > --- a/fs/lockd/svclock.c > +++ b/fs/lockd/svclock.c > @@ -546,6 +546,9 @@ nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file, > ret = nlm_lck_blocked; > goto out; > } > + > + /* Append to list of blocked */ > + nlmsvc_insert_block_locked(block, NLM_NEVER); > spin_unlock(&nlm_blocked_lock); > > if (!wait) > @@ -557,9 +560,12 @@ nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file, > dprintk("lockd: vfs_lock_file returned %d\n", error); > switch (error) { > case 0: > + nlmsvc_remove_block(block); > ret = nlm_granted; > goto out; > case -EAGAIN: > + if (!wait) > + nlmsvc_remove_block(block); > ret = async_block ? nlm_lck_blocked : nlm_lck_denied; > goto out; > case FILE_LOCK_DEFERRED: > @@ -570,17 +576,16 @@ nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file, > ret = nlmsvc_defer_lock_rqst(rqstp, block); > goto out; > case -EDEADLK: > + nlmsvc_remove_block(block); > ret = nlm_deadlock; > goto out; > default: /* includes ENOLCK */ > + nlmsvc_remove_block(block); > ret = nlm_lck_denied_nolocks; > goto out; > } > > ret = nlm_lck_blocked; > - > - /* Append to list of blocked */ > - nlmsvc_insert_block(block, NLM_NEVER); > out: > mutex_unlock(&file->f_mutex); > nlmsvc_release_block(block); Reviewed-by: Jeff Layton <jlayton@kernel.org>
diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c index aa4174fbaf5b..3b158446203b 100644 --- a/fs/lockd/svclock.c +++ b/fs/lockd/svclock.c @@ -546,6 +546,9 @@ nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file, ret = nlm_lck_blocked; goto out; } + + /* Append to list of blocked */ + nlmsvc_insert_block_locked(block, NLM_NEVER); spin_unlock(&nlm_blocked_lock); if (!wait) @@ -557,9 +560,12 @@ nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file, dprintk("lockd: vfs_lock_file returned %d\n", error); switch (error) { case 0: + nlmsvc_remove_block(block); ret = nlm_granted; goto out; case -EAGAIN: + if (!wait) + nlmsvc_remove_block(block); ret = async_block ? nlm_lck_blocked : nlm_lck_denied; goto out; case FILE_LOCK_DEFERRED: @@ -570,17 +576,16 @@ nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file, ret = nlmsvc_defer_lock_rqst(rqstp, block); goto out; case -EDEADLK: + nlmsvc_remove_block(block); ret = nlm_deadlock; goto out; default: /* includes ENOLCK */ + nlmsvc_remove_block(block); ret = nlm_lck_denied_nolocks; goto out; } ret = nlm_lck_blocked; - - /* Append to list of blocked */ - nlmsvc_insert_block(block, NLM_NEVER); out: mutex_unlock(&file->f_mutex); nlmsvc_release_block(block);
This patch fixes a race in async lock request handling between adding the relevant struct nlm_block to nlm_blocked list after the request was sent by vfs_lock_file() and nlmsvc_grant_deferred() does a lookup of the nlm_block in the nlm_blocked list. It could be that the async request is completed before the nlm_block was added to the list. This would end in a -ENOENT and a kernel log message of "lockd: grant for unknown block". To solve this issue we add the nlm_block before the vfs_lock_file() call to be sure it has been added when a possible nlmsvc_grant_deferred() is called. If the vfs_lock_file() results in an case when it wouldn't be added to nlm_blocked list, the nlm_block struct will be removed from this list again. The introducing of the new B_PENDING_CALLBACK nlm_block flag will handle async lock requests on a pending lock requests as a retry on the caller level to hit the -EAGAIN case. Signed-off-by: Alexander Aring <aahringo@redhat.com> --- fs/lockd/svclock.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)