[PATCH RFC 23/24] nfs: optionally request a delegation on GETATTR

From: Jeff Layton
Date: Fri Mar 15 2024 - 13:05:06 EST

Next message: Long Li: "RE: [PATCH rdma-next 2/4] RDMA:mana_ib: Use struct mana_ib_queue for CQs"
Previous message: Jeff Layton: "[PATCH RFC 22/24] nfs: skip dentry revalidation when parent dir has a delegation"
In reply to: Jeff Layton: "[PATCH RFC 22/24] nfs: skip dentry revalidation when parent dir has a delegation"
Next in thread: Jeff Layton: "[PATCH RFC 24/24] nfs: add a module parameter to disable directory delegations"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

We expect directory delegations to most helpful when their children have
frequently revalidated dentries. The way we usually revalidate them is
to check that the dentry's d_time field matches the change attribute on
the parent. Most dentry revalidations that have to issue an RPC end up
as a GETATTR on the wire for the parent, if the parent hasn't changed,
then the dentry is fine.

Add a new NFS_INO_GDD_GETATTR flag to the nfsi->flags field. Whenever we
validate a dentry vs. its parent, set that flag on the parent. Then,
when we next do a GETATTR vs. the parent, test and clear the flag and do
a GDD_GETATTR if it was set. If the the GET_DIR_DELEGATION succeeds, set
the delegation in the inode.

Finally, a lot of servers don't support GET_DIR_DELEGATION, so add a
NFS_CAP_* bit to track whether the server supports it, and enable it by
default on v4.1+, and retry without a delegation if we get back an
ENOTSUPP.

Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
---
fs/nfs/delegation.c | 1 +
fs/nfs/dir.c | 8 ++++++++
fs/nfs/nfs4proc.c | 45 ++++++++++++++++++++++++++++++++++++++++++---
include/linux/nfs_fs.h | 1 +
include/linux/nfs_fs_sb.h | 1 +
5 files changed, 53 insertions(+), 3 deletions(-)

diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
index 88dbec4739d3..14c14bd03465 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -356,6 +356,7 @@ nfs_detach_delegation_locked(struct nfs_inode *nfsi,
delegation->inode = NULL;
rcu_assign_pointer(nfsi->delegation, NULL);
spin_unlock(&delegation->lock);
+ clear_bit(NFS_INO_GDD_GETATTR, &nfsi->flags);
return delegation;
}

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 061648c73116..930fe7e14914 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1504,12 +1504,20 @@ static int nfs_dentry_verify_change(struct inode *dir, struct dentry *dentry)
static int nfs_check_verifier(struct inode *dir, struct dentry *dentry,
int rcu_walk)
{
+ struct nfs_inode *nfsi = NFS_I(dir);
+
if (IS_ROOT(dentry))
return 1;
if (NFS_SERVER(dir)->flags & NFS_MOUNT_LOOKUP_CACHE_NONE)
return 0;
if (!nfs_dentry_verify_change(dir, dentry))
return 0;
+ /*
+ * The dentry matches the directory's change attribute, so
+ * we're likely revalidating here. Flag the dir so that we
+ * ask for a delegation on the next getattr.
+ */
+ set_bit(NFS_INO_GDD_GETATTR, &nfsi->flags);
/* Revalidate nfsi->cache_change_attribute before we declare a match */
if (nfs_mapping_need_revalidate_inode(dir)) {
if (rcu_walk)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index d518388bd0d6..3dbe9a18c9be 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -4318,6 +4318,21 @@ static int nfs4_get_referral(struct rpc_clnt *client, struct inode *dir,
return status;
}

+static bool should_request_dir_deleg(struct inode *inode)
+{
+ if (!inode)
+ return false;
+ if (!S_ISDIR(inode->i_mode))
+ return false;
+ if (!nfs_server_capable(inode, NFS_CAP_GET_DIR_DELEG))
+ return false;
+ if (!test_and_clear_bit(NFS_INO_GDD_GETATTR, &(NFS_I(inode)->flags)))
+ return false;
+ if (nfs4_have_delegation(inode, FMODE_READ))
+ return false;
+ return true;
+}
+
static int _nfs4_proc_getattr(struct nfs_server *server, struct nfs_fh *fhandle,
struct nfs_fattr *fattr, struct inode *inode)
{
@@ -4331,11 +4346,12 @@ static int _nfs4_proc_getattr(struct nfs_server *server, struct nfs_fh *fhandle,
.server = server,
};
struct rpc_message msg = {
- .rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_GETATTR],
.rpc_argp = &args,
.rpc_resp = &res,
};
unsigned short task_flags = 0;
+ bool gdd;
+ int status;

if (nfs4_has_session(server->nfs_client))
task_flags = RPC_TASK_MOVEABLE;
@@ -4344,11 +4360,32 @@ static int _nfs4_proc_getattr(struct nfs_server *server, struct nfs_fh *fhandle,
if (inode && (server->flags & NFS_MOUNT_SOFTREVAL))
task_flags |= RPC_TASK_TIMEOUT;

+retry:
+ gdd = should_request_dir_deleg(inode);
+ if (gdd)
+ msg.rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_GDD_GETATTR];
+ else
+ msg.rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_GETATTR];
+
nfs4_bitmap_copy_adjust(bitmask, nfs4_bitmask(server, fattr->label), inode, 0);
nfs_fattr_init(fattr);
nfs4_init_sequence(&args.seq_args, &res.seq_res, 0, 0);
- return nfs4_do_call_sync(server->client, server, &msg,
- &args.seq_args, &res.seq_res, task_flags);
+ status = nfs4_do_call_sync(server->client, server, &msg,
+ &args.seq_args, &res.seq_res, task_flags);
+ switch (status) {
+ case 0:
+ if (gdd && res.nf_status == GDD4_OK)
+ status = nfs_inode_set_delegation(inode, current_cred(), FMODE_READ,
+ &res.deleg, 0);
+ break;
+ case -ENOTSUPP:
+ /* If the server doesn't support GET_DIR_DELEGATION, retry without it */
+ if (gdd) {
+ server->caps &= ~NFS_CAP_GET_DIR_DELEG;
+ goto retry;
+ }
+ }
+ return status;
}

