Received-SPF: pass (google.com: domain of linux-nfs+bounces-1705-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
MIME-Version: 1.0
From: "NeilBrown" <neilb@suse.de>
To: "Jeff Layton" <jlayton@kernel.org>
Cc: "Steven Rostedt" <rostedt@goodmis.org>,
 "Masami Hiramatsu" <mhiramat@kernel.org>,
 "Mathieu Desnoyers" <mathieu.desnoyers@efficios.com>,
 "Chuck Lever" <chuck.lever@oracle.com>,
 "Alexander Viro" <viro@zeniv.linux.org.uk>,
 "Christian Brauner" <brauner@kernel.org>, "Jan Kara" <jack@suse.cz>,
 "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>,
 "Andreas Gruenbacher" <agruenba@redhat.com>,
 "Olga Kornievskaia" <kolga@netapp.com>, "Dai Ngo" <Dai.Ngo@oracle.com>,
 "Tom Talpey" <tom@talpey.com>,
 "Trond Myklebust" <trond.myklebust@hammerspace.com>,
 "Anna Schumaker" <anna@kernel.org>, "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" <ronniesahlberg@gmail.com>,
 "Shyam Prasad N" <sprasad@microsoft.com>,
 "Namjae Jeon" <linkinjeon@kernel.org>,
 "Sergey Senozhatsky" <senozhatsky@chromium.org>,
 "Miklos Szeredi" <miklos@szeredi.hu>, linux-kernel@vger.kernel.org,
 linux-trace-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
 v9fs@lists.linux.dev, linux-afs@lists.infradead.org,
 ceph-devel@vger.kernel.org, gfs2@lists.linux.dev, linux-nfs@vger.kernel.org,
 ocfs2-devel@lists.linux.dev, linux-cifs@vger.kernel.org,
 "Jeff Layton" <jlayton@kernel.org>
Subject:
 Re: [PATCH v3 00/47] filelock: split file leases out of struct file_lock
In-reply-to: <20240131-flsplit-v3-0-c6129007ee8d@kernel.org>
References: <20240131-flsplit-v3-0-c6129007ee8d@kernel.org>
Date: Fri, 02 Feb 2024 09:49:29 +1100
Message-id: <170682776950.13976.8032076527553420784@noble.neil.brown.name>

On Thu, 01 Feb 2024, Jeff Layton wrote:
> I'm not sure this is much prettier than the last, but contracting
> "fl_core" to "c", as Neil suggested is a bit easier on the eyes.
>=20
> I also added a few small helpers and converted several users over to
> them. That reduces the size of the per-fs conversion patches later in
> the series. I played with some others too, but they were too awkward
> or not frequently used enough to make it worthwhile.
>=20
> Many thanks to Chuck and Neil for the earlier R-b's and comments. I've
> dropped those for now since this set is a bit different from the last.
>=20
> I'd like to get this into linux-next soon and we can see about merging
> it for v6.9, unless anyone has major objections.

For all patches:
  Reviewed-by: NeilBrown <neilb@suse.de>

Thanks Jeff - I think this is a good and useful change and while it
might not all be as pretty as I might like, I don't see any way to
improve it and think it is certainly good enough to merge.

I think the conversion from "fl_core" to "c" does work well enough.
I particularly like how the removal of the IS_* macros (patch 16) turned
out.  The inline expansion of IS_LEASE() in particular makes the code
clearer to me.

Thanks,
NeilBrown


>=20
> Thanks!
>=20
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
> Changes in v3:
> - Rename "flc_core" fields in file_lock and file_lease to "c"
> - new helpers: locks_wake_up, for_each_file_lock, and lock_is_{unlock,read,=
write}
> - Link to v2: https://lore.kernel.org/r/20240125-flsplit-v2-0-7485322b62c7@=
kernel.org
>=20
> 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
>=20
> ---
> Jeff Layton (47):
>       filelock: fl_pid field should be signed int
>       filelock: rename some fields in tracepoints
>       filelock: rename fl_pid variable in lock_get_status
>       filelock: add some new helper functions
>       9p: rename fl_type variable in v9fs_file_do_lock
>       afs: convert to using new filelock helpers
>       ceph: convert to using new filelock helpers
>       dlm: convert to using new filelock helpers
>       gfs2: convert to using new filelock helpers
>       lockd: convert to using new filelock helpers
>       nfs: convert to using new filelock helpers
>       nfsd: convert to using new filelock helpers
>       ocfs2: convert to using new filelock helpers
>       smb/client: convert to using new filelock helpers
>       smb/server: convert to using new filelock helpers
>       filelock: drop the IS_* macros
>       filelock: split common fields into struct file_lock_core
>       filelock: have fs/locks.c deal with file_lock_core directly
>       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 f=
ile_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 point=
er
>       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
>       fuse: 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 compatibility macros
>       filelock: split leases out of struct file_lock
>=20
>  fs/9p/vfs_file.c                |  40 +-
>  fs/afs/flock.c                  |  60 +--
>  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             |  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                      | 851 ++++++++++++++++++++++--------------=
----
>  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             | 120 +++---
>  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            |  78 ++--
>  fs/smb/client/smb2file.c        |   2 +-
>  fs/smb/server/smb2pdu.c         |  44 +--
>  fs/smb/server/vfs.c             |  14 +-
>  include/linux/filelock.h        | 103 +++--
>  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 ++---
>  48 files changed, 1064 insertions(+), 933 deletions(-)
> ---
> base-commit: e96efe9f69ebb12b38c722c159413fd6850b782c
> change-id: 20240116-flsplit-bdb46824db68
>=20
> Best regards,
> --=20
> Jeff Layton <jlayton@kernel.org>
>=20
>=20