| Message ID | 20240131-flsplit-v3-4-c6129007ee8d@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | filelock: split file leases out of struct file_lock | expand |
> 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);
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); >
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.
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>
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 --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);
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(-)