int nfs4_proc_getattr(struct nfs_server *server, struct nfs_fh *fhandle,
@@ -10552,6 +10589,7 @@ static const struct nfs4_minor_version_ops nfs_v4_1_minor_ops = {
.minor_version = 1,
.init_caps = NFS_CAP_READDIRPLUS
| NFS_CAP_ATOMIC_OPEN
+ | NFS_CAP_GET_DIR_DELEG
| NFS_CAP_POSIX_LOCK
| NFS_CAP_STATEID_NFSV41
| NFS_CAP_ATOMIC_OPEN_V1
@@ -10578,6 +10616,7 @@ static const struct nfs4_minor_version_ops nfs_v4_2_minor_ops = {
.minor_version = 2,
.init_caps = NFS_CAP_READDIRPLUS
| NFS_CAP_ATOMIC_OPEN
+ | NFS_CAP_GET_DIR_DELEG
| NFS_CAP_POSIX_LOCK
| NFS_CAP_STATEID_NFSV41
| NFS_CAP_ATOMIC_OPEN_V1
diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index f5ce7b101146..5140e4dac98a 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -318,6 +318,7 @@ struct nfs4_copy_state {
#define NFS_INO_LAYOUTCOMMITTING (10) /* layoutcommit inflight */
#define NFS_INO_LAYOUTSTATS (11) /* layoutstats inflight */
#define NFS_INO_ODIRECT (12) /* I/O setting is O_DIRECT */
+#define NFS_INO_GDD_GETATTR (13) /* Ask for dir deleg on next GETATTR */

static inline struct nfs_inode *NFS_I(const struct inode *inode)
{
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index 92de074e63b9..0ab744cf52d7 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -278,6 +278,7 @@ struct nfs_server {
#define NFS_CAP_LGOPEN (1U << 5)
#define NFS_CAP_CASE_INSENSITIVE (1U << 6)
#define NFS_CAP_CASE_PRESERVING (1U << 7)
+#define NFS_CAP_GET_DIR_DELEG (1U << 13)
#define NFS_CAP_POSIX_LOCK (1U << 14)
#define NFS_CAP_UIDGID_NOMAP (1U << 15)
#define NFS_CAP_STATEID_NFSV41 (1U << 16)

--
2.44.0

Next message: Long Li: "RE: [PATCH rdma-next 2/4] RDMA:mana_ib: Use struct mana_ib_queue for CQs"
Previous message: Jeff Layton: "[PATCH RFC 22/24] nfs: skip dentry revalidation when parent dir has a delegation"
In reply to: Jeff Layton: "[PATCH RFC 22/24] nfs: skip dentry revalidation when parent dir has a delegation"
Next in thread: Jeff Layton: "[PATCH RFC 24/24] nfs: add a module parameter to disable directory delegations"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]