[PATCH 0/3] filelock: remove redundant filp arguments from API

[PATCH 0/3] filelock: remove redundant filp arguments from API

[Jeff Layton]

Some of the exported functions in fs/locks.c take both a struct file
argument and a struct file_lock. struct file_lock has a dedicated field
to record which file it was set on (fl_file). This is redundant, and
there have been some cases where the two didn't match [1], leading to
bugs.

This patchset is intended to remove this ambiguity by eliminating the
separate struct file argument from vfs_lock_file, vfs_test_lock and
vfs_cancel_lock.

Most callers are easy to vet to ensure that they set this correctly, but
lockd had a few places where it wasn't doing the right thing. This
series depends on the lockd patches I sent late last week [2].

I'm targeting this series for v6.3. I'll plan to get it into linux-next
soon unless there are objections.

[1]: https://bugzilla.kernel.org/show_bug.cgi?id=216582
[2]: https://lore.kernel.org/linux-nfs/20221111215538.356543-1-jlayton@kernel.org/T/#t

Jeff Layton (3):
  filelock: remove redundant filp argument from vfs_lock_file
  filelock: remove redundant filp argument from vfs_test_lock
  filelock: remove redundant filp arg from vfs_cancel_lock

 fs/ksmbd/smb2pdu.c  |  4 ++--
 fs/lockd/svclock.c  | 21 +++++++--------------
 fs/lockd/svcsubs.c  |  4 ++--
 fs/locks.c          | 29 ++++++++++++++---------------
 fs/nfsd/nfs4state.c |  6 +++---
 include/linux/fs.h  | 14 +++++++-------
 6 files changed, 35 insertions(+), 43 deletions(-)

-- 
2.38.1

[PATCH 1/3] filelock: remove redundant filp argument from vfs_lock_file

[Jeff Layton]

The existing API requires that the fl_file field be filled out when
calling it, as some underlying filesystems require that information
deep down in their call stacks.

Simplify vfs_lock_file by removing the redundant @filp argument and
using fl_file in its place.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/ksmbd/smb2pdu.c  |  4 ++--
 fs/lockd/svclock.c  | 13 +++++--------
 fs/lockd/svcsubs.c  |  4 ++--
 fs/locks.c          | 13 ++++++-------
 fs/nfsd/nfs4state.c |  4 ++--
 include/linux/fs.h  |  6 +++---
 6 files changed, 20 insertions(+), 24 deletions(-)

diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
index f2bcd2a5fb7f..4668553be5e3 100644
--- a/fs/ksmbd/smb2pdu.c
+++ b/fs/ksmbd/smb2pdu.c
@@ -7007,7 +7007,7 @@ int smb2_lock(struct ksmbd_work *work)
 		flock = smb_lock->fl;
 		list_del(&smb_lock->llist);
 retry:
