2019-06-20 14:52:07

by J. Bruce Fields

[permalink] [raw]
Subject: [PATCH 00/16] exposing knfsd client state to userspace

From: "J. Bruce Fields" <[email protected]>

Changes since last time:
- added display of client implementation ID
- changed display of binary data to escape anything nonprintable
or nonascii
- some minor cleanup and patch reshuffling

I'm happy enough with at this point and hoping to merge it for 5.3.

Recapping discussion from last time:

The following patches expose information about NFSv4 state held by knfsd
on behalf of NFSv4 clients. This is especially important for opens,
which are currently invisible to userspace on the server, unlike locks
(/proc/locks) and local processes' opens (under /proc/<pid>/).

The approach is to add a new directory /proc/fs/nfsd/clients/ with
subdirectories for each active NFSv4 client. Each subdirectory has an
"info" file with some basic information to help identify the client and
a "states" directory that lists the open state held by that client.

Currently these pseudofiles look like:

# find /proc/fs/nfsd/clients -type f|xargs tail
==> /proc/fs/nfsd/clients/3/ctl <==
tail: error reading '/proc/fs/nfsd/clients/3/ctl': Invalid argument

==> /proc/fs/nfsd/clients/3/states <==
- 0x01000000f58f0b5d2898a19801000000: { type: open, access: r-, deny: --, superblock: "fd:10:13649", owner: "open id:\x00\x00\x00&\x00\x00\x00\x00\x00\x00\x02x\xac\xd0 \x09" }
- 0x01000000f58f0b5d2898a19802000000: { type: deleg, access: r, superblock: "fd:10:13649" }
- 0x01000000f58f0b5d2898a19803000000: { type: open, access: r-, deny: --, superblock: "fd:10:13650", owner: "open id:\x00\x00\x00&\x00\x00\x00\x00\x00\x00\x02x\xac\xd0 \x09" }
- 0x01000000f58f0b5d2898a19804000000: { type: deleg, access: r, superblock: "fd:10:13650" }

==> /proc/fs/nfsd/clients/3/info <==
clientid: 0x98a198285d0b8ff5
address: "192.168.122.36:935"
name: "Linux NFSv4.2 test2.fieldses.org"
minor version: 2
Implementation domain: "kernel.org"
Implementation name: "Linux 5.2.0-rc1-00017-ge73cab140ec0 #2263 SMP PREEMPT Tue Jun 18 16:54:41 EDT 2019 x86_64"
Implementation time: [0, 0]

The "ctl" file is not readable; all you can do is write "expire\n" to it
to force the server to revoke all that client's state.

The "info" and "states" files are meant to be valid YAML.

Possibly also todo, though I think none have to be done before merging:
- add some information about krb5 principals to the clients
file.
- add information about the stateids used to represent
asynchronous copies. They're a little different from the
other stateids and might end up in a separate "copies" file,
- this duplicates some functionality of the little-used fault
injection code; could we replace it entirely?
- some of the bits of filesystem code could probably be shared
with rpc_pipefs and libfs.

--b.

J. Bruce Fields (16):
nfsd: persist nfsd filesystem across mounts
nfsd: rename cl_refcount
nfsd4: use reference count to free client
nfsd: add nfsd/clients directory
nfsd: make client/ directory names small ints
nfsd4: add a client info file
nfsd: copy client's address including port number to cl_addr
nfsd: escape high characters in binary data
nfsd: add more information to client info file
nfsd4: add file to display list of client's opens
nfsd: show lock and deleg stateids
nfsd4: show layout stateids
nfsd: create get_nfsdfs_clp helper
nfsd: allow forced expiration of NFSv4 clients
nfsd: create xdr_netobj_dup helper
nfsd: decode implementation id

fs/nfsd/netns.h | 6 +
fs/nfsd/nfs4state.c | 453 ++++++++++++++++++++++++++++++---
fs/nfsd/nfs4xdr.c | 21 +-
fs/nfsd/nfsctl.c | 221 +++++++++++++++-
fs/nfsd/nfsd.h | 11 +
fs/nfsd/state.h | 11 +-
fs/nfsd/xdr4.h | 3 +
fs/seq_file.c | 11 +
include/linux/seq_file.h | 1 +
include/linux/string_helpers.h | 3 +
include/linux/sunrpc/xdr.h | 7 +
lib/string_helpers.c | 19 ++
12 files changed, 723 insertions(+), 44 deletions(-)

--
2.21.0


2019-06-20 14:52:22

by J. Bruce Fields

[permalink] [raw]
Subject: [PATCH 16/16] nfsd: decode implementation id

From: "J. Bruce Fields" <[email protected]>

Decode the implementation ID and display in nfsd/clients/#/info. It may
be help identify the client. It won't be used otherwise.

(When this went into the protocol, I thought the implementation ID would
be a slippery slope towards implementation-specific workarounds as with
the http user-agent. But I guess I was wrong, the risk seems pretty low
now.)

Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/nfsd/nfs4state.c | 30 ++++++++++++++++++++++++++++++
fs/nfsd/nfs4xdr.c | 21 +++++++++------------
fs/nfsd/state.h | 4 ++++
fs/nfsd/xdr4.h | 3 +++
4 files changed, 46 insertions(+), 12 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 8f35e440ef14..4fcbb5d809a6 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1899,6 +1899,8 @@ static void __free_client(struct kref *k)
free_svc_cred(&clp->cl_cred);
kfree(clp->cl_ownerstr_hashtbl);
kfree(clp->cl_name.data);
+ kfree(clp->cl_nii_domain.data);
+ kfree(clp->cl_nii_name.data);
idr_destroy(&clp->cl_stateids);
kmem_cache_free(client_slab, clp);
}
@@ -2261,6 +2263,15 @@ static int client_info_show(struct seq_file *m, void *v)
seq_printf(m, "name: ");
seq_quote_mem(m, clp->cl_name.data, clp->cl_name.len);
seq_printf(m, "\nminor version: %d\n", clp->cl_minorversion);
+ if (clp->cl_nii_domain.data) {
+ seq_printf(m, "Implementation domain: ");
+ seq_quote_mem(m, clp->cl_nii_domain.data,
+ clp->cl_nii_domain.len);
+ seq_printf(m, "\nImplementation name: ");
+ seq_quote_mem(m, clp->cl_nii_name.data, clp->cl_nii_name.len);
+ seq_printf(m, "\nImplementation time: [%ld, %ld]\n",
+ clp->cl_nii_time.tv_sec, clp->cl_nii_time.tv_nsec);
+ }
drop_client(clp);

return 0;
@@ -2901,6 +2912,22 @@ static bool client_has_state(struct nfs4_client *clp)
|| !list_empty(&clp->async_copies);
}

+static __be32 copy_impl_id(struct nfs4_client *clp,
+ struct nfsd4_exchange_id *exid)
+{
+ if (!exid->nii_domain.data)
+ return 0;
+ xdr_netobj_dup(&clp->cl_nii_domain, &exid->nii_domain, GFP_KERNEL);
+ if (!clp->cl_nii_domain.data)
+ return nfserr_jukebox;
+ xdr_netobj_dup(&clp->cl_nii_name, &exid->nii_name, GFP_KERNEL);
+ if (!clp->cl_nii_name.data)
+ return nfserr_jukebox;
+ clp->cl_nii_time.tv_sec = exid->nii_time.tv_sec;
+ clp->cl_nii_time.tv_nsec = exid->nii_time.tv_nsec;
+ return 0;
+}
+
__be32
nfsd4_exchange_id(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
union nfsd4_op_u *u)
@@ -2927,6 +2954,9 @@ nfsd4_exchange_id(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
new = create_client(exid->clname, rqstp, &verf);
if (new == NULL)
return nfserr_jukebox;
+ status = copy_impl_id(new, exid);
+ if (status)
+ goto out_nolock;

switch (exid->spa_how) {
case SP4_MACH_CRED:
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 52c4f6daa649..3bb147822205 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -1398,7 +1398,6 @@ nfsd4_decode_exchange_id(struct nfsd4_compoundargs *argp,
goto xdr_error;
}

- /* Ignore Implementation ID */
READ_BUF(4); /* nfs_impl_id4 array length */
dummy = be32_to_cpup(p++);

@@ -1406,21 +1405,19 @@ nfsd4_decode_exchange_id(struct nfsd4_compoundargs *argp,
goto xdr_error;

if (dummy == 1) {
- /* nii_domain */
- READ_BUF(4);
- dummy = be32_to_cpup(p++);
- READ_BUF(dummy);
- p += XDR_QUADLEN(dummy);
+ status = nfsd4_decode_opaque(argp, &exid->nii_domain);
+ if (status)
+ goto xdr_error;

/* nii_name */
- READ_BUF(4);
- dummy = be32_to_cpup(p++);
- READ_BUF(dummy);
- p += XDR_QUADLEN(dummy);
+ status = nfsd4_decode_opaque(argp, &exid->nii_name);
+ if (status)
+ goto xdr_error;

/* nii_date */
- READ_BUF(12);
- p += 3;
+ status = nfsd4_decode_time(argp, &exid->nii_time);
+ if (status)
+ goto xdr_error;
}
DECODE_TAIL;
}
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 81852cbf6b0a..8cb20cab012b 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -317,6 +317,10 @@ struct nfs4_client {
clientid_t cl_clientid; /* generated by server */
nfs4_verifier cl_confirm; /* generated by server */
u32 cl_minorversion;
+ /* NFSv4.1 client implementation id: */
+ struct xdr_netobj cl_nii_domain;
+ struct xdr_netobj cl_nii_name;
+ struct timespec cl_nii_time;

/* for v4.0 and v4.1 callbacks: */
struct nfs4_cb_conn cl_cb_conn;
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index feeb6d4bdffd..a5222fc9ea44 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -410,6 +410,9 @@ struct nfsd4_exchange_id {
int spa_how;
u32 spo_must_enforce[3];
u32 spo_must_allow[3];
+ struct xdr_netobj nii_domain;
+ struct xdr_netobj nii_name;
+ struct timespec nii_time;
};

struct nfsd4_sequence {
--
2.21.0

2019-06-20 14:52:29

by J. Bruce Fields

[permalink] [raw]
Subject: [PATCH 07/16] nfsd: copy client's address including port number to cl_addr

From: "J. Bruce Fields" <[email protected]>

rpc_copy_addr() copies only the IP address and misses any port numbers.
It seems potentially useful to keep the port number around too.

Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/nfsd/nfs4state.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index d3de89dacf89..dd89dc05f6ee 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2273,7 +2273,7 @@ static struct nfs4_client *create_client(struct xdr_netobj name,
clp->cl_time = get_seconds();
clear_bit(0, &clp->cl_cb_slot_busy);
copy_verf(clp, verf);
- rpc_copy_addr((struct sockaddr *) &clp->cl_addr, sa);
+ memcpy(&clp->cl_addr, sa, sizeof(struct sockaddr_storage));
clp->cl_cb_session = NULL;
clp->net = net;
clp->cl_nfsd_dentry = nfsd_client_mkdir(nn, &clp->cl_nfsdfs,
--
2.21.0

2019-06-20 14:52:31

by J. Bruce Fields

[permalink] [raw]
Subject: [PATCH 12/16] nfsd4: show layout stateids

From: "J. Bruce Fields" <[email protected]>

These are also minimal for now, I'm not sure what information would be
useful.

Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/nfsd/nfs4state.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index ab4302cee2d2..7867372363ff 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2398,6 +2398,24 @@ static int nfs4_show_deleg(struct seq_file *s, struct nfs4_stid *st)
return 0;
}

+static int nfs4_show_layout(struct seq_file *s, struct nfs4_stid *st)
+{
+ struct nfs4_layout_stateid *ls;
+ struct file *file;
+
+ ls = container_of(st, struct nfs4_layout_stateid, ls_stid);
+ file = ls->ls_file;
+
+ seq_printf(s, "- 0x%16phN: { type: layout, ", &st->sc_stateid);
+
+ /* XXX: What else would be useful? */
+
+ nfs4_show_superblock(s, file);
+ seq_printf(s, " }\n");
+
+ return 0;
+}
+
static int states_show(struct seq_file *s, void *v)
{
struct nfs4_stid *st = v;
@@ -2409,6 +2427,8 @@ static int states_show(struct seq_file *s, void *v)
return nfs4_show_lock(s, st);
case NFS4_DELEG_STID:
return nfs4_show_deleg(s, st);
+ case NFS4_LAYOUT_STID:
+ return nfs4_show_layout(s, st);
default:
return 0; /* XXX: or SEQ_SKIP? */
}
--
2.21.0

2019-06-20 14:52:31

by J. Bruce Fields

[permalink] [raw]
Subject: [PATCH 14/16] nfsd: allow forced expiration of NFSv4 clients

From: "J. Bruce Fields" <[email protected]>

NFSv4 clients are automatically expired and all their locks removed if
they don't contact the server for a certain amount of time (the lease
period, 90 seconds by default).

There can still be situations where that's not enough, so allow
userspace to force expiry by writing "expire\n" to the new
nfsd/client/#/ctl file.

(The generic "ctl" name is because I expect we may want to allow other
operations on clients in the future.)

The write will not return until the client is expired and all of its
locks and other state removed.

The fault injection code also provides a way of expiring clients, but it
fails if there are any in-progress RPC's referencing the client. Also,
its method of selecting a client to expire is a little more
primitive--it uses an IP address, which can't always uniquely specify an
NFSv4 client.

Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/nfsd/nfs4state.c | 70 ++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 69 insertions(+), 1 deletion(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 63f6b87e178e..12e370e62453 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -100,6 +100,13 @@ enum nfsd4_st_mutex_lock_subclass {
*/
static DECLARE_WAIT_QUEUE_HEAD(close_wq);

+/*
+ * A waitqueue where a writer to clients/#/ctl destroying a client can
+ * wait for cl_rpc_users to drop to 0 and then for the client to be
+ * unhashed.
+ */
+static DECLARE_WAIT_QUEUE_HEAD(expiry_wq);
+
static struct kmem_cache *client_slab;
static struct kmem_cache *openowner_slab;
static struct kmem_cache *lockowner_slab;
@@ -175,6 +182,8 @@ static void put_client_renew_locked(struct nfs4_client *clp)
return;
if (!is_client_expired(clp))
renew_client_locked(clp);
+ else
+ wake_up_all(&expiry_wq);
}

static void put_client_renew(struct nfs4_client *clp)
@@ -185,6 +194,8 @@ static void put_client_renew(struct nfs4_client *clp)
return;
if (!is_client_expired(clp))
renew_client_locked(clp);
+ else
+ wake_up_all(&expiry_wq);
spin_unlock(&nn->client_lock);
}

@@ -1910,8 +1921,11 @@ free_client(struct nfs4_client *clp)
free_session(ses);
}
rpc_destroy_wait_queue(&clp->cl_cb_waitq);
- if (clp->cl_nfsd_dentry)
+ if (clp->cl_nfsd_dentry) {
nfsd_client_rmdir(clp->cl_nfsd_dentry);
+ clp->cl_nfsd_dentry = NULL;
+ wake_up_all(&expiry_wq);
+ }
drop_client(clp);
}

@@ -2006,6 +2020,7 @@ __destroy_client(struct nfs4_client *clp)
if (clp->cl_cb_conn.cb_xprt)
svc_xprt_put(clp->cl_cb_conn.cb_xprt);
free_client(clp);
+ wake_up_all(&expiry_wq);
}

static void
@@ -2484,9 +2499,62 @@ static const struct file_operations client_states_fops = {
.release = client_opens_release,
};

+/*
+ * Normally we refuse to destroy clients that are in use, but here the
+ * administrator is telling us to just do it. We also want to wait
+ * so the caller has a guarantee that the client's locks are gone by
+ * the time the write returns:
+ */
+void force_expire_client(struct nfs4_client *clp)
+{
+ struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
+ bool already_expired;
+
+ spin_lock(&clp->cl_lock);
+ clp->cl_time = 0;
+ spin_unlock(&clp->cl_lock);
+
+ wait_event(expiry_wq, atomic_read(&clp->cl_rpc_users) == 0);
+ spin_lock(&nn->client_lock);
+ already_expired = list_empty(&clp->cl_lru);
+ if (!already_expired)
+ unhash_client_locked(clp);
+ spin_unlock(&nn->client_lock);
+
+ if (!already_expired)
+ expire_client(clp);
+ else
+ wait_event(expiry_wq, clp->cl_nfsd_dentry == NULL);
+}
+
+static ssize_t client_ctl_write(struct file *file, const char __user *buf,
+ size_t size, loff_t *pos)
+{
+ char *data;
+ struct nfs4_client *clp;
+
+ data = simple_transaction_get(file, buf, size);
+ if (IS_ERR(data))
+ return PTR_ERR(data);
+ if (size != 7 || 0 != memcmp(data, "expire\n", 7))
+ return -EINVAL;
+ clp = get_nfsdfs_clp(file_inode(file));
+ if (!clp)
+ return -ENXIO;
+ force_expire_client(clp);
+ drop_client(clp);
+ return 7;
+}
+
+static const struct file_operations client_ctl_fops = {
+ .write = client_ctl_write,
+ .release = simple_transaction_release,
+};
+
static const struct tree_descr client_files[] = {
[0] = {"info", &client_info_fops, S_IRUSR},
[1] = {"states", &client_states_fops, S_IRUSR},
+ [2] = {"ctl", &client_ctl_fops, S_IRUSR|S_IWUSR},
[3] = {""},
};

--
2.21.0

2019-06-20 14:52:33

by J. Bruce Fields

[permalink] [raw]
Subject: [PATCH 02/16] nfsd: rename cl_refcount

From: "J. Bruce Fields" <[email protected]>

Rename this to a more descriptive name: it counts the number of
in-progress rpc's referencing this client.

Next I'm going to add a second refcount with a slightly different use.

Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/nfsd/nfs4state.c | 26 +++++++++++++-------------
fs/nfsd/state.h | 2 +-
2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 618e66078ee5..2a13b6cbb695 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -138,7 +138,7 @@ static __be32 get_client_locked(struct nfs4_client *clp)

if (is_client_expired(clp))
return nfserr_expired;
- atomic_inc(&clp->cl_refcount);
+ atomic_inc(&clp->cl_rpc_users);
return nfs_ok;
}

@@ -170,7 +170,7 @@ static void put_client_renew_locked(struct nfs4_client *clp)

lockdep_assert_held(&nn->client_lock);

- if (!atomic_dec_and_test(&clp->cl_refcount))
+ if (!atomic_dec_and_test(&clp->cl_rpc_users))
return;
if (!is_client_expired(clp))
renew_client_locked(clp);
@@ -180,7 +180,7 @@ static void put_client_renew(struct nfs4_client *clp)
{
struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);

- if (!atomic_dec_and_lock(&clp->cl_refcount, &nn->client_lock))
+ if (!atomic_dec_and_lock(&clp->cl_rpc_users, &nn->client_lock))
return;
if (!is_client_expired(clp))
renew_client_locked(clp);
@@ -1857,7 +1857,7 @@ static struct nfs4_client *alloc_client(struct xdr_netobj name)
clp->cl_name.len = name.len;
INIT_LIST_HEAD(&clp->cl_sessions);
idr_init(&clp->cl_stateids);
- atomic_set(&clp->cl_refcount, 0);
+ atomic_set(&clp->cl_rpc_users, 0);
clp->cl_cb_state = NFSD4_CB_UNKNOWN;
INIT_LIST_HEAD(&clp->cl_idhash);
INIT_LIST_HEAD(&clp->cl_openowners);
@@ -1936,7 +1936,7 @@ unhash_client(struct nfs4_client *clp)

static __be32 mark_client_expired_locked(struct nfs4_client *clp)
{
- if (atomic_read(&clp->cl_refcount))
+ if (atomic_read(&clp->cl_rpc_users))
return nfserr_jukebox;
unhash_client_locked(clp);
return nfs_ok;
@@ -4092,7 +4092,7 @@ static __be32 lookup_clientid(clientid_t *clid,
spin_unlock(&nn->client_lock);
return nfserr_expired;
}
- atomic_inc(&found->cl_refcount);
+ atomic_inc(&found->cl_rpc_users);
spin_unlock(&nn->client_lock);

/* Cache the nfs4_client in cstate! */
@@ -6584,7 +6584,7 @@ nfs4_check_open_reclaim(clientid_t *clid,
static inline void
put_client(struct nfs4_client *clp)
{
- atomic_dec(&clp->cl_refcount);
+ atomic_dec(&clp->cl_rpc_users);
}

static struct nfs4_client *
@@ -6702,7 +6702,7 @@ nfsd_inject_add_lock_to_list(struct nfs4_ol_stateid *lst,
return;

lockdep_assert_held(&nn->client_lock);
- atomic_inc(&clp->cl_refcount);
+ atomic_inc(&clp->cl_rpc_users);
list_add(&lst->st_locks, collect);
}

@@ -6731,7 +6731,7 @@ static u64 nfsd_foreach_client_lock(struct nfs4_client *clp, u64 max,
* Despite the fact that these functions deal
* with 64-bit integers for "count", we must
* ensure that it doesn't blow up the
- * clp->cl_refcount. Throw a warning if we
+ * clp->cl_rpc_users. Throw a warning if we
* start to approach INT_MAX here.
*/
WARN_ON_ONCE(count == (INT_MAX / 2));
@@ -6855,7 +6855,7 @@ nfsd_foreach_client_openowner(struct nfs4_client *clp, u64 max,
if (func) {
func(oop);
if (collect) {
- atomic_inc(&clp->cl_refcount);
+ atomic_inc(&clp->cl_rpc_users);
list_add(&oop->oo_perclient, collect);
}
}
@@ -6863,7 +6863,7 @@ nfsd_foreach_client_openowner(struct nfs4_client *clp, u64 max,
/*
* Despite the fact that these functions deal with
* 64-bit integers for "count", we must ensure that
- * it doesn't blow up the clp->cl_refcount. Throw a
+ * it doesn't blow up the clp->cl_rpc_users. Throw a
* warning if we start to approach INT_MAX here.
*/
WARN_ON_ONCE(count == (INT_MAX / 2));
@@ -6993,7 +6993,7 @@ static u64 nfsd_find_all_delegations(struct nfs4_client *clp, u64 max,
if (dp->dl_time != 0)
continue;

- atomic_inc(&clp->cl_refcount);
+ atomic_inc(&clp->cl_rpc_users);
WARN_ON(!unhash_delegation_locked(dp));
list_add(&dp->dl_recall_lru, victims);
}
@@ -7001,7 +7001,7 @@ static u64 nfsd_find_all_delegations(struct nfs4_client *clp, u64 max,
/*
* Despite the fact that these functions deal with
* 64-bit integers for "count", we must ensure that
- * it doesn't blow up the clp->cl_refcount. Throw a
+ * it doesn't blow up the clp->cl_rpc_users. Throw a
* warning if we start to approach INT_MAX here.
*/
WARN_ON_ONCE(count == (INT_MAX / 2));
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 0b74d371ed67..f79ad7202e82 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -347,7 +347,7 @@ struct nfs4_client {
struct nfsd4_clid_slot cl_cs_slot; /* create_session slot */
u32 cl_exchange_flags;
/* number of rpc's in progress over an associated session: */
- atomic_t cl_refcount;
+ atomic_t cl_rpc_users;
struct nfs4_op_map cl_spo_must_allow;

/* for nfs41 callbacks */
--
2.21.0

2019-06-21 18:13:41

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 00/16] exposing knfsd client state to userspace

On Thu, Jun 20, 2019 at 10:50:59AM -0400, J. Bruce Fields wrote:
> - this duplicates some functionality of the little-used fault
> injection code; could we replace it entirely?

I'd be really curious to hear from any users of that code, by the way.
Anna, any ideas?

The idea was that it could be used to test client handling of
exceptional conditions like recalled delegations and partially lost
state. Is anyone regularly running such tests?

I don't hate the code, and I'm not on a crusade to tear it all out Right
Now, but it does create a few odd corner cases, so I'm wondering whether
I could get away with replacing it eventually or whether that risks
breaking someone's scripts.

--b.

2019-06-21 19:25:32

by Anna Schumaker

[permalink] [raw]
Subject: Re: [PATCH 00/16] exposing knfsd client state to userspace

On Fri, 2019-06-21 at 14:13 -0400, J. Bruce Fields wrote:
> On Thu, Jun 20, 2019 at 10:50:59AM -0400, J. Bruce Fields wrote:
> > - this duplicates some functionality of the little-used fault
> > injection code; could we replace it entirely?
>
> I'd be really curious to hear from any users of that code, by the
> way.
> Anna, any ideas?

I'm not sure who else has used it besides me, and it's been a while
since I have too.

>
> The idea was that it could be used to test client handling of
> exceptional conditions like recalled delegations and partially lost
> state. Is anyone regularly running such tests?
>
> I don't hate the code, and I'm not on a crusade to tear it all out
> Right
> Now, but it does create a few odd corner cases, so I'm wondering
> whether
> I could get away with replacing it eventually or whether that risks
> breaking someone's scripts.

I'm cool with replacing it if there is a better way to do things.

Anna

>
> --b.

2019-06-21 22:08:31

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 00/16] exposing knfsd client state to userspace

On Fri, Jun 21, 2019 at 03:25:04PM -0400, Anna Schumaker wrote:
> On Fri, 2019-06-21 at 14:13 -0400, J. Bruce Fields wrote:
> > On Thu, Jun 20, 2019 at 10:50:59AM -0400, J. Bruce Fields wrote:
> > > - this duplicates some functionality of the little-used fault
> > > injection code; could we replace it entirely?
> >
> > I'd be really curious to hear from any users of that code, by the
> > way.
> > Anna, any ideas?
>
> I'm not sure who else has used it besides me, and it's been a while
> since I have too.

Do you remember which ones were most useful? They are:

- forget_clients
- forget_locks
- forget_openowners
- forget_delegations
- recall_delegations

We've got a functional replacement for forget_clients, but I haven't
looked into the others yet.

--b.

>
> >
> > The idea was that it could be used to test client handling of
> > exceptional conditions like recalled delegations and partially lost
> > state. Is anyone regularly running such tests?
> >
> > I don't hate the code, and I'm not on a crusade to tear it all out
> > Right
> > Now, but it does create a few odd corner cases, so I'm wondering
> > whether
> > I could get away with replacing it eventually or whether that risks
> > breaking someone's scripts.
>
> I'm cool with replacing it if there is a better way to do things.
>
> Anna
>
> >
> > --b.