| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <ZbJ2zc3I3uBwF/RE@tissot.1015granger.net>
Date: Thu, 25 Jan 2024 09:57:17 -0500
From: Chuck Lever <chuck.lever@...cle.com>
To: Jeff Layton <jlayton@...nel.org>
Cc: Christian Brauner <brauner@...nel.org>,
Alexander Viro <viro@...iv.linux.org.uk>,
Eric Van Hensbergen <ericvh@...nel.org>,
Latchesar Ionkov <lucho@...kov.net>,
Dominique Martinet <asmadeus@...ewreck.org>,
Christian Schoenebeck <linux_oss@...debyte.com>,
David Howells <dhowells@...hat.com>,
Marc Dionne <marc.dionne@...istor.com>, Xiubo Li <xiubli@...hat.com>,
Ilya Dryomov <idryomov@...il.com>,
Alexander Aring <aahringo@...hat.com>,
David Teigland <teigland@...hat.com>,
Miklos Szeredi <miklos@...redi.hu>,
Andreas Gruenbacher <agruenba@...hat.com>,
Trond Myklebust <trond.myklebust@...merspace.com>,
Anna Schumaker <anna@...nel.org>, Neil Brown <neilb@...e.de>,
Olga Kornievskaia <kolga@...app.com>, Dai Ngo <Dai.Ngo@...cle.com>,
Tom Talpey <tom@...pey.com>, Jan Kara <jack@...e.cz>,
Mark Fasheh <mark@...heh.com>, Joel Becker <jlbec@...lplan.org>,
Joseph Qi <joseph.qi@...ux.alibaba.com>,
Steve French <sfrench@...ba.org>, Paulo Alcantara <pc@...guebit.com>,
Shyam Prasad N <sprasad@...rosoft.com>,
Namjae Jeon <linkinjeon@...nel.org>,
Sergey Senozhatsky <senozhatsky@...omium.org>,
Steven Rostedt <rostedt@...dmis.org>,
Masami Hiramatsu <mhiramat@...nel.org>,
Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
Ronnie Sahlberg <ronniesahlberg@...il.com>,
linux-kernel@...r.kernel.org, v9fs@...ts.linux.dev,
linux-afs@...ts.infradead.org, ceph-devel@...r.kernel.org,
gfs2@...ts.linux.dev, linux-fsdevel@...r.kernel.org,
linux-nfs@...r.kernel.org, ocfs2-devel@...ts.linux.dev,
linux-cifs@...r.kernel.org, linux-trace-kernel@...r.kernel.org
Subject: Re: [PATCH v2 00/41] filelock: split struct file_lock into file_lock
and file_lease structs
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@...nel.org>
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.
For the series:
Reviewed-by: Chuck Lever <chuck.lever@...cle.com>
For the nfsd and lockd parts:
Acked-by: Chuck Lever <chuck.lever@...cle.com>
> ---
> 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@kernel.org
>
> ---
> 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@...nel.org>
>
--
Chuck Lever
Powered by blists - more mailing lists