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 <[email protected]>
---
fs/nfsd/nfs4state.c | 18 ++++++------------
fs/nfsd/nfsctl.c | 7 +------
fs/nfsd/nfsd.h | 6 ++----
3 files changed, 9 insertions(+), 22 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 7b2ee535ade8..ee56c9466304 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,7 +8067,10 @@ static int nfs4_state_create_net(struct net *net)
INIT_DELAYED_WORK(&nn->nfsd_shrinker_work, nfsd4_state_shrinker_worker);
get_net(net);
- return 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");
err_sessionid:
kfree(nn->unconf_id_hashtbl);
@@ -8171,6 +8164,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
On Mon, 2023-01-09 at 22:43 -0800, Dai Ngo wrote:
> 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.
>
But this patch doesn't move the reply cache shrinker. Do you intend to
do that too?
> 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 <[email protected]>
> ---
> fs/nfsd/nfs4state.c | 18 ++++++------------
> fs/nfsd/nfsctl.c | 7 +------
> fs/nfsd/nfsd.h | 6 ++----
> 3 files changed, 9 insertions(+), 22 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 7b2ee535ade8..ee56c9466304 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,7 +8067,10 @@ static int nfs4_state_create_net(struct net *net)
> INIT_DELAYED_WORK(&nn->nfsd_shrinker_work, nfsd4_state_shrinker_worker);
> get_net(net);
>
> - return 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");
>
> err_sessionid:
> kfree(nn->unconf_id_hashtbl);
> @@ -8171,6 +8164,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)
Patch looks reasonable. It might be good to also move the reply cache
init in a similar way (as you pointed out in the patch description).
Reviewed-by: Jeff Layton <[email protected]>
> On Jan 10, 2023, at 1:43 AM, Dai Ngo <[email protected]> wrote:
>
> 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 <[email protected]>
> ---
> fs/nfsd/nfs4state.c | 18 ++++++------------
> fs/nfsd/nfsctl.c | 7 +------
> fs/nfsd/nfsd.h | 6 ++----
> 3 files changed, 9 insertions(+), 22 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 7b2ee535ade8..ee56c9466304 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,7 +8067,10 @@ static int nfs4_state_create_net(struct net *net)
> INIT_DELAYED_WORK(&nn->nfsd_shrinker_work, nfsd4_state_shrinker_worker);
> get_net(net);
>
> - return 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");
>
> err_sessionid:
> kfree(nn->unconf_id_hashtbl);
> @@ -8171,6 +8164,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
>
Applied this one to nfsd's for-next. Thanks!
The other patch (once the review comments are addressed) is
to be applied to nfsd's for-rc, I'm assuming.
--
Chuck Lever
On 1/10/23 2:24 AM, Jeff Layton wrote:
> On Mon, 2023-01-09 at 22:43 -0800, Dai Ngo wrote:
>> 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.
>>
> But this patch doesn't move the reply cache shrinker. Do you intend to
> do that too?
yes, I plan to do that soon in a separate patch.
>
>> 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 <[email protected]>
>> ---
>> fs/nfsd/nfs4state.c | 18 ++++++------------
>> fs/nfsd/nfsctl.c | 7 +------
>> fs/nfsd/nfsd.h | 6 ++----
>> 3 files changed, 9 insertions(+), 22 deletions(-)
>>
>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>> index 7b2ee535ade8..ee56c9466304 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,7 +8067,10 @@ static int nfs4_state_create_net(struct net *net)
>> INIT_DELAYED_WORK(&nn->nfsd_shrinker_work, nfsd4_state_shrinker_worker);
>> get_net(net);
>>
>> - return 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");
>>
>> err_sessionid:
>> kfree(nn->unconf_id_hashtbl);
>> @@ -8171,6 +8164,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)
>
>
> Patch looks reasonable. It might be good to also move the reply cache
> init in a similar way (as you pointed out in the patch description).
Thank you for reviewing.
-Dai
>
> Reviewed-by: Jeff Layton <[email protected]>
On Mon, 2023-01-09 at 22:43 -0800, Dai Ngo wrote:
> 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 <[email protected]>
> ---
> fs/nfsd/nfs4state.c | 18 ++++++------------
> fs/nfsd/nfsctl.c | 7 +------
> fs/nfsd/nfsd.h | 6 ++----
> 3 files changed, 9 insertions(+), 22 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 7b2ee535ade8..ee56c9466304 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,7 +8067,10 @@ static int nfs4_state_create_net(struct net *net)
> INIT_DELAYED_WORK(&nn->nfsd_shrinker_work, nfsd4_state_shrinker_worker);
> get_net(net);
>
> - return 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");
>
> err_sessionid:
> kfree(nn->unconf_id_hashtbl);
> @@ -8171,6 +8164,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)
I suspect that this patch is probably enough to fix the problem that
Mike is hitting. Perhaps we should mark this for stable after revising
the changelog?
Mike, would you be able to test this patch and see whether it fixes the
shrinker oopses you've been hitting? It's also commit a77ce15ca9cb10 in
Chuck's git tree.
--
Jeff Layton <[email protected]>
On Wed, 2023-01-11 at 09:34 -0500, Jeff Layton wrote:
>
> I suspect that this patch is probably enough to fix the problem that
> Mike is hitting. Perhaps we should mark this for stable after revising
> the changelog?
>
> Mike, would you be able to test this patch and see whether it fixes the
> shrinker oopses you've been hitting? It's also commit a77ce15ca9cb10 in
> Chuck's git tree.
Yup, all better.
-Mike