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

Chuck Lever <[email protected]>
Newsgroups org.kernel.vger.ecryptfs,dev.linux.lists.netfs,org.kernel.vger.linux-cifs,org.kernel.vger.linux-doc,org.kernel.vger.linux-fsdevel,org.kernel.vger.linux-kernel,org.kernel.vger.linux-nfs,org.kernel.vger.linux-trace-kernel,org.kernel.vger.linux-unionfs,org.kernel.vger.linux-xfs
Message-ID <[email protected]>
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/[email protected]/
> [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 <[email protected]>
> ---
> 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/[email protected]
> 
> Changes in v2:
> - add support for ignoring certain break_lease() events
> - basic support for CB_NOTIFY
> - Link to v1: https://lore.kernel.org/r/[email protected]
> 
> ---
> 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.