Whamcloud - gitweb
LU-12477 lustre: check return status of register_shrinker() 83/40883/4
authorMr NeilBrown <neilb@suse.de>
Mon, 7 Dec 2020 02:07:31 +0000 (13:07 +1100)
committerOleg Drokin <green@whamcloud.com>
Wed, 10 Mar 2021 08:03:01 +0000 (08:03 +0000)
register_shrinker() can fail with -ENOMEM.  We should check for that
and abort the relevant initialization functions when it happens.

For ldlm_pools, ldlm_pools_fini() can be called when ldlm_pools_init()
fails, or even in case where it hasn't been called.  So add a static
flag to ensure we ldlm_pools_fini() does undo things that haven't been
done.

For lu_global_init() we need to add proper cleanup if anything fails.

Signed-off-by: Mr NeilBrown <neilb@suse.de>
Change-Id: Ie66326486c7738547d4211095bb1d37dc75e0b6a
Reviewed-on: https://review.whamcloud.com/40883
Tested-by: jenkins <devops@whamcloud.com>
Reviewed-by: Yang Sheng <ys@whamcloud.com>
Reviewed-by: James Simmons <jsimmons@infradead.org>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
lustre/autoconf/lustre-core.m4
lustre/include/lustre_compat.h
lustre/ldlm/ldlm_pool.c
lustre/obdclass/lu_object.c
lustre/osc/osc_request.c
lustre/ptlrpc/sec_bulk.c

index 5ccdead..f61b6a4 100644 (file)
@@ -619,6 +619,24 @@ kiocb_ki_left, [
 ]) # LC_KIOCB_KI_LEFT
 
 #
+# LC_REGISTER_SHRINKER_RET
+#
+# v3.11-8748-g1d3d4437eae1 register_shrinker returns a status
+#
+AC_DEFUN([LC_REGISTER_SHRINKER_RET], [
+LB_CHECK_COMPILE([if register_shrinker() returns status],
+register_shrinker_ret, [
+       #include <linux/mm.h>
+],[
+       if (register_shrinker(NULL))
+               unregister_shrinker(NULL);
+],[
+       AC_DEFINE(HAVE_REGISTER_SHRINKER_RET, 1,
+               [register_shrinker() returns status])
+])
+]) # LC_REGISTER_SHRINKER_RET
+
+#
 # LC_VFS_RENAME_5ARGS
 #
 # 3.13 has vfs_rename with 5 args
@@ -2375,6 +2393,7 @@ AC_DEFUN([LC_PROG_LINUX], [
        LC_OLDSIZE_TRUNCATE_PAGECACHE
        LC_PTR_ERR_OR_ZERO_MISSING
        LC_KIOCB_KI_LEFT
+       LC_REGISTER_SHRINKER_RET
 
        # 3.13
        LC_VFS_RENAME_5ARGS
index 8ab1fbd..51994c8 100644 (file)
@@ -587,4 +587,8 @@ static inline bool is_root_inode(struct inode *inode)
 }
 #endif
 
+#ifndef HAVE_REGISTER_SHRINKER_RET
+#define register_shrinker(_s) (register_shrinker(_s), 0)
+#endif
+
 #endif /* _LUSTRE_COMPAT_H */
index 5296d2a..fd9f741 100644 (file)
@@ -1437,9 +1437,12 @@ static void ldlm_pools_recalc_task(struct work_struct *ws)
        schedule_delayed_work(&ldlm_pools_recalc_work, cfs_time_seconds(delay));
 }
 
