Re: [PATCH v3 00/38] vfs, nfsd: implement directory delegations

[Chuck Lever]

On 9/24/25 11:05 AM, Jeff Layton wrote:
> This patchset is an update to a patchset that I posted in early June
> this year [1]. This version should be basically feature-complete, with a
> few caveats.
> 
> NFSv4.1 adds a GET_DIR_DELEGATION operation, to allow clients
> to request a delegation on a directory. If the client holds a directory
> delegation, then it knows that nothing will change the dentries in it
> until it has been recalled (modulo the case where the client requests
> notifications of directory changes).
> 
> In 2023, Rick Macklem gave a talk at the NFS Bakeathon on his
> implementation of directory delegations for FreeBSD [2], and showed that
> it can greatly improve LOOKUP-heavy workloads. There is also some
> earlier work by CITI [3] that showed similar results. The SMB protocol
> also has a similar sort of construct, and they have also seen large
> performance improvements on certain workloads.
> 
> This version also starts with support for trivial directory delegations
> that support no notifications.  From there it adds VFS support for
> ignoring certain break_lease() events in directories. It then adds
> support for basic CB_NOTIFY calls (with names only). Next, support for
> sending attributes in the notifications is added.
> 
> I think that this version should be getting close to merge ready. Anna
> has graciously agreed to work on the client-side pieces for this. I've
> mostly been testing using pynfs tests (which I will submit soon).
> 
> The main limitation at this point is that callback requests are
> currently limited to a single page, so we can't send very many in a
> single CB_NOTIFY call. This will make it easy to "get into the weeds" if
> you're changing a directory quickly. The server will just recall the
> delegation in that case, so it's harmless even though it's not ideal.
> 
> If this approach looks acceptable I'll see if we can increase that
> limitation (it seems doable).
> 
> If anyone wishes to try this out, it's in the "dir-deleg" branch in my
> tree at kernel.org [4].
> 
> [1]: https://lore.kernel.org/linux-nfs/20250602-dir-deleg-v2-0-a7919700de86@xxxxxxxxxx/
> [2]: https://www.youtube.com/watch?v=DdFyH3BN5pI
> [3]: https://linux-nfs.org/wiki/index.php/CITI_Experience_with_Directory_Delegations
> [4]: https://git.kernel.org/pub/scm/linux/kernel/git/jlayton/linux.git/
> 
> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> ---
> Changes in v3:
> - Rework to do minimal work in fsnotify callbacks
> - Add support for sending attributes in CB_NOTIFY calls
> - Add support for dir attr change notifications
> - Link to v2: https://lore.kernel.org/r/20250602-dir-deleg-v2-0-a7919700de86@xxxxxxxxxx
> 
> Changes in v2:
> - add support for ignoring certain break_lease() events
> - basic support for CB_NOTIFY
> - Link to v1: https://lore.kernel.org/r/20240315-dir-deleg-v1-0-a1d6209a3654@xxxxxxxxxx
> 
> ---
> Jeff Layton (38):
>       filelock: push the S_ISREG check down to ->setlease handlers
>       filelock: add a lm_may_setlease lease_manager callback
>       vfs: add try_break_deleg calls for parents to vfs_{link,rename,unlink}
>       vfs: allow mkdir to wait for delegation break on parent
>       vfs: allow rmdir to wait for delegation break on parent
>       vfs: break parent dir delegations in open(..., O_CREAT) codepath
>       vfs: make vfs_create break delegations on parent directory
>       vfs: make vfs_mknod break delegations on parent directory
>       filelock: lift the ban on directory leases in generic_setlease
>       nfsd: allow filecache to hold S_IFDIR files
>       nfsd: allow DELEGRETURN on directories
>       nfsd: check for delegation conflicts vs. the same client
>       nfsd: wire up GET_DIR_DELEGATION handling
>       filelock: rework the __break_lease API to use flags
>       filelock: add struct delegated_inode
>       filelock: add support for ignoring deleg breaks for dir change events
>       filelock: add a tracepoint to start of break_lease()
>       filelock: add an inode_lease_ignore_mask helper
>       nfsd: add protocol support for CB_NOTIFY
>       nfs_common: add new NOTIFY4_* flags proposed in RFC8881bis
>       nfsd: allow nfsd to get a dir lease with an ignore mask
>       vfs: add fsnotify_modify_mark_mask()
>       nfsd: update the fsnotify mark when setting or removing a dir delegation
>       nfsd: make nfsd4_callback_ops->prepare operation bool return
>       nfsd: add callback encoding and decoding linkages for CB_NOTIFY
>       nfsd: add data structures for handling CB_NOTIFY to directory delegation
>       nfsd: add notification handlers for dir events
>       nfsd: add tracepoint to dir_event handler
>       nfsd: apply the notify mask to the delegation when requested
>       nfsd: add helper to marshal a fattr4 from completed args
>       nfsd: allow nfsd4_encode_fattr4_change() to work with no export
>       nfsd: send basic file attributes in CB_NOTIFY
>       nfsd: allow encoding a filehandle into fattr4 without a svc_fh
>       nfsd: add a fi_connectable flag to struct nfs4_file
>       nfsd: add the filehandle to returned attributes in CB_NOTIFY
>       nfsd: properly track requested child attributes
>       nfsd: track requested dir attributes
>       nfsd: add support to CB_NOTIFY for dir attribute changes
> 
>  Documentation/sunrpc/xdr/nfs4_1.x    | 267 +++++++++++++++++-
>  drivers/base/devtmpfs.c              |   2 +-
>  fs/attr.c                            |   4 +-
>  fs/cachefiles/namei.c                |   2 +-
>  fs/ecryptfs/inode.c                  |   2 +-
>  fs/fuse/dir.c                        |   1 +
>  fs/init.c                            |   2 +-
>  fs/locks.c                           | 122 ++++++--
>  fs/namei.c                           | 253 +++++++++++------
>  fs/nfs/nfs4file.c                    |   2 +
>  fs/nfsd/filecache.c                  | 101 +++++--
>  fs/nfsd/filecache.h                  |   2 +
>  fs/nfsd/nfs4callback.c               |  60 +++-
>  fs/nfsd/nfs4layouts.c                |   3 +-
>  fs/nfsd/nfs4proc.c                   |  36 ++-
>  fs/nfsd/nfs4recover.c                |   2 +-
>  fs/nfsd/nfs4state.c                  | 531 +++++++++++++++++++++++++++++++++--
>  fs/nfsd/nfs4xdr.c                    | 298 +++++++++++++++++---
>  fs/nfsd/nfs4xdr_gen.c                | 506 ++++++++++++++++++++++++++++++++-
>  fs/nfsd/nfs4xdr_gen.h                |  20 +-
>  fs/nfsd/state.h                      |  73 ++++-
>  fs/nfsd/trace.h                      |  21 ++
>  fs/nfsd/vfs.c                        |   7 +-
>  fs/nfsd/vfs.h                        |   2 +-
>  fs/nfsd/xdr4.h                       |   3 +
>  fs/nfsd/xdr4cb.h                     |  12 +
>  fs/notify/mark.c                     |  29 ++
>  fs/open.c                            |   8 +-
>  fs/overlayfs/overlayfs.h             |   2 +-
>  fs/posix_acl.c                       |  12 +-
>  fs/smb/client/cifsfs.c               |   3 +
>  fs/smb/server/vfs.c                  |   2 +-
>  fs/utimes.c                          |   4 +-
>  fs/xattr.c                           |  16 +-
>  fs/xfs/scrub/orphanage.c             |   2 +-
>  include/linux/filelock.h             | 143 +++++++---
>  include/linux/fs.h                   |  11 +-
>  include/linux/fsnotify_backend.h     |   1 +
>  include/linux/nfs4.h                 | 127 ---------
>  include/linux/sunrpc/xdrgen/nfs4_1.h | 304 +++++++++++++++++++-
>  include/linux/xattr.h                |   4 +-
>  include/trace/events/filelock.h      |  38 ++-
>  include/uapi/linux/nfs4.h            |   2 -
>  43 files changed, 2636 insertions(+), 406 deletions(-)
> ---
> base-commit: 36c204d169319562eed170f266c58460d5dad635
> change-id: 20240215-dir-deleg-e212210ba9d4
> 
> Best regards,

Series is clean and easy to read, thanks for your hard work! I agree
that the NFSD portions appear to be complete and ready to accept.

Because the series is cross-subsystem, we will need to discuss a merge
plan. So I'll hold off on R-b or Acked until that is nailed down.


-- 
Chuck Lever