From: Jeff Layton <jlayton@kernel.org>
To: Chuck Lever <chuck.lever@oracle.com>
Cc: Christian Brauner <brauner@kernel.org>,
Alexander Viro <viro@zeniv.linux.org.uk>,
Eric Van Hensbergen <ericvh@kernel.org>,
Latchesar Ionkov <lucho@ionkov.net>,
Dominique Martinet <asmadeus@codewreck.org>,
Christian Schoenebeck <linux_oss@crudebyte.com>,
David Howells <dhowells@redhat.com>,
Marc Dionne <marc.dionne@auristor.com>,
Xiubo Li <xiubli@redhat.com>, Ilya Dryomov <idryomov@gmail.com>,
Alexander Aring <aahringo@redhat.com>,
David Teigland <teigland@redhat.com>,
Miklos Szeredi <miklos@szeredi.hu>,
Andreas Gruenbacher <agruenba@redhat.com>,
Trond Myklebust <trond.myklebust@hammerspace.com>,
Anna Schumaker <anna@kernel.org>, Neil Brown <neilb@suse.de>,
Olga Kornievskaia <kolga@netapp.com>,
Dai Ngo <Dai.Ngo@oracle.com>, Tom Talpey <tom@talpey.com>,
Jan Kara <jack@suse.cz>, Mark Fasheh <mark@fasheh.com>,
Joel Becker <jlbec@evilplan.org>,
Joseph Qi <joseph.qi@linux.alibaba.com>,
Steve French <sfrench@samba.org>,
Paulo Alcantara <pc@manguebit.com>,
Ronnie Sahlberg <lsahlber@redhat.com>,
Shyam Prasad N <sprasad@microsoft.com>,
Namjae Jeon <linkinjeon@kernel.org>,
Sergey Senozhatsky <senozhatsky@chromium.org>,
Steven Rostedt <rostedt@goodmis.org>,
Masami Hiramatsu <mhiramat@kernel.org>,
Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
linux-kernel@vger.kernel.org, v9fs@lists.linux.dev,
linux-afs@lists.infradead.org, ceph-devel@vger.kernel.org,
gfs2@lists.linux.dev, linux-fsdevel@vger.kernel.org,
linux-nfs@vger.kernel.org, ocfs2-devel@lists.linux.dev,
linux-cifs@vger.kernel.org, samba-technical@lists.samba.org,
linux-trace-kernel@vger.kernel.org
Subject: Re: [PATCH 00/20] filelock: split struct file_lock into file_lock and file_lease structs
Date: Wed, 17 Jan 2024 12:32:09 -0500 [thread overview]
Message-ID: <b065ef8abc701c2dc05448bb1d8016f75b6f9191.camel@kernel.org> (raw)
In-Reply-To: <ZafuXpR4Y8Y6HFFl@tissot.1015granger.net>
On Wed, 2024-01-17 at 10:12 -0500, Chuck Lever wrote:
> On Tue, Jan 16, 2024 at 02:45:56PM -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.
>
> Naive question: locks and leases are similar. Why do they need to be
> split apart? The cover letter doesn't address that, and I'm new
> enough at this that I don't have that context.
>
Leases and locks do have some similarities, but it's mostly the
internals (stuff like the blocker/waiter handling) where they are
similar. Superficially they are very different objects, and handling
them with the same struct is unintuitive.
So, for now this is just about cleaning up the lock and lease handling
APIs for better type safety and clarity. It's also nice to separate out
things like the kasync handling, which only applies to leases, as well
as splitting up the lock_manager_operations, which don't share any
operations between locks and leases.
Longer term, we're also considering adding things like directory
delegations, which may need to either expand struct file_lease, or add
a new variant (dir_deleg ?). I'd rather not add that complexity to
struct file_lock.
>
> > 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.
> >
> > I'd like to have this considered for inclusion in v6.9. Christian, would
> > you be amenable to shepherding this into mainline (assuming there are no
> > major objections, of course)?
> >
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> > Jeff Layton (20):
> > filelock: split common fields into struct file_lock_core
> > filelock: add coccinelle scripts to move fields to struct file_lock_core
> > filelock: the results of the coccinelle conversion
> > filelock: fixups after the coccinelle changes
> > 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: convert the IS_* macros to take file_lock_core
> > 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: split leases out of struct file_lock
> >
> > cocci/filelock.cocci | 81 +++++
> > cocci/filelock2.cocci | 6 +
> > cocci/nlm.cocci | 81 +++++
> > fs/9p/vfs_file.c | 38 +-
> > fs/afs/flock.c | 55 +--
> > fs/ceph/locks.c | 74 ++--
> > fs/dlm/plock.c | 44 +--
> > fs/fuse/file.c | 14 +-
> > fs/gfs2/file.c | 16 +-
> > fs/libfs.c | 2 +-
> > fs/lockd/clnt4xdr.c | 14 +-
> > fs/lockd/clntlock.c | 2 +-
> > fs/lockd/clntproc.c | 60 +--
> > 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 | 785 ++++++++++++++++++++++------------------
> > 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 | 6 +-
> > 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 | 98 ++---
> > fs/ocfs2/locks.c | 12 +-
> > fs/ocfs2/stack_user.c | 2 +-
> > fs/smb/client/cifsfs.c | 2 +-
> > fs/smb/client/cifssmb.c | 8 +-
> > fs/smb/client/file.c | 74 ++--
> > fs/smb/client/smb2file.c | 2 +-
> > fs/smb/server/smb2pdu.c | 44 +--
> > fs/smb/server/vfs.c | 14 +-
> > include/linux/filelock.h | 58 ++-
> > include/linux/fs.h | 5 +-
> > include/linux/lockd/lockd.h | 8 +-
> > include/trace/events/afs.h | 4 +-
> > include/trace/events/filelock.h | 54 +--
> > 48 files changed, 1119 insertions(+), 825 deletions(-)
> > ---
> > base-commit: 052d534373b7ed33712a63d5e17b2b6cdbce84fd
> > change-id: 20240116-flsplit-bdb46824db68
> >
> > Best regards,
> > --
> > Jeff Layton <jlayton@kernel.org>
> >
>
--
Jeff Layton <jlayton@kernel.org>
next prev parent reply other threads:[~2024-01-17 17:32 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-16 19:45 [PATCH 00/20] filelock: split struct file_lock into file_lock and file_lease structs Jeff Layton
2024-01-16 19:45 ` [PATCH 01/20] filelock: split common fields into struct file_lock_core Jeff Layton
2024-01-16 22:07 ` NeilBrown
2024-01-17 12:46 ` Jeff Layton
2024-01-16 19:45 ` [PATCH 02/20] filelock: add coccinelle scripts to move fields to " Jeff Layton
2024-01-16 19:45 ` [PATCH 03/20] filelock: the results of the coccinelle conversion Jeff Layton
2024-01-17 15:10 ` Chuck Lever
2024-01-16 19:46 ` [PATCH 04/20] filelock: fixups after the coccinelle changes Jeff Layton
2024-01-17 15:13 ` Chuck Lever
2024-01-16 19:46 ` [PATCH 05/20] filelock: convert some internal functions to use file_lock_core instead Jeff Layton
2024-01-16 19:46 ` [PATCH 06/20] filelock: convert more internal functions to use file_lock_core Jeff Layton
2024-01-16 19:46 ` [PATCH 07/20] filelock: make posix_same_owner take file_lock_core pointers Jeff Layton
2024-01-16 19:46 ` [PATCH 08/20] filelock: convert posix_owner_key to take file_lock_core arg Jeff Layton
2024-01-16 19:46 ` [PATCH 09/20] filelock: make locks_{insert,delete}_global_locks " Jeff Layton
2024-01-16 19:46 ` [PATCH 10/20] filelock: convert locks_{insert,delete}_global_blocked Jeff Layton
2024-01-16 19:46 ` [PATCH 11/20] filelock: convert the IS_* macros to take file_lock_core Jeff Layton
2024-01-16 22:16 ` NeilBrown
2024-01-17 12:32 ` Jeff Layton
2024-01-16 19:46 ` [PATCH 12/20] filelock: make __locks_delete_block and __locks_wake_up_blocks " Jeff Layton
2024-01-16 22:23 ` NeilBrown
2024-01-17 12:40 ` Jeff Layton
2024-01-16 19:46 ` [PATCH 13/20] filelock: convert __locks_insert_block, conflict and deadlock checks to use file_lock_core Jeff Layton
2024-01-16 22:32 ` NeilBrown
2024-01-17 12:42 ` Jeff Layton
2024-01-16 19:46 ` [PATCH 14/20] filelock: convert fl_blocker to file_lock_core Jeff Layton
2024-01-16 19:46 ` [PATCH 15/20] filelock: clean up locks_delete_block internals Jeff Layton
2024-01-16 19:46 ` [PATCH 16/20] filelock: reorganize locks_delete_block and __locks_insert_block Jeff Layton
2024-01-16 19:46 ` [PATCH 17/20] filelock: make assign_type helper take a file_lock_core pointer Jeff Layton
2024-01-16 19:46 ` [PATCH 18/20] filelock: convert locks_wake_up_blocks to " Jeff Layton
2024-01-16 19:46 ` [PATCH 19/20] filelock: convert locks_insert_lock_ctx and locks_delete_lock_ctx Jeff Layton
2024-01-16 19:46 ` [PATCH 20/20] filelock: split leases out of struct file_lock Jeff Layton
2024-01-16 22:44 ` NeilBrown
2024-01-17 12:45 ` Jeff Layton
2024-01-17 12:48 ` [PATCH 00/20] filelock: split struct file_lock into file_lock and file_lease structs Christian Brauner
2024-01-17 12:59 ` Jeff Layton
2024-01-17 13:25 ` [PATCH 02/20] filelock: add coccinelle scripts to move fields to struct file_lock_core David Howells
2024-01-17 13:40 ` Jeff Layton
2024-01-17 15:12 ` [PATCH 00/20] filelock: split struct file_lock into file_lock and file_lease structs Chuck Lever
2024-01-17 17:32 ` Jeff Layton [this message]
2024-01-17 18:59 ` NeilBrown
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=b065ef8abc701c2dc05448bb1d8016f75b6f9191.camel@kernel.org \
--to=jlayton@kernel.org \
--cc=Dai.Ngo@oracle.com \
--cc=aahringo@redhat.com \
--cc=agruenba@redhat.com \
--cc=anna@kernel.org \
--cc=asmadeus@codewreck.org \
--cc=brauner@kernel.org \
--cc=ceph-devel@vger.kernel.org \
--cc=chuck.lever@oracle.com \
--cc=dhowells@redhat.com \
--cc=ericvh@kernel.org \
--cc=gfs2@lists.linux.dev \
--cc=idryomov@gmail.com \
--cc=jack@suse.cz \
--cc=jlbec@evilplan.org \
--cc=joseph.qi@linux.alibaba.com \
--cc=kolga@netapp.com \
--cc=linkinjeon@kernel.org \
--cc=linux-afs@lists.infradead.org \
--cc=linux-cifs@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-nfs@vger.kernel.org \
--cc=linux-trace-kernel@vger.kernel.org \
--cc=linux_oss@crudebyte.com \
--cc=lsahlber@redhat.com \
--cc=lucho@ionkov.net \
--cc=marc.dionne@auristor.com \
--cc=mark@fasheh.com \
--cc=mathieu.desnoyers@efficios.com \
--cc=mhiramat@kernel.org \
--cc=miklos@szeredi.hu \
--cc=neilb@suse.de \
--cc=ocfs2-devel@lists.linux.dev \
--cc=pc@manguebit.com \
--cc=rostedt@goodmis.org \
--cc=samba-technical@lists.samba.org \
--cc=senozhatsky@chromium.org \
--cc=sfrench@samba.org \
--cc=sprasad@microsoft.com \
--cc=teigland@redhat.com \
--cc=tom@talpey.com \
--cc=trond.myklebust@hammerspace.com \
--cc=v9fs@lists.linux.dev \
--cc=viro@zeniv.linux.org.uk \
--cc=xiubli@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).