On Thu, 2024-01-25 at 09:57 -0500, Chuck Lever wrote: > On Thu, Jan 25, 2024 at 05:42:41AM -0500, Jeff Layton wrote: > > Long ago, file locks used to hang off of a singly-linked list in struct > > inode. Because of this, when leases were added, they were added to the > > same list and so they had to be tracked using the same sort of > > structure. > > > > Several years ago, we added struct file_lock_context, which allowed us > > to use separate lists to track different types of file locks. Given > > that, leases no longer need to be tracked using struct file_lock. > > > > That said, a lot of the underlying infrastructure _is_ the same between > > file leases and locks, so we can't completely separate everything. > > > > This patchset first splits a group of fields used by both file locks and > > leases into a new struct file_lock_core, that is then embedded in struct > > file_lock. Coccinelle was then used to convert a lot of the callers to > > deal with the move, with the remaining 25% or so converted by hand. > > > > It then converts several internal functions in fs/locks.c to work > > with struct file_lock_core. Lastly, struct file_lock is split into > > struct file_lock and file_lease, and the lease-related APIs converted to > > take struct file_lease. > > > > After the first few patches (which I left split up for easier review), > > the set should be bisectable. I'll plan to squash the first few > > together to make sure the resulting set is bisectable before merge. > > > > Finally, I left the coccinelle scripts I used in tree. I had heard it > > was preferable to merge those along with the patches that they > > generate, but I wasn't sure where they go. I can either move those to a > > more appropriate location or we can just drop that commit if it's not > > needed. > > > > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx> > > v2 looks nicer. > > I would add a few list handling primitives, as I see enough > instances of list_for_each_entry, list_for_each_entry_safe, > list_first_entry, and list_first_entry_or_null on fl_core.flc_list > to make it worth having those. > > Also, there doesn't seem to be benefit for API consumers to have to > understand the internal structure of struct file_lock/lease to reach > into fl_core. Having accessor functions for common fields like > fl_type and fl_flags could be cleaner. > That is a good suggestion. I had considered it before and figured "why bother", but I think that would make things simpler. I'll plan to do a v3 that has more helpers. Possibly we can just convert some of the subsystems ahead of time and avoid some churn. Stay tuned... > For the series: > > Reviewed-by: Chuck Lever <chuck.lever@xxxxxxxxxx> > > For the nfsd and lockd parts: > > Acked-by: Chuck Lever <chuck.lever@xxxxxxxxxx> > > > > --- > > Changes in v2: > > - renamed file_lock_core fields to have "flc_" prefix > > - used macros to more easily do the change piecemeal > > - broke up patches into per-subsystem ones > > - Link to v1: https://lore.kernel.org/r/20240116-flsplit-v1-0-c9d0f4370a5d@xxxxxxxxxx > > > > --- > > Jeff Layton (41): > > filelock: rename some fields in tracepoints > > filelock: rename fl_pid variable in lock_get_status > > dlm: rename fl_flags variable in dlm_posix_unlock > > nfs: rename fl_flags variable in nfs4_proc_unlck > > nfsd: rename fl_type and fl_flags variables in nfsd4_lock > > lockd: rename fl_flags and fl_type variables in nlmclnt_lock > > 9p: rename fl_type variable in v9fs_file_do_lock > > afs: rename fl_type variable in afs_next_locker > > filelock: drop the IS_* macros > > filelock: split common fields into struct file_lock_core > > filelock: add coccinelle scripts to move fields to struct file_lock_core > > filelock: have fs/locks.c deal with file_lock_core directly > > filelock: convert some internal functions to use file_lock_core instead > > filelock: convert more internal functions to use file_lock_core > > filelock: make posix_same_owner take file_lock_core pointers > > filelock: convert posix_owner_key to take file_lock_core arg > > filelock: make locks_{insert,delete}_global_locks take file_lock_core arg > > filelock: convert locks_{insert,delete}_global_blocked > > filelock: make __locks_delete_block and __locks_wake_up_blocks take file_lock_core > > filelock: convert __locks_insert_block, conflict and deadlock checks to use file_lock_core > > filelock: convert fl_blocker to file_lock_core > > filelock: clean up locks_delete_block internals > > filelock: reorganize locks_delete_block and __locks_insert_block > > filelock: make assign_type helper take a file_lock_core pointer > > filelock: convert locks_wake_up_blocks to take a file_lock_core pointer > > filelock: convert locks_insert_lock_ctx and locks_delete_lock_ctx > > filelock: convert locks_translate_pid to take file_lock_core > > filelock: convert seqfile handling to use file_lock_core > > 9p: adapt to breakup of struct file_lock > > afs: adapt to breakup of struct file_lock > > ceph: adapt to breakup of struct file_lock > > dlm: adapt to breakup of struct file_lock > > gfs2: adapt to breakup of struct file_lock > > lockd: adapt to breakup of struct file_lock > > nfs: adapt to breakup of struct file_lock > > nfsd: adapt to breakup of struct file_lock > > ocfs2: adapt to breakup of struct file_lock > > smb/client: adapt to breakup of struct file_lock > > smb/server: adapt to breakup of struct file_lock > > filelock: remove temporary compatability macros > > filelock: split leases out of struct file_lock > > > > cocci/filelock.cocci | 88 +++++ > > cocci/nlm.cocci | 81 ++++ > > fs/9p/vfs_file.c | 40 +- > > fs/afs/flock.c | 59 +-- > > fs/ceph/locks.c | 74 ++-- > > fs/dlm/plock.c | 44 +-- > > fs/gfs2/file.c | 16 +- > > fs/libfs.c | 2 +- > > fs/lockd/clnt4xdr.c | 14 +- > > fs/lockd/clntlock.c | 2 +- > > fs/lockd/clntproc.c | 65 +-- > > fs/lockd/clntxdr.c | 14 +- > > fs/lockd/svc4proc.c | 10 +- > > fs/lockd/svclock.c | 64 +-- > > fs/lockd/svcproc.c | 10 +- > > fs/lockd/svcsubs.c | 24 +- > > fs/lockd/xdr.c | 14 +- > > fs/lockd/xdr4.c | 14 +- > > fs/locks.c | 848 ++++++++++++++++++++++------------------ > > fs/nfs/delegation.c | 4 +- > > fs/nfs/file.c | 22 +- > > fs/nfs/nfs3proc.c | 2 +- > > fs/nfs/nfs4_fs.h | 2 +- > > fs/nfs/nfs4file.c | 2 +- > > fs/nfs/nfs4proc.c | 39 +- > > fs/nfs/nfs4state.c | 22 +- > > fs/nfs/nfs4trace.h | 4 +- > > fs/nfs/nfs4xdr.c | 8 +- > > fs/nfs/write.c | 8 +- > > fs/nfsd/filecache.c | 4 +- > > fs/nfsd/nfs4callback.c | 2 +- > > fs/nfsd/nfs4layouts.c | 34 +- > > fs/nfsd/nfs4state.c | 118 +++--- > > fs/ocfs2/locks.c | 12 +- > > fs/ocfs2/stack_user.c | 2 +- > > fs/open.c | 2 +- > > fs/posix_acl.c | 4 +- > > fs/smb/client/cifsfs.c | 2 +- > > fs/smb/client/cifssmb.c | 8 +- > > fs/smb/client/file.c | 76 ++-- > > fs/smb/client/smb2file.c | 2 +- > > fs/smb/server/smb2pdu.c | 44 +-- > > fs/smb/server/vfs.c | 14 +- > > include/linux/filelock.h | 80 ++-- > > include/linux/fs.h | 5 +- > > include/linux/lockd/lockd.h | 8 +- > > include/linux/lockd/xdr.h | 2 +- > > include/trace/events/afs.h | 4 +- > > include/trace/events/filelock.h | 102 ++--- > > 49 files changed, 1198 insertions(+), 923 deletions(-) > > --- > > base-commit: 615d300648869c774bd1fe54b4627bb0c20faed4 > > change-id: 20240116-flsplit-bdb46824db68 > > > > Best regards, > > -- > > Jeff Layton <jlayton@xxxxxxxxxx> > > > -- Jeff Layton <jlayton@xxxxxxxxxx>