+static bool ldlm_pools_init_done;
+
 int ldlm_pools_init(void)
 {
        time64_t delay;
+       int rc;
 
 #ifdef HAVE_SERVER_SUPPORT
        delay = min(LDLM_POOL_SRV_DEF_RECALC_PERIOD,
@@ -1448,19 +1451,34 @@ int ldlm_pools_init(void)
        delay = LDLM_POOL_CLI_DEF_RECALC_PERIOD;
 #endif
 
-       schedule_delayed_work(&ldlm_pools_recalc_work, delay);
-       register_shrinker(&ldlm_pools_srv_shrinker);
-       register_shrinker(&ldlm_pools_cli_shrinker);
+       rc = register_shrinker(&ldlm_pools_srv_shrinker);
+       if (rc)
+               goto out;
 
+       rc = register_shrinker(&ldlm_pools_cli_shrinker);
+       if (rc)
+               goto out_shrinker;
+
+       schedule_delayed_work(&ldlm_pools_recalc_work, delay);
+       ldlm_pools_init_done = true;
        return 0;
+
+out_shrinker:
+       unregister_shrinker(&ldlm_pools_cli_shrinker);
+out:
+       return rc;
 }
 
 void ldlm_pools_fini(void)
 {
-       unregister_shrinker(&ldlm_pools_srv_shrinker);
-       unregister_shrinker(&ldlm_pools_cli_shrinker);
+       if (ldlm_pools_init_done) {
+               unregister_shrinker(&ldlm_pools_srv_shrinker);
+               unregister_shrinker(&ldlm_pools_cli_shrinker);
+
+               cancel_delayed_work_sync(&ldlm_pools_recalc_work);
+       }
 
-       cancel_delayed_work_sync(&ldlm_pools_recalc_work);
+       ldlm_pools_init_done = false;
 }
 
 #else /* !HAVE_LRU_RESIZE_SUPPORT */
index 645a815..8e79c57 100644 (file)
@@ -2309,8 +2309,8 @@ int lu_global_init(void)
 
        LU_CONTEXT_KEY_INIT(&lu_global_key);
        result = lu_context_key_register(&lu_global_key);
-       if (result != 0)
-               return result;
+       if (result)
+               goto out_lu_ref;
 
        /*
         * At this level, we don't know what tags are needed, so allocate them
@@ -2320,18 +2320,37 @@ int lu_global_init(void)
        down_write(&lu_sites_guard);
        result = lu_env_init(&lu_shrink_env, LCT_SHRINKER);
        up_write(&lu_sites_guard);
-       if (result != 0)
-               return result;
+       if (result) {
+               lu_context_key_degister(&lu_global_key);
+               goto out_lu_ref;
+       }
 
        /*
         * seeks estimation: 3 seeks to read a record from oi, one to read
         * inode, one for ea. Unfortunately setting this high value results in
         * lu_object/inode cache consuming all the memory.
         */
-       register_shrinker(&lu_site_shrinker);
+       result = register_shrinker(&lu_site_shrinker);
+       if (result)
+               goto out_env;
 
        result = rhashtable_init(&lu_env_rhash, &lu_env_rhash_params);
 
+       if (result)
+               goto out_shrinker;
+
+       return result;
+
+out_shrinker:
+       unregister_shrinker(&lu_site_shrinker);
+out_env:
+       /* ordering here is explained in lu_global_fini() */
+       lu_context_key_degister(&lu_global_key);
+       down_write(&lu_sites_guard);
+       lu_env_fini(&lu_shrink_env);
+       up_write(&lu_sites_guard);
+out_lu_ref:
+       lu_ref_global_fini();
        return result;
 }
 
index 1c2b859..d6e5718 100644 (file)
@@ -3662,11 +3662,13 @@ static int __init osc_init(void)
        if (rc)
                GOTO(out_kmem, rc);
 
-       register_shrinker(&osc_cache_shrinker);
+       rc = register_shrinker(&osc_cache_shrinker);
+       if (rc)
+               GOTO(out_type, rc);
 
        /* This is obviously too much memory, only prevent overflow here */
        if (osc_reqpool_mem_max >= 1 << 12 || osc_reqpool_mem_max == 0)
-               GOTO(out_type, rc = -EINVAL);
+               GOTO(out_shrinker, rc = -EINVAL);
 
        reqpool_size = osc_reqpool_mem_max << 20;
 
@@ -3687,7 +3689,7 @@ static int __init osc_init(void)
                                          ptlrpc_add_rqs_to_pool);
 
        if (osc_rq_pool == NULL)
-               GOTO(out_type, rc = -ENOMEM);
+               GOTO(out_shrinker, rc = -ENOMEM);
 
        rc = osc_start_grant_work();
        if (rc != 0)
@@ -3697,6 +3699,8 @@ static int __init osc_init(void)
 
 out_req_pool:
        ptlrpc_free_rq_pool(osc_rq_pool);
+out_shrinker:
+       unregister_shrinker(&osc_cache_shrinker);
 out_type:
        class_unregister_type(LUSTRE_OSC_NAME);
 out_kmem:
index 9f14666..3bf607f 100644 (file)
@@ -762,6 +762,8 @@ static inline void enc_pools_free(void)
 
 int sptlrpc_enc_pool_init(void)
 {
+       int rc;
+
        page_pools.epp_max_pages = cfs_totalram_pages() / 8;
        if (enc_pool_max_memory_mb > 0 &&
            enc_pool_max_memory_mb <= (cfs_totalram_pages() >> mult))
@@ -798,9 +800,11 @@ int sptlrpc_enc_pool_init(void)
        if (page_pools.epp_pools == NULL)
                return -ENOMEM;
 
-       register_shrinker(&pools_shrinker);
+       rc = register_shrinker(&pools_shrinker);
+       if (rc)
+               enc_pools_free();
 
-       return 0;
+       return rc;
 }
 
 void sptlrpc_enc_pool_fini(void)