-		rc = vfs_lock_file(filp, smb_lock->cmd, flock, NULL);
+		rc = vfs_lock_file(smb_lock->cmd, flock, NULL);
 skip:
 		if (flags & SMB2_LOCKFLAG_UNLOCK) {
 			if (!rc) {
@@ -7129,7 +7129,7 @@ int smb2_lock(struct ksmbd_work *work)
 		rlock->fl_start = smb_lock->start;
 		rlock->fl_end = smb_lock->end;
 
-		rc = vfs_lock_file(filp, F_SETLK, rlock, NULL);
+		rc = vfs_lock_file(F_SETLK, rlock, NULL);
 		if (rc)
 			pr_err("rollback unlock fail : %d\n", rc);
 
diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c
index 4e30f3c50970..c965783b71a6 100644
--- a/fs/lockd/svclock.c
+++ b/fs/lockd/svclock.c
@@ -475,7 +475,6 @@ nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file,
 #endif
 	struct nlm_block	*block = NULL;
 	int			error;
-	int			mode;
 	int			async_block = 0;
 	__be32			ret;
 
@@ -534,8 +533,7 @@ nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file,
 
 	if (!wait)
 		lock->fl.fl_flags &= ~FL_SLEEP;
-	mode = lock_to_openmode(&lock->fl);
-	error = vfs_lock_file(file->f_file[mode], F_SETLK, &lock->fl, NULL);
+	error = vfs_lock_file(F_SETLK, &lock->fl, NULL);
 	lock->fl.fl_flags &= ~FL_SLEEP;
 
 	dprintk("lockd: vfs_lock_file returned %d\n", error);
@@ -661,12 +659,10 @@ nlmsvc_unlock(struct net *net, struct nlm_file *file, struct nlm_lock *lock)
 	lock->fl.fl_type = F_UNLCK;
 	lock->fl.fl_file = file->f_file[O_RDONLY];
 	if (lock->fl.fl_file)
-		error = vfs_lock_file(lock->fl.fl_file, F_SETLK,
-					&lock->fl, NULL);
+		error = vfs_lock_file(F_SETLK, &lock->fl, NULL);
 	lock->fl.fl_file = file->f_file[O_WRONLY];
 	if (lock->fl.fl_file)
-		error |= vfs_lock_file(lock->fl.fl_file, F_SETLK,
-					&lock->fl, NULL);
+		error |= vfs_lock_file(F_SETLK, &lock->fl, NULL);
 
 	return (error < 0)? nlm_lck_denied_nolocks : nlm_granted;
 }
@@ -845,7 +841,8 @@ nlmsvc_grant_blocked(struct nlm_block *block)
 	fl_start = lock->fl.fl_start;
 	fl_end = lock->fl.fl_end;
 	mode = lock_to_openmode(&lock->fl);
-	error = vfs_lock_file(file->f_file[mode], F_SETLK, &lock->fl, NULL);
+	WARN_ON_ONCE(lock->fl.fl_file != file->f_file[mode]);
+	error = vfs_lock_file(F_SETLK, &lock->fl, NULL);
 	lock->fl.fl_flags &= ~FL_SLEEP;
 	lock->fl.fl_start = fl_start;
 	lock->fl.fl_end = fl_end;
diff --git a/fs/lockd/svcsubs.c b/fs/lockd/svcsubs.c
index 3515f17eaf3f..33842d67daa7 100644
--- a/fs/lockd/svcsubs.c
+++ b/fs/lockd/svcsubs.c
@@ -189,10 +189,10 @@ static int nlm_unlock_files(struct nlm_file *file, const struct file_lock *fl)
 	lock.fl_flags = FL_POSIX;
 
 	lock.fl_file = file->f_file[O_RDONLY];
-	if (lock.fl_file && vfs_lock_file(lock.fl_file, F_SETLK, &lock, NULL))
+	if (lock.fl_file && vfs_lock_file(F_SETLK, &lock, NULL))
 		goto out_err;
 	lock.fl_file = file->f_file[O_WRONLY];
-	if (lock.fl_file && vfs_lock_file(lock.fl_file, F_SETLK, &lock, NULL))
+	if (lock.fl_file && vfs_lock_file(F_SETLK, &lock, NULL))
 		goto out_err;
 	return 0;
 out_err:
diff --git a/fs/locks.c b/fs/locks.c
index b429d614316b..a38845633f73 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -2263,7 +2263,6 @@ int fcntl_getlk(struct file *filp, unsigned int cmd, struct flock *flock)
 
 /**
  * vfs_lock_file - file byte range lock
- * @filp: The file to apply the lock to
  * @cmd: type of locking operation (F_SETLK, F_GETLK, etc.)
  * @fl: The lock to be applied
  * @conf: Place to return a copy of the conflicting lock, if found.
@@ -2294,13 +2293,13 @@ int fcntl_getlk(struct file *filp, unsigned int cmd, struct flock *flock)
  * ->lm_grant() before returning to the caller with a FILE_LOCK_DEFERRED
  * return code.
  */
-int vfs_lock_file(struct file *filp, unsigned int cmd, struct file_lock *fl, struct file_lock *conf)
+int vfs_lock_file(unsigned int cmd, struct file_lock *fl, struct file_lock *conf)
 {
-	WARN_ON_ONCE(filp != fl->fl_file);
+	struct file *filp = fl->fl_file;
+
 	if (filp->f_op->lock)
 		return filp->f_op->lock(filp, cmd, fl);
-	else
-		return posix_lock_file(filp, fl, conf);
+	return posix_lock_file(filp, fl, conf);
 }
 EXPORT_SYMBOL_GPL(vfs_lock_file);
 
@@ -2314,7 +2313,7 @@ static int do_lock_file_wait(struct file *filp, unsigned int cmd,
 		return error;
 
 	for (;;) {
-		error = vfs_lock_file(filp, cmd, fl, NULL);
+		error = vfs_lock_file(cmd, fl, NULL);
 		if (error != FILE_LOCK_DEFERRED)
 			break;
 		error = wait_event_interruptible(fl->fl_wait,
@@ -2578,7 +2577,7 @@ void locks_remove_posix(struct file *filp, fl_owner_t owner)
 	lock.fl_ops = NULL;
 	lock.fl_lmops = NULL;
 
-	error = vfs_lock_file(filp, F_SETLK, &lock, NULL);
+	error = vfs_lock_file(F_SETLK, &lock, NULL);
 
 	if (lock.fl_ops && lock.fl_ops->fl_release_private)
 		lock.fl_ops->fl_release_private(&lock);
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 836bd825ca4a..eab75ba9f44d 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -7448,7 +7448,7 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		spin_unlock(&nn->blocked_locks_lock);
 	}
 
-	err = vfs_lock_file(nf->nf_file, F_SETLK, file_lock, conflock);
+	err = vfs_lock_file(F_SETLK, file_lock, conflock);
 	switch (err) {
 	case 0: /* success! */
 		nfs4_inc_and_copy_stateid(&lock->lk_resp_stateid, &lock_stp->st_stid);
@@ -7670,7 +7670,7 @@ nfsd4_locku(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 						locku->lu_length);
 	nfs4_transform_lock_offset(file_lock);
 
-	err = vfs_lock_file(nf->nf_file, F_SETLK, file_lock, NULL);
+	err = vfs_lock_file(F_SETLK, file_lock, NULL);
 	if (err) {
 		dprintk("NFSD: nfs4_locku: vfs_lock_file failed!\n");
 		goto out_nfserr;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index e4d0f1fa7f9f..57b6eed9a44d 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1168,7 +1168,7 @@ extern void posix_test_lock(struct file *, struct file_lock *);
 extern int posix_lock_file(struct file *, struct file_lock *, struct file_lock *);
 extern int locks_delete_block(struct file_lock *);
 extern int vfs_test_lock(struct file *, struct file_lock *);
-extern int vfs_lock_file(struct file *, unsigned int, struct file_lock *, struct file_lock *);
+extern int vfs_lock_file(unsigned int, struct file_lock *, struct file_lock *);
 extern int vfs_cancel_lock(struct file *filp, struct file_lock *fl);
 bool vfs_file_has_locks(struct file *file);
 extern int locks_lock_inode_wait(struct inode *inode, struct file_lock *fl);
@@ -1274,8 +1274,8 @@ static inline int vfs_test_lock(struct file *filp, struct file_lock *fl)
 	return 0;
 }
 
-static inline int vfs_lock_file(struct file *filp, unsigned int cmd,
-				struct file_lock *fl, struct file_lock *conf)
+static inline int vfs_lock_file(unsigned int cmd, struct file_lock *fl,
+				struct file_lock *conf)
 {
 	return -ENOLCK;
 }
-- 
2.38.1

[PATCH 2/3] filelock: remove redundant filp argument from vfs_test_lock

[Jeff Layton]

struct file_lock already has a fl_file field that must be populated, so
the @filp argument to this function is redundant. Remove it and use
fl_file instead.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/lockd/svclock.c  |  4 +---
 fs/locks.c          | 10 +++++-----
 fs/nfsd/nfs4state.c |  2 +-
 include/linux/fs.h  |  4 ++--
 4 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c
index c965783b71a6..21ee6b1c4d9e 100644
--- a/fs/lockd/svclock.c
+++ b/fs/lockd/svclock.c
@@ -586,7 +586,6 @@ nlmsvc_testlock(struct svc_rqst *rqstp, struct nlm_file *file,
 		struct nlm_lock *conflock, struct nlm_cookie *cookie)
 {
 	int			error;
-	int			mode;
 	__be32			ret;
 
 	dprintk("lockd: nlmsvc_testlock(%s/%ld, ty=%d, %Ld-%Ld)\n",
@@ -601,8 +600,7 @@ nlmsvc_testlock(struct svc_rqst *rqstp, struct nlm_file *file,
 		goto out;
 	}
 
-	mode = lock_to_openmode(&lock->fl);
-	error = vfs_test_lock(file->f_file[mode], &lock->fl);
+	error = vfs_test_lock(&lock->fl);
 	if (error) {
 		/* We can't currently deal with deferred test requests */
 		if (error == FILE_LOCK_DEFERRED)
diff --git a/fs/locks.c b/fs/locks.c
index a38845633f73..0bc1808f7d98 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -2138,15 +2138,15 @@ SYSCALL_DEFINE2(flock, unsigned int, fd, unsigned int, cmd)
 
 /**
  * vfs_test_lock - test file byte range lock
- * @filp: The file to test lock for
  * @fl: The lock to test; also used to hold result
  *
  * Returns -ERRNO on failure.  Indicates presence of conflicting lock by
  * setting conf->fl_type to something other than F_UNLCK.
  */
-int vfs_test_lock(struct file *filp, struct file_lock *fl)
+int vfs_test_lock(struct file_lock *fl)
 {
-	WARN_ON_ONCE(filp != fl->fl_file);
+	struct file *filp = fl->fl_file;
+
 	if (filp->f_op->lock)
 		return filp->f_op->lock(filp, F_GETLK, fl);
 	posix_test_lock(filp, fl);
@@ -2246,7 +2246,7 @@ int fcntl_getlk(struct file *filp, unsigned int cmd, struct flock *flock)
 		fl->fl_owner = filp;
 	}
 
-	error = vfs_test_lock(filp, fl);
+	error = vfs_test_lock(fl);
 	if (error)
 		goto out;
 
@@ -2453,7 +2453,7 @@ int fcntl_getlk64(struct file *filp, unsigned int cmd, struct flock64 *flock)
 		fl->fl_owner = filp;
 	}
 
-	error = vfs_test_lock(filp, fl);
+	error = vfs_test_lock(fl);
 	if (error)
 		goto out;
 
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index eab75ba9f44d..beec9da50016 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -7537,7 +7537,7 @@ static __be32 nfsd_test_lock(struct svc_rqst *rqstp, struct svc_fh *fhp, struct
 	if (err)
 		goto out;
 	lock->fl_file = nf->nf_file;
-	err = nfserrno(vfs_test_lock(nf->nf_file, lock));
+	err = nfserrno(vfs_test_lock(lock));
 	lock->fl_file = NULL;
 out:
 	inode_unlock(inode);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 57b6eed9a44d..507fa1a61bb5 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1167,7 +1167,7 @@ extern void locks_release_private(struct file_lock *);
 extern void posix_test_lock(struct file *, struct file_lock *);
 extern int posix_lock_file(struct file *, struct file_lock *, struct file_lock *);
 extern int locks_delete_block(struct file_lock *);
-extern int vfs_test_lock(struct file *, struct file_lock *);
+extern int vfs_test_lock(struct file_lock *);
 extern int vfs_lock_file(unsigned int, struct file_lock *, struct file_lock *);
 extern int vfs_cancel_lock(struct file *filp, struct file_lock *fl);
 bool vfs_file_has_locks(struct file *file);
@@ -1269,7 +1269,7 @@ static inline int locks_delete_block(struct file_lock *waiter)
 	return -ENOENT;
 }
 
-static inline int vfs_test_lock(struct file *filp, struct file_lock *fl)
+static inline int vfs_test_lock(struct file_lock *fl)
 {
 	return 0;
 }
-- 
2.38.1

[PATCH 3/3] filelock: remove redundant filp arg from vfs_cancel_lock

[Jeff Layton]

struct file_lock already has a fl_file field that must be populated, so
the @filp argument to this function is redundant. Remove it and use
fl_file instead.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/lockd/svclock.c | 4 +---
 fs/locks.c         | 6 +++---
 include/linux/fs.h | 4 ++--
 3 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c
index 21ee6b1c4d9e..2bced428b078 100644
--- a/fs/lockd/svclock.c
+++ b/fs/lockd/svclock.c
@@ -677,7 +677,6 @@ nlmsvc_cancel_blocked(struct net *net, struct nlm_file *file, struct nlm_lock *l
 {
 	struct nlm_block	*block;
 	int status = 0;
-	int mode;
 
 	dprintk("lockd: nlmsvc_cancel(%s/%ld, pi=%d, %Ld-%Ld)\n",
 				nlmsvc_file_inode(file)->i_sb->s_id,
@@ -695,8 +694,7 @@ nlmsvc_cancel_blocked(struct net *net, struct nlm_file *file, struct nlm_lock *l
 	if (block != NULL) {
 		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);
+		vfs_cancel_lock(fl);
 		status = nlmsvc_unlink_block(block);
 		nlmsvc_release_block(block);
 	}
diff --git a/fs/locks.c b/fs/locks.c
index 0bc1808f7d98..64eeb4002bbb 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -2657,14 +2657,14 @@ void locks_remove_file(struct file *filp)
 
 /**
  * vfs_cancel_lock - file byte range unblock lock
- * @filp: The file to apply the unblock to
  * @fl: The lock to be unblocked
  *
  * Used by lock managers to cancel blocked requests
  */
-int vfs_cancel_lock(struct file *filp, struct file_lock *fl)
+int vfs_cancel_lock(struct file_lock *fl)
 {
-	WARN_ON_ONCE(filp != fl->fl_file);
+	struct file *filp = fl->fl_file;
+
 	if (filp->f_op->lock)
 		return filp->f_op->lock(filp, F_CANCELLK, fl);
 	return 0;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 507fa1a61bb5..d5da4c448cd8 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1169,7 +1169,7 @@ extern int posix_lock_file(struct file *, struct file_lock *, struct file_lock *
 extern int locks_delete_block(struct file_lock *);
 extern int vfs_test_lock(struct file_lock *);
 extern int vfs_lock_file(unsigned int, struct file_lock *, struct file_lock *);
-extern int vfs_cancel_lock(struct file *filp, struct file_lock *fl);
+extern int vfs_cancel_lock(struct file_lock *fl);
 bool vfs_file_has_locks(struct file *file);
 extern int locks_lock_inode_wait(struct inode *inode, struct file_lock *fl);
 extern int __break_lease(struct inode *inode, unsigned int flags, unsigned int type);
@@ -1280,7 +1280,7 @@ static inline int vfs_lock_file(unsigned int cmd, struct file_lock *fl,
 	return -ENOLCK;
 }
 
-static inline int vfs_cancel_lock(struct file *filp, struct file_lock *fl)
+static inline int vfs_cancel_lock(struct file_lock *fl)
 {
 	return 0;
 }
-- 
2.38.1

Re: [PATCH 0/3] filelock: remove redundant filp arguments from API

[Chuck Lever III]

> On Nov 14, 2022, at 10:02 AM, Jeff Layton <jlayton@kernel.org> wrote:
> 
> Some of the exported functions in fs/locks.c take both a struct file
> argument and a struct file_lock. struct file_lock has a dedicated field
> to record which file it was set on (fl_file). This is redundant, and
> there have been some cases where the two didn't match [1], leading to
> bugs.

Hi Jeff, doesn't the same argument apply to f_ops->lock ? Do you
have a plan for updating that API as well?


> This patchset is intended to remove this ambiguity by eliminating the
> separate struct file argument from vfs_lock_file, vfs_test_lock and
> vfs_cancel_lock.
> 
> Most callers are easy to vet to ensure that they set this correctly, but
> lockd had a few places where it wasn't doing the right thing. This
> series depends on the lockd patches I sent late last week [2].
> 
> I'm targeting this series for v6.3. I'll plan to get it into linux-next
> soon unless there are objections.
> 
> [1]: https://bugzilla.kernel.org/show_bug.cgi?id=216582
> [2]: https://lore.kernel.org/linux-nfs/20221111215538.356543-1-jlayton@kernel.org/T/#t
> 
> Jeff Layton (3):
>  filelock: remove redundant filp argument from vfs_lock_file
>  filelock: remove redundant filp argument from vfs_test_lock
>  filelock: remove redundant filp arg from vfs_cancel_lock
> 
> fs/ksmbd/smb2pdu.c  |  4 ++--
> fs/lockd/svclock.c  | 21 +++++++--------------
> fs/lockd/svcsubs.c  |  4 ++--
> fs/locks.c          | 29 ++++++++++++++---------------
> fs/nfsd/nfs4state.c |  6 +++---
> include/linux/fs.h  | 14 +++++++-------
> 6 files changed, 35 insertions(+), 43 deletions(-)
> 
> -- 
> 2.38.1
> 

--
Chuck Lever

Re: [PATCH 0/3] filelock: remove redundant filp arguments from API

[Jeff Layton]

On Mon, 2022-11-14 at 15:08 +0000, Chuck Lever III wrote:
> 
> > On Nov 14, 2022, at 10:02 AM, Jeff Layton <jlayton@kernel.org> wrote:
> > 
> > Some of the exported functions in fs/locks.c take both a struct file
> > argument and a struct file_lock. struct file_lock has a dedicated field
> > to record which file it was set on (fl_file). This is redundant, and
> > there have been some cases where the two didn't match [1], leading to
> > bugs.
> 
> Hi Jeff, doesn't the same argument apply to f_ops->lock ? Do you
> have a plan for updating that API as well?
> 
> 

It does apply to fops->lock. I don't have a real plan as of yet. I
figure we'll get this set in first and then we can look at changing that
API as well.

> > This patchset is intended to remove this ambiguity by eliminating the
> > separate struct file argument from vfs_lock_file, vfs_test_lock and
> > vfs_cancel_lock.
> > 
> > Most callers are easy to vet to ensure that they set this correctly, but
> > lockd had a few places where it wasn't doing the right thing. This
> > series depends on the lockd patches I sent late last week [2].
> > 
> > I'm targeting this series for v6.3. I'll plan to get it into linux-next
> > soon unless there are objections.
> > 
> > [1]: https://bugzilla.kernel.org/show_bug.cgi?id=216582
> > [2]: https://lore.kernel.org/linux-nfs/20221111215538.356543-1-jlayton@kernel.org/T/#t
> > 
> > Jeff Layton (3):
> >  filelock: remove redundant filp argument from vfs_lock_file
> >  filelock: remove redundant filp argument from vfs_test_lock
> >  filelock: remove redundant filp arg from vfs_cancel_lock
> > 
> > fs/ksmbd/smb2pdu.c  |  4 ++--
> > fs/lockd/svclock.c  | 21 +++++++--------------
> > fs/lockd/svcsubs.c  |  4 ++--
> > fs/locks.c          | 29 ++++++++++++++---------------
> > fs/nfsd/nfs4state.c |  6 +++---
> > include/linux/fs.h  | 14 +++++++-------
> > 6 files changed, 35 insertions(+), 43 deletions(-)
> > 
> > -- 
> > 2.38.1
> > 
> 
> --
> Chuck Lever
> 
> 
> 

-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH 1/3] filelock: remove redundant filp argument from vfs_lock_file

[Christoph Hellwig]

On Mon, Nov 14, 2022 at 10:02:38AM -0500, Jeff Layton wrote:
> -int vfs_lock_file(struct file *filp, unsigned int cmd, struct file_lock *fl, struct file_lock *conf)
> +int vfs_lock_file(unsigned int cmd, struct file_lock *fl, struct file_lock *conf)

I'd pass fl as the first argument for a saner argument order here.
Also can you please break the line at 80 characters?  The previous
version is insanely unreadable, and the new one just slightly less
so.

> +extern int vfs_lock_file(unsigned int, struct file_lock *, struct file_lock *);

And please drop the pointless extern here.

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 2/3] filelock: remove redundant filp argument from vfs_test_lock

[Christoph Hellwig]

Same nitpck about the extern as for the last one, otherwise:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 3/3] filelock: remove redundant filp arg from vfs_cancel_lock

[Christoph Hellwig]

Same nitpick once more :)

Otherwise:

Reviewed-by: Christoph Hellwig <hch@lst.de>