* [PATCH v2 1/1] NFSD: register/unregister of nfsd-client shrinker at nfsd startup/shutdown time
@ 2023-01-10 18:27 Dai Ngo
0 siblings, 0 replies; 3+ messages in thread
From: Dai Ngo @ 2023-01-10 18:27 UTC (permalink / raw)
To: chuck.lever, jlayton; +Cc: linux-nfs
Currently the nfsd-client shrinker is registered and unregistered at
the time the nfsd module is loaded and unloaded. This means after the
nfsd service is shutdown, the nfsd-client shrinker is still registered
in the system. This causes the nfsd-client shrinker to be called when
memory is low even thought nfsd service is not running. This is also
true for the nfsd_reply_cache_shrinker.
This patch moves the register/unregister of nfsd-client shrinker from
module load/unload time to nfsd startup/shutdown time.
Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
---
v2: handle error in nfs4_state_create_net when register_shrinker fails.
fs/nfsd/nfs4state.c | 22 +++++++++++-----------
fs/nfsd/nfsctl.c | 7 +------
fs/nfsd/nfsd.h | 6 ++----
3 files changed, 14 insertions(+), 21 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 7b2ee535ade8..a7cfefd7c205 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4421,7 +4421,7 @@ nfsd4_state_shrinker_scan(struct shrinker *shrink, struct shrink_control *sc)
return SHRINK_STOP;
}
-int
+void
nfsd4_init_leases_net(struct nfsd_net *nn)
{
struct sysinfo si;
@@ -4443,16 +4443,6 @@ 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 = nfsd4_state_shrinker_scan;
- nn->nfsd_client_shrinker.count_objects = nfsd4_state_shrinker_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)
@@ -8077,8 +8067,17 @@ static int nfs4_state_create_net(struct net *net)
INIT_DELAYED_WORK(&nn->nfsd_shrinker_work, nfsd4_state_shrinker_worker);
get_net(net);
+ nn->nfsd_client_shrinker.scan_objects = nfsd4_state_shrinker_scan;
+ nn->nfsd_client_shrinker.count_objects = nfsd4_state_shrinker_count;
+ nn->nfsd_client_shrinker.seeks = DEFAULT_SEEKS;
+
+ if (register_shrinker(&nn->nfsd_client_shrinker, "nfsd-client"))
+ goto err_shrinker;
return 0;
+err_shrinker:
+ put_net(net);
+ kfree(nn->sessionid_hashtbl);
err_sessionid:
kfree(nn->unconf_id_hashtbl);
err_unconf_id:
@@ -8171,6 +8170,7 @@ nfs4_state_shutdown_net(struct net *net)
struct list_head *pos, *next, reaplist;
struct nfsd_net *nn = net_generic(net, nfsd_net_id);
+ unregister_shrinker(&nn->nfsd_client_shrinker);
cancel_delayed_work_sync(&nn->laundromat_work);
locks_end_grace(&nn->nfsd4_manager);
diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index d1e581a60480..c2577ee7ffb2 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -1457,9 +1457,7 @@ 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;
+ nfsd4_init_leases_net(nn);
retval = nfsd_reply_cache_init(nn);
if (retval)
goto out_cache_error;
@@ -1469,8 +1467,6 @@ static __net_init int nfsd_init_net(struct net *net)
return 0;
out_cache_error:
- nfsd4_leases_net_shutdown(nn);
-out_drc_error:
nfsd_idmap_shutdown(net);
out_idmap_error:
nfsd_export_shutdown(net);
@@ -1486,7 +1482,6 @@ 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 93b42ef9ed91..fa0144a74267 100644
--- a/fs/nfsd/nfsd.h
+++ b/fs/nfsd/nfsd.h
@@ -504,8 +504,7 @@ extern void unregister_cld_notifier(void);
extern void nfsd4_ssc_init_umount_work(struct nfsd_net *nn);
#endif
-extern int nfsd4_init_leases_net(struct nfsd_net *nn);
-extern void nfsd4_leases_net_shutdown(struct nfsd_net *nn);
+extern void nfsd4_init_leases_net(struct nfsd_net *nn);
#else /* CONFIG_NFSD_V4 */
static inline int nfsd4_is_junction(struct dentry *dentry)
@@ -513,8 +512,7 @@ static inline int nfsd4_is_junction(struct dentry *dentry)
return 0;
}
-static inline int nfsd4_init_leases_net(struct nfsd_net *nn) { return 0; };
-static inline void nfsd4_leases_net_shutdown(struct nfsd_net *nn) {};
+static inline void nfsd4_init_leases_net(struct nfsd_net *nn) { };
#define register_cld_notifier() 0
#define unregister_cld_notifier() do { } while(0)
--
2.9.5
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH v2 1/1] NFSD: register/unregister of nfsd-client shrinker at nfsd startup/shutdown time
@ 2023-01-11 20:17 Dai Ngo
2023-01-12 2:38 ` Chuck Lever III
0 siblings, 1 reply; 3+ messages in thread
From: Dai Ngo @ 2023-01-11 20:17 UTC (permalink / raw)
To: chuck.lever, jlayton; +Cc: efault, linux-nfs
Currently the nfsd-client shrinker is registered and unregistered at
the time the nfsd module is loaded and unloaded. The problem with this
is the shrinker is being registered before all of the relevant fields
in nfsd_net are initialized when nfsd is started. This can lead to an
oops when memory is low and the shrinker is called while nfsd is not
running.
This patch moves the register/unregister of nfsd-client shrinker from
module load/unload time to nfsd startup/shutdown time.
Fixes: 44df6f439a17 ("NFSD: add delegation reaper to react to low memory condition")
Reported-by: Mike Galbraith <efault@gmx.de>
Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
---
v2: reword change log for better description of the problem
fs/nfsd/nfs4state.c | 22 +++++++++++-----------
fs/nfsd/nfsctl.c | 7 +------
fs/nfsd/nfsd.h | 6 ++----
3 files changed, 14 insertions(+), 21 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 7b2ee535ade8..a7cfefd7c205 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4421,7 +4421,7 @@ nfsd4_state_shrinker_scan(struct shrinker *shrink, struct shrink_control *sc)
return SHRINK_STOP;
}
-int
+void
nfsd4_init_leases_net(struct nfsd_net *nn)
{
struct sysinfo si;
@@ -4443,16 +4443,6 @@ 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 = nfsd4_state_shrinker_scan;
- nn->nfsd_client_shrinker.count_objects = nfsd4_state_shrinker_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)
@@ -8077,8 +8067,17 @@ static int nfs4_state_create_net(struct net *net)
INIT_DELAYED_WORK(&nn->nfsd_shrinker_work, nfsd4_state_shrinker_worker);
get_net(net);
+ nn->nfsd_client_shrinker.scan_objects = nfsd4_state_shrinker_scan;
+ nn->nfsd_client_shrinker.count_objects = nfsd4_state_shrinker_count;
+ nn->nfsd_client_shrinker.seeks = DEFAULT_SEEKS;
+
+ if (register_shrinker(&nn->nfsd_client_shrinker, "nfsd-client"))
+ goto err_shrinker;
return 0;
+err_shrinker:
+ put_net(net);
+ kfree(nn->sessionid_hashtbl);
err_sessionid:
kfree(nn->unconf_id_hashtbl);
err_unconf_id:
@@ -8171,6 +8170,7 @@ nfs4_state_shutdown_net(struct net *net)
struct list_head *pos, *next, reaplist;
struct nfsd_net *nn = net_generic(net, nfsd_net_id);
+ unregister_shrinker(&nn->nfsd_client_shrinker);
cancel_delayed_work_sync(&nn->laundromat_work);
locks_end_grace(&nn->nfsd4_manager);
diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index d1e581a60480..c2577ee7ffb2 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -1457,9 +1457,7 @@ 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;
+ nfsd4_init_leases_net(nn);
retval = nfsd_reply_cache_init(nn);
if (retval)
goto out_cache_error;
@@ -1469,8 +1467,6 @@ static __net_init int nfsd_init_net(struct net *net)
return 0;
out_cache_error:
- nfsd4_leases_net_shutdown(nn);
-out_drc_error:
nfsd_idmap_shutdown(net);
out_idmap_error:
nfsd_export_shutdown(net);
@@ -1486,7 +1482,6 @@ 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 93b42ef9ed91..fa0144a74267 100644
--- a/fs/nfsd/nfsd.h
+++ b/fs/nfsd/nfsd.h
@@ -504,8 +504,7 @@ extern void unregister_cld_notifier(void);
extern void nfsd4_ssc_init_umount_work(struct nfsd_net *nn);
#endif
-extern int nfsd4_init_leases_net(struct nfsd_net *nn);
-extern void nfsd4_leases_net_shutdown(struct nfsd_net *nn);
+extern void nfsd4_init_leases_net(struct nfsd_net *nn);
#else /* CONFIG_NFSD_V4 */
static inline int nfsd4_is_junction(struct dentry *dentry)
@@ -513,8 +512,7 @@ static inline int nfsd4_is_junction(struct dentry *dentry)
return 0;
}
-static inline int nfsd4_init_leases_net(struct nfsd_net *nn) { return 0; };
-static inline void nfsd4_leases_net_shutdown(struct nfsd_net *nn) {};
+static inline void nfsd4_init_leases_net(struct nfsd_net *nn) { };
#define register_cld_notifier() 0
#define unregister_cld_notifier() do { } while(0)
--
2.9.5
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2 1/1] NFSD: register/unregister of nfsd-client shrinker at nfsd startup/shutdown time
2023-01-11 20:17 Dai Ngo
@ 2023-01-12 2:38 ` Chuck Lever III
0 siblings, 0 replies; 3+ messages in thread
From: Chuck Lever III @ 2023-01-12 2:38 UTC (permalink / raw)
To: Dai Ngo; +Cc: Jeff Layton, efault@gmx.de, Linux NFS Mailing List
> On Jan 11, 2023, at 3:17 PM, Dai Ngo <dai.ngo@oracle.com> wrote:
>
> Currently the nfsd-client shrinker is registered and unregistered at
> the time the nfsd module is loaded and unloaded. The problem with this
> is the shrinker is being registered before all of the relevant fields
> in nfsd_net are initialized when nfsd is started. This can lead to an
> oops when memory is low and the shrinker is called while nfsd is not
> running.
>
> This patch moves the register/unregister of nfsd-client shrinker from
> module load/unload time to nfsd startup/shutdown time.
>
> Fixes: 44df6f439a17 ("NFSD: add delegation reaper to react to low memory condition")
> Reported-by: Mike Galbraith <efault@gmx.de>
> Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
> ---
> v2: reword change log for better description of the problem
Hello Dai, I've applied this to nfsd's for-rc, and expect to send
it along to Linus in a few days. Thanks for seeing it through!
> fs/nfsd/nfs4state.c | 22 +++++++++++-----------
> fs/nfsd/nfsctl.c | 7 +------
> fs/nfsd/nfsd.h | 6 ++----
> 3 files changed, 14 insertions(+), 21 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 7b2ee535ade8..a7cfefd7c205 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -4421,7 +4421,7 @@ nfsd4_state_shrinker_scan(struct shrinker *shrink, struct shrink_control *sc)
> return SHRINK_STOP;
> }
>
> -int
> +void
> nfsd4_init_leases_net(struct nfsd_net *nn)
> {
> struct sysinfo si;
> @@ -4443,16 +4443,6 @@ 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 = nfsd4_state_shrinker_scan;
> - nn->nfsd_client_shrinker.count_objects = nfsd4_state_shrinker_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)
> @@ -8077,8 +8067,17 @@ static int nfs4_state_create_net(struct net *net)
> INIT_DELAYED_WORK(&nn->nfsd_shrinker_work, nfsd4_state_shrinker_worker);
> get_net(net);
>
> + nn->nfsd_client_shrinker.scan_objects = nfsd4_state_shrinker_scan;
> + nn->nfsd_client_shrinker.count_objects = nfsd4_state_shrinker_count;
> + nn->nfsd_client_shrinker.seeks = DEFAULT_SEEKS;
> +
> + if (register_shrinker(&nn->nfsd_client_shrinker, "nfsd-client"))
> + goto err_shrinker;
> return 0;
>
> +err_shrinker:
> + put_net(net);
> + kfree(nn->sessionid_hashtbl);
> err_sessionid:
> kfree(nn->unconf_id_hashtbl);
> err_unconf_id:
> @@ -8171,6 +8170,7 @@ nfs4_state_shutdown_net(struct net *net)
> struct list_head *pos, *next, reaplist;
> struct nfsd_net *nn = net_generic(net, nfsd_net_id);
>
> + unregister_shrinker(&nn->nfsd_client_shrinker);
> cancel_delayed_work_sync(&nn->laundromat_work);
> locks_end_grace(&nn->nfsd4_manager);
>
> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> index d1e581a60480..c2577ee7ffb2 100644
> --- a/fs/nfsd/nfsctl.c
> +++ b/fs/nfsd/nfsctl.c
> @@ -1457,9 +1457,7 @@ 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;
> + nfsd4_init_leases_net(nn);
> retval = nfsd_reply_cache_init(nn);
> if (retval)
> goto out_cache_error;
> @@ -1469,8 +1467,6 @@ static __net_init int nfsd_init_net(struct net *net)
> return 0;
>
> out_cache_error:
> - nfsd4_leases_net_shutdown(nn);
> -out_drc_error:
> nfsd_idmap_shutdown(net);
> out_idmap_error:
> nfsd_export_shutdown(net);
> @@ -1486,7 +1482,6 @@ 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 93b42ef9ed91..fa0144a74267 100644
> --- a/fs/nfsd/nfsd.h
> +++ b/fs/nfsd/nfsd.h
> @@ -504,8 +504,7 @@ extern void unregister_cld_notifier(void);
> extern void nfsd4_ssc_init_umount_work(struct nfsd_net *nn);
> #endif
>
> -extern int nfsd4_init_leases_net(struct nfsd_net *nn);
> -extern void nfsd4_leases_net_shutdown(struct nfsd_net *nn);
> +extern void nfsd4_init_leases_net(struct nfsd_net *nn);
>
> #else /* CONFIG_NFSD_V4 */
> static inline int nfsd4_is_junction(struct dentry *dentry)
> @@ -513,8 +512,7 @@ static inline int nfsd4_is_junction(struct dentry *dentry)
> return 0;
> }
>
> -static inline int nfsd4_init_leases_net(struct nfsd_net *nn) { return 0; };
> -static inline void nfsd4_leases_net_shutdown(struct nfsd_net *nn) {};
> +static inline void nfsd4_init_leases_net(struct nfsd_net *nn) { };
>
> #define register_cld_notifier() 0
> #define unregister_cld_notifier() do { } while(0)
> --
> 2.9.5
>
--
Chuck Lever
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-01-12 2:38 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-10 18:27 [PATCH v2 1/1] NFSD: register/unregister of nfsd-client shrinker at nfsd startup/shutdown time Dai Ngo
-- strict thread matches above, loose matches on Subject: below --
2023-01-11 20:17 Dai Ngo
2023-01-12 2:38 ` Chuck Lever III
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox