2022-09-14 16:10:17

by Dai Ngo

[permalink] [raw]
Subject: [PATCH v7 0/2] NFSD: memory shrinker for NFSv4 clients

This patch series implements the memory shrinker for NFSv4 clients
to react to system low memory condition.

The first patch adds a counter to keep track of the number of
courtesy clients in the system.

The second patch implements the courtesy_client_reaper used to
expiring the courtesy clients.

By destroying the courtesy clients, all states associated with
these clients are also released.

v2:
. fix kernel test robot errors in nfsd.h when CONFIG_NFSD_V4 not defined.

v3:
. add mod_delayed_work in nfsd_courtesy_client_scan to kick start
the laundromat.

v4:
. replace the use of xchg() with vanilla '=' in patch 1.

v5:
. rename nfsd_courtesy_client_count to nfsd_courtesy_clients
. add helper nfsd4_update_courtesy_client_count
. move nfsd_register_client_shrinker into nfsd4_init_leases_net
. move nfsd4_leases_net_shutdown from nfsd.h to nfs4state.c
. do away with shrinker 'scan' callback, just return SHRINK_STOP
. remove unused nfsd_client_shrinker_reapcount

v6:
. create courtesy_client_reaper and a separate delayed_work for it
using the laundromat_wq.
I tried merging nfs4_get_courtesy_client_reaplist and
nfs4_get_client_reaplist but it make the code looks ugly and
hard to read so I leave them as separate for now.

v7:
. patch1: rename nfsd4_decr_courtesy_client_count to
nfsd4_dec_courtesy_client_count
. patch 2: get rid of nfsd_client_shrinker_cb_count and do not
reschedule courtesy_client_reaper
---

Dai Ngo (2):
NFSD: keep track of the number of courtesy clients in the system
NFSD: add shrinker to reap courtesy clients on low memory condition

fs/nfsd/netns.h | 4 ++
fs/nfsd/nfs4state.c | 111 +++++++++++++++++++++++++++++++++++++++++++----
fs/nfsd/nfsctl.c | 6 ++-
fs/nfsd/nfsd.h | 7 ++-
4 files changed, 115 insertions(+), 13 deletions(-)


2022-09-14 16:10:18

by Dai Ngo

[permalink] [raw]
Subject: [PATCH v7 2/2] NFSD: add shrinker to reap courtesy clients on low memory condition

Add courtesy_client_reaper to react to low memory condition triggered
by the system memory shrinker.

The delayed_work for the courtesy_client_reaper is scheduled on
the shrinker's count callback using the laundry_wq.

The shrinker's scan callback is not used for expiring the courtesy
clients due to potential deadlocks.

Signed-off-by: Dai Ngo <[email protected]>
---
fs/nfsd/netns.h | 2 ++
fs/nfsd/nfs4state.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++-----
fs/nfsd/nfsctl.c | 6 ++--
fs/nfsd/nfsd.h | 7 ++--
4 files changed, 97 insertions(+), 12 deletions(-)

diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
index 55c7006d6109..8c854ba3285b 100644
--- a/fs/nfsd/netns.h
+++ b/fs/nfsd/netns.h
@@ -194,6 +194,8 @@ struct nfsd_net {
int nfs4_max_clients;

atomic_t nfsd_courtesy_clients;
+ struct shrinker nfsd_client_shrinker;
+ struct delayed_work nfsd_shrinker_work;
};

/* Simple check to find out if a given net was properly initialized */
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 2827329704ea..62b848bb55df 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4347,7 +4347,27 @@ nfsd4_init_slabs(void)
return -ENOMEM;
}

-void nfsd4_init_leases_net(struct nfsd_net *nn)
+static unsigned long
+nfsd_courtesy_client_count(struct shrinker *shrink, struct shrink_control *sc)
+{
+ int cnt;
+ struct nfsd_net *nn = container_of(shrink,
+ struct nfsd_net, nfsd_client_shrinker);
+
+ cnt = atomic_read(&nn->nfsd_courtesy_clients);
+ if (cnt > 0)
+ mod_delayed_work(laundry_wq, &nn->nfsd_shrinker_work, 0);
+ return (unsigned long)cnt;
+}
+
+static unsigned long
+nfsd_courtesy_client_scan(struct shrinker *shrink, struct shrink_control *sc)
+{
+ return SHRINK_STOP;
+}
+
+int
+nfsd4_init_leases_net(struct nfsd_net *nn)
{
struct sysinfo si;
u64 max_clients;
@@ -4368,6 +4388,16 @@ void nfsd4_init_leases_net(struct nfsd_net *nn)
nn->nfs4_max_clients = max_t(int, max_clients, NFS4_CLIENTS_PER_GB);

atomic_set(&nn->nfsd_courtesy_clients, 0);
+ nn->nfsd_client_shrinker.scan_objects = nfsd_courtesy_client_scan;
+ nn->nfsd_client_shrinker.count_objects = nfsd_courtesy_client_count;
+ nn->nfsd_client_shrinker.seeks = DEFAULT_SEEKS;
+ return register_shrinker(&nn->nfsd_client_shrinker, "nfsd-client");
+}
+
+void
+nfsd4_leases_net_shutdown(struct nfsd_net *nn)
+{
+ unregister_shrinker(&nn->nfsd_client_shrinker);
}

static void init_nfs4_replay(struct nfs4_replay *rp)
@@ -5909,10 +5939,49 @@ nfs4_get_client_reaplist(struct nfsd_net *nn, struct list_head *reaplist,
spin_unlock(&nn->client_lock);
}

+static void
+nfs4_get_courtesy_client_reaplist(struct nfsd_net *nn,
+ struct list_head *reaplist)
+{
+ unsigned int maxreap = 0, reapcnt = 0;
+ struct list_head *pos, *next;
+ struct nfs4_client *clp;
+
+ maxreap = NFSD_CLIENT_MAX_TRIM_PER_RUN;
+ INIT_LIST_HEAD(reaplist);
+
+ spin_lock(&nn->client_lock);
+ list_for_each_safe(pos, next, &nn->client_lru) {
+ clp = list_entry(pos, struct nfs4_client, cl_lru);
+ if (clp->cl_state == NFSD4_ACTIVE)
+ break;
+ if (reapcnt >= maxreap)
+ break;
+ if (!mark_client_expired_locked(clp)) {
+ list_add(&clp->cl_lru, reaplist);
+ reapcnt++;
+ }
+ }
+ spin_unlock(&nn->client_lock);
+}
+
+static void
+nfs4_process_client_reaplist(struct list_head *reaplist)
+{
+ struct list_head *pos, *next;
+ struct nfs4_client *clp;
+
+ list_for_each_safe(pos, next, reaplist) {
+ clp = list_entry(pos, struct nfs4_client, cl_lru);
+ trace_nfsd_clid_purged(&clp->cl_clientid);
+ list_del_init(&clp->cl_lru);
+ expire_client(clp);
+ }
+}
+
static time64_t
nfs4_laundromat(struct nfsd_net *nn)
{
- struct nfs4_client *clp;
struct nfs4_openowner *oo;
struct nfs4_delegation *dp;
struct nfs4_ol_stateid *stp;
@@ -5941,12 +6010,8 @@ nfs4_laundromat(struct nfsd_net *nn)
}
spin_unlock(&nn->s2s_cp_lock);
nfs4_get_client_reaplist(nn, &reaplist, &lt);
- list_for_each_safe(pos, next, &reaplist) {
- clp = list_entry(pos, struct nfs4_client, cl_lru);
- trace_nfsd_clid_purged(&clp->cl_clientid);
- list_del_init(&clp->cl_lru);
- expire_client(clp);
- }
+ nfs4_process_client_reaplist(&reaplist);
+
spin_lock(&state_lock);
list_for_each_safe(pos, next, &nn->del_recall_lru) {
dp = list_entry (pos, struct nfs4_delegation, dl_recall_lru);
@@ -6029,6 +6094,18 @@ laundromat_main(struct work_struct *laundry)
queue_delayed_work(laundry_wq, &nn->laundromat_work, t*HZ);
}

+static void
+courtesy_client_reaper(struct work_struct *reaper)
+{
+ struct list_head reaplist;
+ struct delayed_work *dwork = to_delayed_work(reaper);
+ struct nfsd_net *nn = container_of(dwork, struct nfsd_net,
+ nfsd_shrinker_work);
+
+ nfs4_get_courtesy_client_reaplist(nn, &reaplist);
+ nfs4_process_client_reaplist(&reaplist);
+}
+
static inline __be32 nfs4_check_fh(struct svc_fh *fhp, struct nfs4_stid *stp)
{
if (!fh_match(&fhp->fh_handle, &stp->sc_file->fi_fhandle))
@@ -7845,6 +7922,7 @@ static int nfs4_state_create_net(struct net *net)
INIT_LIST_HEAD(&nn->blocked_locks_lru);

INIT_DELAYED_WORK(&nn->laundromat_work, laundromat_main);
+ INIT_DELAYED_WORK(&nn->nfsd_shrinker_work, courtesy_client_reaper);
get_net(net);

return 0;
diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 917fa1892fd2..597a26ad4183 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -1481,11 +1481,12 @@ static __net_init int nfsd_init_net(struct net *net)
goto out_idmap_error;
nn->nfsd_versions = NULL;
nn->nfsd4_minorversions = NULL;
+ retval = nfsd4_init_leases_net(nn);
+ if (retval)
+ goto out_drc_error;
retval = nfsd_reply_cache_init(nn);
if (retval)
goto out_drc_error;
- nfsd4_init_leases_net(nn);
-
get_random_bytes(&nn->siphash_key, sizeof(nn->siphash_key));
seqlock_init(&nn->writeverf_lock);

@@ -1507,6 +1508,7 @@ static __net_exit void nfsd_exit_net(struct net *net)
nfsd_idmap_shutdown(net);
nfsd_export_shutdown(net);
nfsd_netns_free_versions(net_generic(net, nfsd_net_id));
+ nfsd4_leases_net_shutdown(nn);
}

static struct pernet_operations nfsd_net_ops = {
diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
index 57a468ed85c3..cd92f615faa3 100644
--- a/fs/nfsd/nfsd.h
+++ b/fs/nfsd/nfsd.h
@@ -343,6 +343,7 @@ void nfsd_lockd_shutdown(void);
#define NFSD_COURTESY_CLIENT_TIMEOUT (24 * 60 * 60) /* seconds */
#define NFSD_CLIENT_MAX_TRIM_PER_RUN 128
#define NFS4_CLIENTS_PER_GB 1024
+#define NFSD_CLIENT_SHRINKER_MINTIMEOUT 1 /* seconds */

/*
* The following attributes are currently not supported by the NFSv4 server:
@@ -498,7 +499,8 @@ extern void unregister_cld_notifier(void);
extern void nfsd4_ssc_init_umount_work(struct nfsd_net *nn);
#endif

-extern void nfsd4_init_leases_net(struct nfsd_net *nn);
+extern int nfsd4_init_leases_net(struct nfsd_net *nn);
+extern void nfsd4_leases_net_shutdown(struct nfsd_net *nn);

#else /* CONFIG_NFSD_V4 */
static inline int nfsd4_is_junction(struct dentry *dentry)
@@ -506,7 +508,8 @@ static inline int nfsd4_is_junction(struct dentry *dentry)
return 0;
}

-static inline void nfsd4_init_leases_net(struct nfsd_net *nn) {};
+static inline int nfsd4_init_leases_net(struct nfsd_net *nn) { return 0; };
+static inline void nfsd4_leases_net_shutdown(struct nfsd_net *nn) {};

#define register_cld_notifier() 0
#define unregister_cld_notifier() do { } while(0)
--
2.9.5

2022-09-14 16:10:49

by Dai Ngo

[permalink] [raw]
Subject: [PATCH v7 1/2] NFSD: keep track of the number of courtesy clients in the system

Add counter nfs4_courtesy_client_count to nfsd_net to keep track
of the number of courtesy clients in the system.

Signed-off-by: Dai Ngo <[email protected]>
---
fs/nfsd/netns.h | 2 ++
fs/nfsd/nfs4state.c | 17 ++++++++++++++++-
2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
index ffe17743cc74..55c7006d6109 100644
--- a/fs/nfsd/netns.h
+++ b/fs/nfsd/netns.h
@@ -192,6 +192,8 @@ struct nfsd_net {

atomic_t nfs4_client_count;
int nfs4_max_clients;
+
+ atomic_t nfsd_courtesy_clients;
};

/* Simple check to find out if a given net was properly initialized */
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index c5d199d7e6b4..2827329704ea 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -160,6 +160,13 @@ static bool is_client_expired(struct nfs4_client *clp)
return clp->cl_time == 0;
}

+static void nfsd4_dec_courtesy_client_count(struct nfsd_net *nn,
+ struct nfs4_client *clp)
+{
+ if (clp->cl_state != NFSD4_ACTIVE)
+ atomic_add_unless(&nn->nfsd_courtesy_clients, -1, 0);
+}
+
static __be32 get_client_locked(struct nfs4_client *clp)
{
struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
@@ -169,6 +176,7 @@ static __be32 get_client_locked(struct nfs4_client *clp)
if (is_client_expired(clp))
return nfserr_expired;
atomic_inc(&clp->cl_rpc_users);
+ nfsd4_dec_courtesy_client_count(nn, clp);
clp->cl_state = NFSD4_ACTIVE;
return nfs_ok;
}
@@ -190,6 +198,7 @@ renew_client_locked(struct nfs4_client *clp)

list_move_tail(&clp->cl_lru, &nn->client_lru);
clp->cl_time = ktime_get_boottime_seconds();
+ nfsd4_dec_courtesy_client_count(nn, clp);
clp->cl_state = NFSD4_ACTIVE;
}

@@ -2233,6 +2242,7 @@ __destroy_client(struct nfs4_client *clp)
if (clp->cl_cb_conn.cb_xprt)
svc_xprt_put(clp->cl_cb_conn.cb_xprt);
atomic_add_unless(&nn->nfs4_client_count, -1, 0);
+ nfsd4_dec_courtesy_client_count(nn, clp);
free_client(clp);
wake_up_all(&expiry_wq);
}
@@ -4356,6 +4366,8 @@ void nfsd4_init_leases_net(struct nfsd_net *nn)
max_clients = (u64)si.totalram * si.mem_unit / (1024 * 1024 * 1024);
max_clients *= NFS4_CLIENTS_PER_GB;
nn->nfs4_max_clients = max_t(int, max_clients, NFS4_CLIENTS_PER_GB);
+
+ atomic_set(&nn->nfsd_courtesy_clients, 0);
}

static void init_nfs4_replay(struct nfs4_replay *rp)
@@ -5878,8 +5890,11 @@ nfs4_get_client_reaplist(struct nfsd_net *nn, struct list_head *reaplist,
goto exp_client;
if (!state_expired(lt, clp->cl_time))
break;
- if (!atomic_read(&clp->cl_rpc_users))
+ if (!atomic_read(&clp->cl_rpc_users)) {
+ if (clp->cl_state == NFSD4_ACTIVE)
+ atomic_inc(&nn->nfsd_courtesy_clients);
clp->cl_state = NFSD4_COURTESY;
+ }
if (!client_has_state(clp))
goto exp_client;
if (!nfs4_anylock_blockers(clp))
--
2.9.5

2022-09-14 18:42:12

by Chuck Lever

[permalink] [raw]
Subject: Re: [PATCH v7 2/2] NFSD: add shrinker to reap courtesy clients on low memory condition



> On Sep 14, 2022, at 8:54 AM, Dai Ngo <[email protected]> wrote:
>
> Add courtesy_client_reaper to react to low memory condition triggered
> by the system memory shrinker.
>
> The delayed_work for the courtesy_client_reaper is scheduled on
> the shrinker's count callback using the laundry_wq.
>
> The shrinker's scan callback is not used for expiring the courtesy
> clients due to potential deadlocks.
>
> Signed-off-by: Dai Ngo <[email protected]>
> ---
> fs/nfsd/netns.h | 2 ++
> fs/nfsd/nfs4state.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++-----
> fs/nfsd/nfsctl.c | 6 ++--
> fs/nfsd/nfsd.h | 7 ++--
> 4 files changed, 97 insertions(+), 12 deletions(-)
>
> diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
> index 55c7006d6109..8c854ba3285b 100644
> --- a/fs/nfsd/netns.h
> +++ b/fs/nfsd/netns.h
> @@ -194,6 +194,8 @@ struct nfsd_net {
> int nfs4_max_clients;
>
> atomic_t nfsd_courtesy_clients;
> + struct shrinker nfsd_client_shrinker;
> + struct delayed_work nfsd_shrinker_work;
> };
>
> /* Simple check to find out if a given net was properly initialized */
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 2827329704ea..62b848bb55df 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -4347,7 +4347,27 @@ nfsd4_init_slabs(void)
> return -ENOMEM;
> }
>
> -void nfsd4_init_leases_net(struct nfsd_net *nn)
> +static unsigned long
> +nfsd_courtesy_client_count(struct shrinker *shrink, struct shrink_control *sc)
> +{
> + int cnt;
> + struct nfsd_net *nn = container_of(shrink,
> + struct nfsd_net, nfsd_client_shrinker);
> +
> + cnt = atomic_read(&nn->nfsd_courtesy_clients);
> + if (cnt > 0)
> + mod_delayed_work(laundry_wq, &nn->nfsd_shrinker_work, 0);
> + return (unsigned long)cnt;
> +}
> +
> +static unsigned long
> +nfsd_courtesy_client_scan(struct shrinker *shrink, struct shrink_control *sc)
> +{
> + return SHRINK_STOP;
> +}
> +
> +int
> +nfsd4_init_leases_net(struct nfsd_net *nn)
> {
> struct sysinfo si;
> u64 max_clients;
> @@ -4368,6 +4388,16 @@ void nfsd4_init_leases_net(struct nfsd_net *nn)
> nn->nfs4_max_clients = max_t(int, max_clients, NFS4_CLIENTS_PER_GB);
>
> atomic_set(&nn->nfsd_courtesy_clients, 0);
> + nn->nfsd_client_shrinker.scan_objects = nfsd_courtesy_client_scan;
> + nn->nfsd_client_shrinker.count_objects = nfsd_courtesy_client_count;
> + nn->nfsd_client_shrinker.seeks = DEFAULT_SEEKS;
> + return register_shrinker(&nn->nfsd_client_shrinker, "nfsd-client");
> +}
> +
> +void
> +nfsd4_leases_net_shutdown(struct nfsd_net *nn)
> +{
> + unregister_shrinker(&nn->nfsd_client_shrinker);
> }
>
> static void init_nfs4_replay(struct nfs4_replay *rp)
> @@ -5909,10 +5939,49 @@ nfs4_get_client_reaplist(struct nfsd_net *nn, struct list_head *reaplist,
> spin_unlock(&nn->client_lock);
> }
>
> +static void
> +nfs4_get_courtesy_client_reaplist(struct nfsd_net *nn,
> + struct list_head *reaplist)
> +{
> + unsigned int maxreap = 0, reapcnt = 0;
> + struct list_head *pos, *next;
> + struct nfs4_client *clp;
> +
> + maxreap = NFSD_CLIENT_MAX_TRIM_PER_RUN;
> + INIT_LIST_HEAD(reaplist);
> +
> + spin_lock(&nn->client_lock);
> + list_for_each_safe(pos, next, &nn->client_lru) {
> + clp = list_entry(pos, struct nfs4_client, cl_lru);
> + if (clp->cl_state == NFSD4_ACTIVE)
> + break;
> + if (reapcnt >= maxreap)
> + break;
> + if (!mark_client_expired_locked(clp)) {
> + list_add(&clp->cl_lru, reaplist);
> + reapcnt++;
> + }
> + }
> + spin_unlock(&nn->client_lock);
> +}
> +
> +static void
> +nfs4_process_client_reaplist(struct list_head *reaplist)
> +{
> + struct list_head *pos, *next;
> + struct nfs4_client *clp;
> +
> + list_for_each_safe(pos, next, reaplist) {
> + clp = list_entry(pos, struct nfs4_client, cl_lru);
> + trace_nfsd_clid_purged(&clp->cl_clientid);
> + list_del_init(&clp->cl_lru);
> + expire_client(clp);
> + }
> +}
> +
> static time64_t
> nfs4_laundromat(struct nfsd_net *nn)
> {
> - struct nfs4_client *clp;
> struct nfs4_openowner *oo;
> struct nfs4_delegation *dp;
> struct nfs4_ol_stateid *stp;
> @@ -5941,12 +6010,8 @@ nfs4_laundromat(struct nfsd_net *nn)
> }
> spin_unlock(&nn->s2s_cp_lock);
> nfs4_get_client_reaplist(nn, &reaplist, &lt);
> - list_for_each_safe(pos, next, &reaplist) {
> - clp = list_entry(pos, struct nfs4_client, cl_lru);
> - trace_nfsd_clid_purged(&clp->cl_clientid);
> - list_del_init(&clp->cl_lru);
> - expire_client(clp);
> - }
> + nfs4_process_client_reaplist(&reaplist);
> +
> spin_lock(&state_lock);
> list_for_each_safe(pos, next, &nn->del_recall_lru) {
> dp = list_entry (pos, struct nfs4_delegation, dl_recall_lru);
> @@ -6029,6 +6094,18 @@ laundromat_main(struct work_struct *laundry)
> queue_delayed_work(laundry_wq, &nn->laundromat_work, t*HZ);
> }
>
> +static void
> +courtesy_client_reaper(struct work_struct *reaper)
> +{
> + struct list_head reaplist;
> + struct delayed_work *dwork = to_delayed_work(reaper);
> + struct nfsd_net *nn = container_of(dwork, struct nfsd_net,
> + nfsd_shrinker_work);
> +
> + nfs4_get_courtesy_client_reaplist(nn, &reaplist);
> + nfs4_process_client_reaplist(&reaplist);
> +}
> +
> static inline __be32 nfs4_check_fh(struct svc_fh *fhp, struct nfs4_stid *stp)
> {
> if (!fh_match(&fhp->fh_handle, &stp->sc_file->fi_fhandle))
> @@ -7845,6 +7922,7 @@ static int nfs4_state_create_net(struct net *net)
> INIT_LIST_HEAD(&nn->blocked_locks_lru);
>
> INIT_DELAYED_WORK(&nn->laundromat_work, laundromat_main);
> + INIT_DELAYED_WORK(&nn->nfsd_shrinker_work, courtesy_client_reaper);
> get_net(net);
>
> return 0;
> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> index 917fa1892fd2..597a26ad4183 100644
> --- a/fs/nfsd/nfsctl.c
> +++ b/fs/nfsd/nfsctl.c
> @@ -1481,11 +1481,12 @@ static __net_init int nfsd_init_net(struct net *net)
> goto out_idmap_error;
> nn->nfsd_versions = NULL;
> nn->nfsd4_minorversions = NULL;
> + retval = nfsd4_init_leases_net(nn);
> + if (retval)
> + goto out_drc_error;
> retval = nfsd_reply_cache_init(nn);
> if (retval)
> goto out_drc_error;
> - nfsd4_init_leases_net(nn);
> -
> get_random_bytes(&nn->siphash_key, sizeof(nn->siphash_key));
> seqlock_init(&nn->writeverf_lock);
>
> @@ -1507,6 +1508,7 @@ static __net_exit void nfsd_exit_net(struct net *net)
> nfsd_idmap_shutdown(net);
> nfsd_export_shutdown(net);
> nfsd_netns_free_versions(net_generic(net, nfsd_net_id));
> + nfsd4_leases_net_shutdown(nn);
> }
>
> static struct pernet_operations nfsd_net_ops = {
> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
> index 57a468ed85c3..cd92f615faa3 100644
> --- a/fs/nfsd/nfsd.h
> +++ b/fs/nfsd/nfsd.h
> @@ -343,6 +343,7 @@ void nfsd_lockd_shutdown(void);
> #define NFSD_COURTESY_CLIENT_TIMEOUT (24 * 60 * 60) /* seconds */
> #define NFSD_CLIENT_MAX_TRIM_PER_RUN 128
> #define NFS4_CLIENTS_PER_GB 1024
> +#define NFSD_CLIENT_SHRINKER_MINTIMEOUT 1 /* seconds */

You don't need this definition any more. I can remove it
when I apply the patch.

Otherwise, these patches look great. I will give a few
more days for more review comments.


> /*
> * The following attributes are currently not supported by the NFSv4 server:
> @@ -498,7 +499,8 @@ extern void unregister_cld_notifier(void);
> extern void nfsd4_ssc_init_umount_work(struct nfsd_net *nn);
> #endif
>
> -extern void nfsd4_init_leases_net(struct nfsd_net *nn);
> +extern int nfsd4_init_leases_net(struct nfsd_net *nn);
> +extern void nfsd4_leases_net_shutdown(struct nfsd_net *nn);
>
> #else /* CONFIG_NFSD_V4 */
> static inline int nfsd4_is_junction(struct dentry *dentry)
> @@ -506,7 +508,8 @@ static inline int nfsd4_is_junction(struct dentry *dentry)
> return 0;
> }
>
> -static inline void nfsd4_init_leases_net(struct nfsd_net *nn) {};
> +static inline int nfsd4_init_leases_net(struct nfsd_net *nn) { return 0; };
> +static inline void nfsd4_leases_net_shutdown(struct nfsd_net *nn) {};
>
> #define register_cld_notifier() 0
> #define unregister_cld_notifier() do { } while(0)
> --
> 2.9.5
>

--
Chuck Lever



2022-09-14 18:52:19

by Dai Ngo

[permalink] [raw]
Subject: Re: [PATCH v7 2/2] NFSD: add shrinker to reap courtesy clients on low memory condition


On 9/14/22 11:32 AM, Chuck Lever III wrote:
>
>> On Sep 14, 2022, at 8:54 AM, Dai Ngo <[email protected]> wrote:
>>
>> Add courtesy_client_reaper to react to low memory condition triggered
>> by the system memory shrinker.
>>
>> The delayed_work for the courtesy_client_reaper is scheduled on
>> the shrinker's count callback using the laundry_wq.
>>
>> The shrinker's scan callback is not used for expiring the courtesy
>> clients due to potential deadlocks.
>>
>> Signed-off-by: Dai Ngo <[email protected]>
>> ---
>> fs/nfsd/netns.h | 2 ++
>> fs/nfsd/nfs4state.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++-----
>> fs/nfsd/nfsctl.c | 6 ++--
>> fs/nfsd/nfsd.h | 7 ++--
>> 4 files changed, 97 insertions(+), 12 deletions(-)
>>
>> diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
>> index 55c7006d6109..8c854ba3285b 100644
>> --- a/fs/nfsd/netns.h
>> +++ b/fs/nfsd/netns.h
>> @@ -194,6 +194,8 @@ struct nfsd_net {
>> int nfs4_max_clients;
>>
>> atomic_t nfsd_courtesy_clients;
>> + struct shrinker nfsd_client_shrinker;
>> + struct delayed_work nfsd_shrinker_work;
>> };
>>
>> /* Simple check to find out if a given net was properly initialized */
>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>> index 2827329704ea..62b848bb55df 100644
>> --- a/fs/nfsd/nfs4state.c
>> +++ b/fs/nfsd/nfs4state.c
>> @@ -4347,7 +4347,27 @@ nfsd4_init_slabs(void)
>> return -ENOMEM;
>> }
>>
>> -void nfsd4_init_leases_net(struct nfsd_net *nn)
>> +static unsigned long
>> +nfsd_courtesy_client_count(struct shrinker *shrink, struct shrink_control *sc)
>> +{
>> + int cnt;
>> + struct nfsd_net *nn = container_of(shrink,
>> + struct nfsd_net, nfsd_client_shrinker);
>> +
>> + cnt = atomic_read(&nn->nfsd_courtesy_clients);
>> + if (cnt > 0)
>> + mod_delayed_work(laundry_wq, &nn->nfsd_shrinker_work, 0);
>> + return (unsigned long)cnt;
>> +}
>> +
>> +static unsigned long
>> +nfsd_courtesy_client_scan(struct shrinker *shrink, struct shrink_control *sc)
>> +{
>> + return SHRINK_STOP;
>> +}
>> +
>> +int
>> +nfsd4_init_leases_net(struct nfsd_net *nn)
>> {
>> struct sysinfo si;
>> u64 max_clients;
>> @@ -4368,6 +4388,16 @@ void nfsd4_init_leases_net(struct nfsd_net *nn)
>> nn->nfs4_max_clients = max_t(int, max_clients, NFS4_CLIENTS_PER_GB);
>>
>> atomic_set(&nn->nfsd_courtesy_clients, 0);
>> + nn->nfsd_client_shrinker.scan_objects = nfsd_courtesy_client_scan;
>> + nn->nfsd_client_shrinker.count_objects = nfsd_courtesy_client_count;
>> + nn->nfsd_client_shrinker.seeks = DEFAULT_SEEKS;
>> + return register_shrinker(&nn->nfsd_client_shrinker, "nfsd-client");
>> +}
>> +
>> +void
>> +nfsd4_leases_net_shutdown(struct nfsd_net *nn)
>> +{
>> + unregister_shrinker(&nn->nfsd_client_shrinker);
>> }
>>
>> static void init_nfs4_replay(struct nfs4_replay *rp)
>> @@ -5909,10 +5939,49 @@ nfs4_get_client_reaplist(struct nfsd_net *nn, struct list_head *reaplist,
>> spin_unlock(&nn->client_lock);
>> }
>>
>> +static void
>> +nfs4_get_courtesy_client_reaplist(struct nfsd_net *nn,
>> + struct list_head *reaplist)
>> +{
>> + unsigned int maxreap = 0, reapcnt = 0;
>> + struct list_head *pos, *next;
>> + struct nfs4_client *clp;
>> +
>> + maxreap = NFSD_CLIENT_MAX_TRIM_PER_RUN;
>> + INIT_LIST_HEAD(reaplist);
>> +
>> + spin_lock(&nn->client_lock);
>> + list_for_each_safe(pos, next, &nn->client_lru) {
>> + clp = list_entry(pos, struct nfs4_client, cl_lru);
>> + if (clp->cl_state == NFSD4_ACTIVE)
>> + break;
>> + if (reapcnt >= maxreap)
>> + break;
>> + if (!mark_client_expired_locked(clp)) {
>> + list_add(&clp->cl_lru, reaplist);
>> + reapcnt++;
>> + }
>> + }
>> + spin_unlock(&nn->client_lock);
>> +}
>> +
>> +static void
>> +nfs4_process_client_reaplist(struct list_head *reaplist)
>> +{
>> + struct list_head *pos, *next;
>> + struct nfs4_client *clp;
>> +
>> + list_for_each_safe(pos, next, reaplist) {
>> + clp = list_entry(pos, struct nfs4_client, cl_lru);
>> + trace_nfsd_clid_purged(&clp->cl_clientid);
>> + list_del_init(&clp->cl_lru);
>> + expire_client(clp);
>> + }
>> +}
>> +
>> static time64_t
>> nfs4_laundromat(struct nfsd_net *nn)
>> {
>> - struct nfs4_client *clp;
>> struct nfs4_openowner *oo;
>> struct nfs4_delegation *dp;
>> struct nfs4_ol_stateid *stp;
>> @@ -5941,12 +6010,8 @@ nfs4_laundromat(struct nfsd_net *nn)
>> }
>> spin_unlock(&nn->s2s_cp_lock);
>> nfs4_get_client_reaplist(nn, &reaplist, &lt);
>> - list_for_each_safe(pos, next, &reaplist) {
>> - clp = list_entry(pos, struct nfs4_client, cl_lru);
>> - trace_nfsd_clid_purged(&clp->cl_clientid);
>> - list_del_init(&clp->cl_lru);
>> - expire_client(clp);
>> - }
>> + nfs4_process_client_reaplist(&reaplist);
>> +
>> spin_lock(&state_lock);
>> list_for_each_safe(pos, next, &nn->del_recall_lru) {
>> dp = list_entry (pos, struct nfs4_delegation, dl_recall_lru);
>> @@ -6029,6 +6094,18 @@ laundromat_main(struct work_struct *laundry)
>> queue_delayed_work(laundry_wq, &nn->laundromat_work, t*HZ);
>> }
>>
>> +static void
>> +courtesy_client_reaper(struct work_struct *reaper)
>> +{
>> + struct list_head reaplist;
>> + struct delayed_work *dwork = to_delayed_work(reaper);
>> + struct nfsd_net *nn = container_of(dwork, struct nfsd_net,
>> + nfsd_shrinker_work);
>> +
>> + nfs4_get_courtesy_client_reaplist(nn, &reaplist);
>> + nfs4_process_client_reaplist(&reaplist);
>> +}
>> +
>> static inline __be32 nfs4_check_fh(struct svc_fh *fhp, struct nfs4_stid *stp)
>> {
>> if (!fh_match(&fhp->fh_handle, &stp->sc_file->fi_fhandle))
>> @@ -7845,6 +7922,7 @@ static int nfs4_state_create_net(struct net *net)
>> INIT_LIST_HEAD(&nn->blocked_locks_lru);
>>
>> INIT_DELAYED_WORK(&nn->laundromat_work, laundromat_main);
>> + INIT_DELAYED_WORK(&nn->nfsd_shrinker_work, courtesy_client_reaper);
>> get_net(net);
>>
>> return 0;
>> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
>> index 917fa1892fd2..597a26ad4183 100644
>> --- a/fs/nfsd/nfsctl.c
>> +++ b/fs/nfsd/nfsctl.c
>> @@ -1481,11 +1481,12 @@ static __net_init int nfsd_init_net(struct net *net)
>> goto out_idmap_error;
>> nn->nfsd_versions = NULL;
>> nn->nfsd4_minorversions = NULL;
>> + retval = nfsd4_init_leases_net(nn);
>> + if (retval)
>> + goto out_drc_error;
>> retval = nfsd_reply_cache_init(nn);
>> if (retval)
>> goto out_drc_error;
>> - nfsd4_init_leases_net(nn);
>> -
>> get_random_bytes(&nn->siphash_key, sizeof(nn->siphash_key));
>> seqlock_init(&nn->writeverf_lock);
>>
>> @@ -1507,6 +1508,7 @@ static __net_exit void nfsd_exit_net(struct net *net)
>> nfsd_idmap_shutdown(net);
>> nfsd_export_shutdown(net);
>> nfsd_netns_free_versions(net_generic(net, nfsd_net_id));
>> + nfsd4_leases_net_shutdown(nn);
>> }
>>
>> static struct pernet_operations nfsd_net_ops = {
>> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
>> index 57a468ed85c3..cd92f615faa3 100644
>> --- a/fs/nfsd/nfsd.h
>> +++ b/fs/nfsd/nfsd.h
>> @@ -343,6 +343,7 @@ void nfsd_lockd_shutdown(void);
>> #define NFSD_COURTESY_CLIENT_TIMEOUT (24 * 60 * 60) /* seconds */
>> #define NFSD_CLIENT_MAX_TRIM_PER_RUN 128
>> #define NFS4_CLIENTS_PER_GB 1024
>> +#define NFSD_CLIENT_SHRINKER_MINTIMEOUT 1 /* seconds */
> You don't need this definition any more. I can remove it
> when I apply the patch.

Oh yes, I missed this.

>
> Otherwise, these patches look great. I will give a few
> more days for more review comments.

Thank you Chuck,

-Dai

>
>
>> /*
>> * The following attributes are currently not supported by the NFSv4 server:
>> @@ -498,7 +499,8 @@ extern void unregister_cld_notifier(void);
>> extern void nfsd4_ssc_init_umount_work(struct nfsd_net *nn);
>> #endif
>>
>> -extern void nfsd4_init_leases_net(struct nfsd_net *nn);
>> +extern int nfsd4_init_leases_net(struct nfsd_net *nn);
>> +extern void nfsd4_leases_net_shutdown(struct nfsd_net *nn);
>>
>> #else /* CONFIG_NFSD_V4 */
>> static inline int nfsd4_is_junction(struct dentry *dentry)
>> @@ -506,7 +508,8 @@ static inline int nfsd4_is_junction(struct dentry *dentry)
>> return 0;
>> }
>>
>> -static inline void nfsd4_init_leases_net(struct nfsd_net *nn) {};
>> +static inline int nfsd4_init_leases_net(struct nfsd_net *nn) { return 0; };
>> +static inline void nfsd4_leases_net_shutdown(struct nfsd_net *nn) {};
>>
>> #define register_cld_notifier() 0
>> #define unregister_cld_notifier() do { } while(0)
>> --
>> 2.9.5
>>
> --
> Chuck Lever
>
>
>