| Message ID | 20221111193639.346992-4-jlayton@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | filelock: WARN when @filp and fl_file don't match | expand |
> On Nov 11, 2022, at 2:36 PM, Jeff Layton <jlayton@kernel.org> wrote: > > We currently do a lock_to_openmode call based on the arguments from the > NLM_UNLOCK call, but that will always set the fl_type of the lock to > F_UNLCK, the the O_RDONLY descriptor is always chosen. Except for the above sentence, these all look sane to me. I can apply them to nfsd's for-next once they've seen some review on fsdevel, as you mentioned in the other thread. > Fix it to use the file_lock from the block instead. > > Signed-off-by: Jeff Layton <jlayton@kernel.org> > --- > fs/lockd/svclock.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c > index 9eae99e08e69..4e30f3c50970 100644 > --- a/fs/lockd/svclock.c > +++ b/fs/lockd/svclock.c > @@ -699,9 +699,10 @@ nlmsvc_cancel_blocked(struct net *net, struct nlm_file *file, struct nlm_lock *l > block = nlmsvc_lookup_block(file, lock); > mutex_unlock(&file->f_mutex); > if (block != NULL) { > - mode = lock_to_openmode(&lock->fl); > - vfs_cancel_lock(block->b_file->f_file[mode], > - &block->b_call->a_args.lock.fl); > + struct file_lock *fl = &block->b_call->a_args.lock.fl; > + > + mode = lock_to_openmode(fl); > + vfs_cancel_lock(block->b_file->f_file[mode], fl); > status = nlmsvc_unlink_block(block); > nlmsvc_release_block(block); > } > -- > 2.38.1 > -- Chuck Lever
On Fri, 2022-11-11 at 20:29 +0000, Chuck Lever III wrote: > > > On Nov 11, 2022, at 2:36 PM, Jeff Layton <jlayton@kernel.org> wrote: > > > > We currently do a lock_to_openmode call based on the arguments from the > > NLM_UNLOCK call, but that will always set the fl_type of the lock to > > F_UNLCK, the the O_RDONLY descriptor is always chosen. > > Except for the above sentence, these all look sane to me. > I can apply them to nfsd's for-next once they've seen some > review on fsdevel, as you mentioned in the other thread. > > Thanks. That should say "and the O_RDONLY...". Fixed in my tree. I'll go ahead and resend with fsdevel included. > > Fix it to use the file_lock from the block instead. > > > > Signed-off-by: Jeff Layton <jlayton@kernel.org> > > --- > > fs/lockd/svclock.c | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c > > index 9eae99e08e69..4e30f3c50970 100644 > > --- a/fs/lockd/svclock.c > > +++ b/fs/lockd/svclock.c > > @@ -699,9 +699,10 @@ nlmsvc_cancel_blocked(struct net *net, struct nlm_file *file, struct nlm_lock *l > > block = nlmsvc_lookup_block(file, lock); > > mutex_unlock(&file->f_mutex); > > if (block != NULL) { > > - mode = lock_to_openmode(&lock->fl); > > - vfs_cancel_lock(block->b_file->f_file[mode], > > - &block->b_call->a_args.lock.fl); > > + struct file_lock *fl = &block->b_call->a_args.lock.fl; > > + > > + mode = lock_to_openmode(fl); > > + vfs_cancel_lock(block->b_file->f_file[mode], fl); > > status = nlmsvc_unlink_block(block); > > nlmsvc_release_block(block); > > } > > -- > > 2.38.1 > > > > -- > Chuck Lever > > >
On Fri, 2022-11-11 at 16:52 -0500, Jeff Layton wrote: > On Fri, 2022-11-11 at 20:29 +0000, Chuck Lever III wrote: > > > > > On Nov 11, 2022, at 2:36 PM, Jeff Layton <jlayton@kernel.org> wrote: > > > > > > We currently do a lock_to_openmode call based on the arguments from the > > > NLM_UNLOCK call, but that will always set the fl_type of the lock to > > > F_UNLCK, the the O_RDONLY descriptor is always chosen. > > > > Except for the above sentence, these all look sane to me. > > I can apply them to nfsd's for-next once they've seen some > > review on fsdevel, as you mentioned in the other thread. > > > > > > Thanks. That should say "and the O_RDONLY...". Fixed in my tree. > > I'll go ahead and resend with fsdevel included. > I reposted the series Friday afternoon. What might be best is for you to carry the first 3 patches in the nfsd tree, and I'll take the filelock: patch into the locks-next branch, along with the other filelock API cleanups. Sound OK? > > > Fix it to use the file_lock from the block instead. > > > > > > Signed-off-by: Jeff Layton <jlayton@kernel.org> > > > --- > > > fs/lockd/svclock.c | 7 ++++--- > > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > > > diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c > > > index 9eae99e08e69..4e30f3c50970 100644 > > > --- a/fs/lockd/svclock.c > > > +++ b/fs/lockd/svclock.c > > > @@ -699,9 +699,10 @@ nlmsvc_cancel_blocked(struct net *net, struct nlm_file *file, struct nlm_lock *l > > > block = nlmsvc_lookup_block(file, lock); > > > mutex_unlock(&file->f_mutex); > > > if (block != NULL) { > > > - mode = lock_to_openmode(&lock->fl); > > > - vfs_cancel_lock(block->b_file->f_file[mode], > > > - &block->b_call->a_args.lock.fl); > > > + struct file_lock *fl = &block->b_call->a_args.lock.fl; > > > + > > > + mode = lock_to_openmode(fl); > > > + vfs_cancel_lock(block->b_file->f_file[mode], fl); > > > status = nlmsvc_unlink_block(block); > > > nlmsvc_release_block(block); > > > } > > > -- > > > 2.38.1 > > > > > > > -- > > Chuck Lever > > > > > > >
> On Nov 14, 2022, at 1:38 PM, Jeff Layton <jlayton@kernel.org> wrote: > > On Fri, 2022-11-11 at 16:52 -0500, Jeff Layton wrote: >> On Fri, 2022-11-11 at 20:29 +0000, Chuck Lever III wrote: >>> >>>> On Nov 11, 2022, at 2:36 PM, Jeff Layton <jlayton@kernel.org> wrote: >>>> >>>> We currently do a lock_to_openmode call based on the arguments from the >>>> NLM_UNLOCK call, but that will always set the fl_type of the lock to >>>> F_UNLCK, the the O_RDONLY descriptor is always chosen. >>> >>> Except for the above sentence, these all look sane to me. >>> I can apply them to nfsd's for-next once they've seen some >>> review on fsdevel, as you mentioned in the other thread. >>> >>> >> >> Thanks. That should say "and the O_RDONLY...". Fixed in my tree. >> >> I'll go ahead and resend with fsdevel included. >> > > I reposted the series Friday afternoon. > > What might be best is for you to carry the first 3 patches in the nfsd > tree, and I'll take the filelock: patch into the locks-next branch, > along with the other filelock API cleanups. > > Sound OK? 1/4 through 3/4 have been applied and pushed. Thanks! -- Chuck Lever
diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c index 9eae99e08e69..4e30f3c50970 100644 --- a/fs/lockd/svclock.c +++ b/fs/lockd/svclock.c @@ -699,9 +699,10 @@ nlmsvc_cancel_blocked(struct net *net, struct nlm_file *file, struct nlm_lock *l block = nlmsvc_lookup_block(file, lock); mutex_unlock(&file->f_mutex); if (block != NULL) { - mode = lock_to_openmode(&lock->fl); - vfs_cancel_lock(block->b_file->f_file[mode], - &block->b_call->a_args.lock.fl); + struct file_lock *fl = &block->b_call->a_args.lock.fl; + + mode = lock_to_openmode(fl); + vfs_cancel_lock(block->b_file->f_file[mode], fl); status = nlmsvc_unlink_block(block); nlmsvc_release_block(block); }
We currently do a lock_to_openmode call based on the arguments from the NLM_UNLOCK call, but that will always set the fl_type of the lock to F_UNLCK, the the O_RDONLY descriptor is always chosen. Fix it to use the file_lock from the block instead. Signed-off-by: Jeff Layton <jlayton@kernel.org> --- fs/lockd/svclock.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)