diff mbox series

[v3,04/47] filelock: add some new helper functions

Message ID 20240131-flsplit-v3-4-c6129007ee8d@kernel.org (mailing list archive)
State New, archived
Headers show
Series filelock: split file leases out of struct file_lock | expand

Commit Message

Jeff Layton Jan. 31, 2024, 11:01 p.m. UTC 
In later patches we're going to embed some common fields into a new
structure inside struct file_lock. Smooth the transition by adding some
new helper functions, and converting the core file locking code to use
them.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/locks.c               | 18 +++++++++---------
 include/linux/filelock.h | 23 +++++++++++++++++++++++
 2 files changed, 32 insertions(+), 9 deletions(-)

Comments

Christian Brauner Feb. 5, 2024, 11:36 a.m. UTC | #1 
> diff --git a/include/linux/filelock.h b/include/linux/filelock.h
> index 085ff6ba0653..a814664b1053 100644
> --- a/include/linux/filelock.h
> +++ b/include/linux/filelock.h
> @@ -147,6 +147,29 @@ int fcntl_setlk64(unsigned int, struct file *, unsigned int,
>  int fcntl_setlease(unsigned int fd, struct file *filp, int arg);
>  int fcntl_getlease(struct file *filp);
>  
> +static inline bool lock_is_unlock(struct file_lock *fl)
> +{
> +	return fl->fl_type == F_UNLCK;
> +}
> +
> +static inline bool lock_is_read(struct file_lock *fl)
> +{
> +	return fl->fl_type == F_RDLCK;
> +}
> +
> +static inline bool lock_is_write(struct file_lock *fl)
> +{
> +	return fl->fl_type == F_WRLCK;
> +}
> +
> +static inline void locks_wake_up(struct file_lock *fl)
> +{
> +	wake_up(&fl->fl_wait);
> +}
> +
> +/* for walking lists of file_locks linked by fl_list */
> +#define for_each_file_lock(_fl, _head)	list_for_each_entry(_fl, _head, fl_list)
> +

This causes a build warning for fs/ceph/ and fs/afs when
!CONFIG_FILE_LOCKING. I'm about to fold the following diff into this
patch. The diff looks a bit wonky but essentially I've moved
lock_is_unlock(), lock_is_{read,write}(), locks_wake_up() and
for_each_file_lock() out of the ifdef CONFIG_FILE_LOCKING:

diff --git a/include/linux/filelock.h b/include/linux/filelock.h
index a814664b1053..62be9c6b1e59 100644
--- a/include/linux/filelock.h
+++ b/include/linux/filelock.h
@@ -133,20 +133,6 @@ struct file_lock_context {
        struct list_head        flc_lease;
 };

-#ifdef CONFIG_FILE_LOCKING
-int fcntl_getlk(struct file *, unsigned int, struct flock *);
-int fcntl_setlk(unsigned int, struct file *, unsigned int,
-                       struct flock *);
-
-#if BITS_PER_LONG == 32
-int fcntl_getlk64(struct file *, unsigned int, struct flock64 *);
-int fcntl_setlk64(unsigned int, struct file *, unsigned int,
-                       struct flock64 *);
-#endif
-
-int fcntl_setlease(unsigned int fd, struct file *filp, int arg);
-int fcntl_getlease(struct file *filp);
-
 static inline bool lock_is_unlock(struct file_lock *fl)
 {
        return fl->fl_type == F_UNLCK;
@@ -170,6 +156,20 @@ static inline void locks_wake_up(struct file_lock *fl)
 /* for walking lists of file_locks linked by fl_list */
 #define for_each_file_lock(_fl, _head) list_for_each_entry(_fl, _head, fl_list)

+#ifdef CONFIG_FILE_LOCKING
+int fcntl_getlk(struct file *, unsigned int, struct flock *);
+int fcntl_setlk(unsigned int, struct file *, unsigned int,
+                       struct flock *);
+
+#if BITS_PER_LONG == 32
+int fcntl_getlk64(struct file *, unsigned int, struct flock64 *);
+int fcntl_setlk64(unsigned int, struct file *, unsigned int,
+                       struct flock64 *);
+#endif
+
+int fcntl_setlease(unsigned int fd, struct file *filp, int arg);
+int fcntl_getlease(struct file *filp);
+
 /* fs/locks.c */
 void locks_free_lock_context(struct inode *inode);
 void locks_free_lock(struct file_lock *fl);
Jeff Layton Feb. 5, 2024, 11:55 a.m. UTC | #2 
On Mon, 2024-02-05 at 12:36 +0100, Christian Brauner wrote:
> > diff --git a/include/linux/filelock.h b/include/linux/filelock.h
> > index 085ff6ba0653..a814664b1053 100644
> > --- a/include/linux/filelock.h
> > +++ b/include/linux/filelock.h
> > @@ -147,6 +147,29 @@ int fcntl_setlk64(unsigned int, struct file *, unsigned int,
> >  int fcntl_setlease(unsigned int fd, struct file *filp, int arg);
> >  int fcntl_getlease(struct file *filp);
> >  
> > 
> > 
> > 
> > 
> > 
> > 
> > 
> > +static inline bool lock_is_unlock(struct file_lock *fl)
> > +{
> > +	return fl->fl_type == F_UNLCK;
> > +}
> > +
> > +static inline bool lock_is_read(struct file_lock *fl)
> > +{
> > +	return fl->fl_type == F_RDLCK;
> > +}
> > +
> > +static inline bool lock_is_write(struct file_lock *fl)
> > +{
> > +	return fl->fl_type == F_WRLCK;
> > +}
> > +
> > +static inline void locks_wake_up(struct file_lock *fl)
> > +{
> > +	wake_up(&fl->fl_wait);
> > +}
> > +
> > +/* for walking lists of file_locks linked by fl_list */
> > +#define for_each_file_lock(_fl, _head)	list_for_each_entry(_fl, _head, fl_list)
> > +
> 
> This causes a build warning for fs/ceph/ and fs/afs when
> !CONFIG_FILE_LOCKING. I'm about to fold the following diff into this
> patch. The diff looks a bit wonky but essentially I've moved
> lock_is_unlock(), lock_is_{read,write}(), locks_wake_up() and
> for_each_file_lock() out of the ifdef CONFIG_FILE_LOCKING:
> 

I sent a patch for this problem yesterday. Did you not get it?


> diff --git a/include/linux/filelock.h b/include/linux/filelock.h
> index a814664b1053..62be9c6b1e59 100644
> --- a/include/linux/filelock.h
> +++ b/include/linux/filelock.h
> @@ -133,20 +133,6 @@ struct file_lock_context {
>         struct list_head        flc_lease;
>  };
> 
> -#ifdef CONFIG_FILE_LOCKING
> -int fcntl_getlk(struct file *, unsigned int, struct flock *);
> -int fcntl_setlk(unsigned int, struct file *, unsigned int,
> -                       struct flock *);
> -
> -#if BITS_PER_LONG == 32
> -int fcntl_getlk64(struct file *, unsigned int, struct flock64 *);
> -int fcntl_setlk64(unsigned int, struct file *, unsigned int,
> -                       struct flock64 *);
> -#endif
> -
> -int fcntl_setlease(unsigned int fd, struct file *filp, int arg);
> -int fcntl_getlease(struct file *filp);
> -
>  static inline bool lock_is_unlock(struct file_lock *fl)
>  {
>         return fl->fl_type == F_UNLCK;
> @@ -170,6 +156,20 @@ static inline void locks_wake_up(struct file_lock *fl)
>  /* for walking lists of file_locks linked by fl_list */
>  #define for_each_file_lock(_fl, _head) list_for_each_entry(_fl, _head, fl_list)
> 
> +#ifdef CONFIG_FILE_LOCKING
> +int fcntl_getlk(struct file *, unsigned int, struct flock *);
> +int fcntl_setlk(unsigned int, struct file *, unsigned int,
> +                       struct flock *);
> +
> +#if BITS_PER_LONG == 32
> +int fcntl_getlk64(struct file *, unsigned int, struct flock64 *);
> +int fcntl_setlk64(unsigned int, struct file *, unsigned int,
> +                       struct flock64 *);
> +#endif
> +
> +int fcntl_setlease(unsigned int fd, struct file *filp, int arg);
> +int fcntl_getlease(struct file *filp);
> +
>  /* fs/locks.c */
>  void locks_free_lock_context(struct inode *inode);
>  void locks_free_lock(struct file_lock *fl);
>
Christian Brauner Feb. 5, 2024, 11:57 a.m. UTC | #3 
On Mon, Feb 05, 2024 at 06:55:44AM -0500, Jeff Layton wrote:
> On Mon, 2024-02-05 at 12:36 +0100, Christian Brauner wrote:
> > > diff --git a/include/linux/filelock.h b/include/linux/filelock.h
> > > index 085ff6ba0653..a814664b1053 100644
> > > --- a/include/linux/filelock.h
> > > +++ b/include/linux/filelock.h
> > > @@ -147,6 +147,29 @@ int fcntl_setlk64(unsigned int, struct file *, unsigned int,
> > >  int fcntl_setlease(unsigned int fd, struct file *filp, int arg);
> > >  int fcntl_getlease(struct file *filp);
> > >  
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > +static inline bool lock_is_unlock(struct file_lock *fl)
> > > +{
> > > +	return fl->fl_type == F_UNLCK;
> > > +}
> > > +
> > > +static inline bool lock_is_read(struct file_lock *fl)
> > > +{
> > > +	return fl->fl_type == F_RDLCK;
> > > +}
> > > +
> > > +static inline bool lock_is_write(struct file_lock *fl)
> > > +{
> > > +	return fl->fl_type == F_WRLCK;
> > > +}
> > > +
> > > +static inline void locks_wake_up(struct file_lock *fl)
> > > +{
> > > +	wake_up(&fl->fl_wait);
> > > +}
> > > +
> > > +/* for walking lists of file_locks linked by fl_list */
> > > +#define for_each_file_lock(_fl, _head)	list_for_each_entry(_fl, _head, fl_list)
> > > +
> > 
> > This causes a build warning for fs/ceph/ and fs/afs when
> > !CONFIG_FILE_LOCKING. I'm about to fold the following diff into this
> > patch. The diff looks a bit wonky but essentially I've moved
> > lock_is_unlock(), lock_is_{read,write}(), locks_wake_up() and
> > for_each_file_lock() out of the ifdef CONFIG_FILE_LOCKING:
> > 
> 
> I sent a patch for this problem yesterday. Did you not get it?

Whoops, probably missed it on the trip back from fosdem.
I'll double check now.
Jeff Layton Feb. 5, 2024, 12:06 p.m. UTC | #4 
On Mon, 2024-02-05 at 12:57 +0100, Christian Brauner wrote:
> On Mon, Feb 05, 2024 at 06:55:44AM -0500, Jeff Layton wrote:
> > On Mon, 2024-02-05 at 12:36 +0100, Christian Brauner wrote:
> > > > diff --git a/include/linux/filelock.h b/include/linux/filelock.h
> > > > index 085ff6ba0653..a814664b1053 100644
> > > > --- a/include/linux/filelock.h
> > > > +++ b/include/linux/filelock.h
> > > > @@ -147,6 +147,29 @@ int fcntl_setlk64(unsigned int, struct file *, unsigned int,
> > > >  int fcntl_setlease(unsigned int fd, struct file *filp, int arg);
> > > >  int fcntl_getlease(struct file *filp);
> > > >  
> > > > 
> > > > 
> > > > 
> > > > 
> > > > 
> > > > 
> > > > 
> > > > +static inline bool lock_is_unlock(struct file_lock *fl)
> > > > +{
> > > > +	return fl->fl_type == F_UNLCK;
> > > > +}
> > > > +
> > > > +static inline bool lock_is_read(struct file_lock *fl)
> > > > +{
> > > > +	return fl->fl_type == F_RDLCK;
> > > > +}
> > > > +
> > > > +static inline bool lock_is_write(struct file_lock *fl)
> > > > +{
> > > > +	return fl->fl_type == F_WRLCK;
> > > > +}
> > > > +
> > > > +static inline void locks_wake_up(struct file_lock *fl)
> > > > +{
> > > > +	wake_up(&fl->fl_wait);
> > > > +}
> > > > +
> > > > +/* for walking lists of file_locks linked by fl_list */
> > > > +#define for_each_file_lock(_fl, _head)	list_for_each_entry(_fl, _head, fl_list)
> > > > +
> > > 
> > > This causes a build warning for fs/ceph/ and fs/afs when
> > > !CONFIG_FILE_LOCKING. I'm about to fold the following diff into this
> > > patch. The diff looks a bit wonky but essentially I've moved
> > > lock_is_unlock(), lock_is_{read,write}(), locks_wake_up() and
> > > for_each_file_lock() out of the ifdef CONFIG_FILE_LOCKING:
> > > 
> > 
> > I sent a patch for this problem yesterday. Did you not get it?
> 
> Whoops, probably missed it on the trip back from fosdem.
> I'll double check now.

No worries. If you choose to go with your version, you can add:

Reviewed-by: Jeff Layton <jlayton@kernel.org>
Christian Brauner Feb. 5, 2024, 12:19 p.m. UTC | #5 
On Mon, Feb 05, 2024 at 07:06:00AM -0500, Jeff Layton wrote:
> On Mon, 2024-02-05 at 12:57 +0100, Christian Brauner wrote:
> > On Mon, Feb 05, 2024 at 06:55:44AM -0500, Jeff Layton wrote:
> > > On Mon, 2024-02-05 at 12:36 +0100, Christian Brauner wrote:
> > > > > diff --git a/include/linux/filelock.h b/include/linux/filelock.h
> > > > > index 085ff6ba0653..a814664b1053 100644
> > > > > --- a/include/linux/filelock.h
> > > > > +++ b/include/linux/filelock.h
> > > > > @@ -147,6 +147,29 @@ int fcntl_setlk64(unsigned int, struct file *, unsigned int,
> > > > >  int fcntl_setlease(unsigned int fd, struct file *filp, int arg);
> > > > >  int fcntl_getlease(struct file *filp);
> > > > >  
> > > > > 
> > > > > 
> > > > > 
> > > > > 
> > > > > 
> > > > > 
> > > > > 
> > > > > +static inline bool lock_is_unlock(struct file_lock *fl)
> > > > > +{
> > > > > +	return fl->fl_type == F_UNLCK;
> > > > > +}
> > > > > +
> > > > > +static inline bool lock_is_read(struct file_lock *fl)
> > > > > +{
> > > > > +	return fl->fl_type == F_RDLCK;
> > > > > +}
> > > > > +
> > > > > +static inline bool lock_is_write(struct file_lock *fl)
> > > > > +{
> > > > > +	return fl->fl_type == F_WRLCK;
> > > > > +}
> > > > > +
> > > > > +static inline void locks_wake_up(struct file_lock *fl)
> > > > > +{
> > > > > +	wake_up(&fl->fl_wait);
> > > > > +}
> > > > > +
> > > > > +/* for walking lists of file_locks linked by fl_list */
> > > > > +#define for_each_file_lock(_fl, _head)	list_for_each_entry(_fl, _head, fl_list)
> > > > > +
> > > > 
> > > > This causes a build warning for fs/ceph/ and fs/afs when
> > > > !CONFIG_FILE_LOCKING. I'm about to fold the following diff into this
> > > > patch. The diff looks a bit wonky but essentially I've moved
> > > > lock_is_unlock(), lock_is_{read,write}(), locks_wake_up() and
> > > > for_each_file_lock() out of the ifdef CONFIG_FILE_LOCKING:
> > > > 
> > > 
> > > I sent a patch for this problem yesterday. Did you not get it?
> > 
> > Whoops, probably missed it on the trip back from fosdem.
> > I'll double check now.
> 
> No worries. If you choose to go with your version, you can add:

No, I took yours. :)
diff mbox series

Patch

diff --git a/fs/locks.c b/fs/locks.c
index 1eceaa56e47f..149070fd3b66 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -674,7 +674,7 @@  static void __locks_wake_up_blocks(struct file_lock *blocker)
 		if (waiter->fl_lmops && waiter->fl_lmops->lm_notify)
 			waiter->fl_lmops->lm_notify(waiter);
 		else
-			wake_up(&waiter->fl_wait);
+			locks_wake_up(waiter);
 
 		/*
 		 * The setting of fl_blocker to NULL marks the "done"
@@ -841,9 +841,9 @@  locks_delete_lock_ctx(struct file_lock *fl, struct list_head *dispose)
 static bool locks_conflict(struct file_lock *caller_fl,
 			   struct file_lock *sys_fl)
 {
-	if (sys_fl->fl_type == F_WRLCK)
+	if (lock_is_write(sys_fl))
 		return true;
-	if (caller_fl->fl_type == F_WRLCK)
+	if (lock_is_write(caller_fl))
 		return true;
 	return false;
 }
@@ -874,7 +874,7 @@  static bool posix_test_locks_conflict(struct file_lock *caller_fl,
 				      struct file_lock *sys_fl)
 {
 	/* F_UNLCK checks any locks on the same fd. */
-	if (caller_fl->fl_type == F_UNLCK) {
+	if (lock_is_unlock(caller_fl)) {
 		if (!posix_same_owner(caller_fl, sys_fl))
 			return false;
 		return locks_overlap(caller_fl, sys_fl);
@@ -1055,7 +1055,7 @@  static int flock_lock_inode(struct inode *inode, struct file_lock *request)
 		break;
 	}
 
-	if (request->fl_type == F_UNLCK) {
+	if (lock_is_unlock(request)) {
 		if ((request->fl_flags & FL_EXISTS) && !found)
 			error = -ENOENT;
 		goto out;
@@ -1107,7 +1107,7 @@  static int posix_lock_inode(struct inode *inode, struct file_lock *request,
 
 	ctx = locks_get_lock_context(inode, request->fl_type);
 	if (!ctx)
-		return (request->fl_type == F_UNLCK) ? 0 : -ENOMEM;
+		return lock_is_unlock(request) ? 0 : -ENOMEM;
 
 	/*
 	 * We may need two file_lock structures for this operation,
@@ -1228,7 +1228,7 @@  static int posix_lock_inode(struct inode *inode, struct file_lock *request,
 				continue;
 			if (fl->fl_start > request->fl_end)
 				break;
-			if (request->fl_type == F_UNLCK)
+			if (lock_is_unlock(request))
 				added = true;
 			if (fl->fl_start < request->fl_start)
 				left = fl;
@@ -1279,7 +1279,7 @@  static int posix_lock_inode(struct inode *inode, struct file_lock *request,
 
 	error = 0;
 	if (!added) {
-		if (request->fl_type == F_UNLCK) {
+		if (lock_is_unlock(request)) {
 			if (request->fl_flags & FL_EXISTS)
 				error = -ENOENT;
 			goto out;
@@ -1608,7 +1608,7 @@  void lease_get_mtime(struct inode *inode, struct timespec64 *time)
 		spin_lock(&ctx->flc_lock);
 		fl = list_first_entry_or_null(&ctx->flc_lease,
 					      struct file_lock, fl_list);
-		if (fl && (fl->fl_type == F_WRLCK))
+		if (fl && lock_is_write(fl))
 			has_lease = true;
 		spin_unlock(&ctx->flc_lock);
 	}
diff --git a/include/linux/filelock.h b/include/linux/filelock.h
index 085ff6ba0653..a814664b1053 100644
--- a/include/linux/filelock.h
+++ b/include/linux/filelock.h
@@ -147,6 +147,29 @@  int fcntl_setlk64(unsigned int, struct file *, unsigned int,
 int fcntl_setlease(unsigned int fd, struct file *filp, int arg);
 int fcntl_getlease(struct file *filp);
 
+static inline bool lock_is_unlock(struct file_lock *fl)
+{
+	return fl->fl_type == F_UNLCK;
+}
+
+static inline bool lock_is_read(struct file_lock *fl)
+{
+	return fl->fl_type == F_RDLCK;
+}
+
+static inline bool lock_is_write(struct file_lock *fl)
+{
+	return fl->fl_type == F_WRLCK;
+}
+
+static inline void locks_wake_up(struct file_lock *fl)
+{
+	wake_up(&fl->fl_wait);
+}
+
+/* for walking lists of file_locks linked by fl_list */
+#define for_each_file_lock(_fl, _head)	list_for_each_entry(_fl, _head, fl_list)
+
 /* fs/locks.c */
 void locks_free_lock_context(struct inode *inode);
 void locks_free_lock(struct file_lock *fl);