On Thu, 2025-09-25 at 09:39 -0400, Chuck Lever wrote: > 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. >
Thanks. It's sensible to hold off for a bit. There is at least one leak that I found earlier today, and a few cleanups that I have queued up. We also have a bake-a-thon in another couple of weeks where I hope to test this more extensively. After that, I'm hoping we'll be in reasonable shape to take it into linux-next. What I may do is reorder the vfs patches to the front of the queue and plead to Christian and Al to take them into a branch that feeds into linux-next. That way we can at least get some feedback and testing with those bits in place, and a foundation on which we can merge the nfsd bits. -- Jeff Layton <[email protected]>
