LU-6600 obdclass: race lustre_profile_list

running multiple mounts at the same time results in
lustre_profile_list corruption when adding a new profile.

This patch adds a new spin_lock to protect the list and
avoid the bug

Signed-off-by: Hiroya Nozaki <nozaki.hiroya@jp.fujitsu.com>
Change-Id: I0d6ef47e9fa3dd9de8bd33d426112242136a9e26
Reviewed-on: http://review.whamcloud.com/14896
Reviewed-by: Andreas Dilger <andreas.dilger@intel.com>
Tested-by: Jenkins
Tested-by: Maloo <hpdd-maloo@intel.com>
Reviewed-by: Jian Yu <jian.yu@intel.com>
Reviewed-by: Oleg Drokin <oleg.drokin@intel.com>

diff --git a/lustre/include/obd_class.h b/lustre/include/obd_class.h
index b4b2d6e..4ab8d16 100644 (file)
--- a/lustre/include/obd_class.h
+++ b/lustre/include/obd_class.h
@@ -216,12 +216,16 @@ struct lustre_profile {
        char                    *lp_profile;
        char                    *lp_dt;
        char                    *lp_md;
+       int                      lp_refs;
+       bool                     lp_list_deleted;
 };
 
 struct lustre_profile *class_get_profile(const char * prof);
 void class_del_profile(const char *prof);
+void class_put_profile(struct lustre_profile *lprof);
 void class_del_profiles(void);
 
+
 #if LUSTRE_TRACKS_LOCK_EXP_REFS
 
 void __class_export_add_lock_ref(struct obd_export *, struct ldlm_lock *);

diff --git a/lustre/llite/llite_lib.c b/lustre/llite/llite_lib.c
index cb0718d..92f691f 100644 (file)
--- a/lustre/llite/llite_lib.c
+++ b/lustre/llite/llite_lib.c
@@ -1054,17 +1054,19 @@ int ll_fill_super(struct super_block *sb, struct vfsmount *mnt)
        sbi->ll_client_common_fill_super_succeeded = 1;
 
 out_free:
-        if (md)
-                OBD_FREE(md, strlen(lprof->lp_md) + instlen + 2);
-        if (dt)
-                OBD_FREE(dt, strlen(lprof->lp_dt) + instlen + 2);
-        if (err)
-                ll_put_super(sb);
-        else if (sbi->ll_flags & LL_SBI_VERBOSE)
-                LCONSOLE_WARN("Mounted %s\n", profilenm);
+       if (md)
+               OBD_FREE(md, strlen(lprof->lp_md) + instlen + 2);
+       if (dt)
+               OBD_FREE(dt, strlen(lprof->lp_dt) + instlen + 2);
+       if (lprof != NULL)
+               class_put_profile(lprof);
+       if (err)
+               ll_put_super(sb);
+       else if (sbi->ll_flags & LL_SBI_VERBOSE)
+               LCONSOLE_WARN("Mounted %s\n", profilenm);
 
-        OBD_FREE_PTR(cfg);
-        RETURN(err);
+       OBD_FREE_PTR(cfg);
+       RETURN(err);
 } /* ll_fill_super */
 
 void ll_put_super(struct super_block *sb)

diff --git a/lustre/mdt/mdt_handler.c b/lustre/mdt/mdt_handler.c
index 9c69c54..f7ac4c0 100644 (file)
--- a/lustre/mdt/mdt_handler.c
+++ b/lustre/mdt/mdt_handler.c
@@ -3890,7 +3890,7 @@ static int mdt_stack_init(const struct lu_env *env, struct mdt_device *mdt,
 
        lcfg = lustre_cfg_new(LCFG_ATTACH, bufs);
        if (lcfg == NULL)
-               GOTO(free_bufs, rc = -ENOMEM);
+               GOTO(put_profile, rc = -ENOMEM);
 
        rc = class_attach(lcfg);
        if (rc)
@@ -3950,6 +3950,8 @@ class_detach:
                class_detach(obd, lcfg);
 lcfg_cleanup:
        lustre_cfg_free(lcfg);
+put_profile:
+       class_put_profile(lprof);
 free_bufs:
        OBD_FREE_PTR(bufs);
 cleanup_mem:
@@ -4020,7 +4022,7 @@ static int mdt_quota_init(const struct lu_env *env, struct mdt_device *mdt,
 
        lcfg = lustre_cfg_new(LCFG_ATTACH, bufs);
        if (lcfg == NULL)
-               GOTO(cleanup_mem, rc = -ENOMEM);
+               GOTO(put_profile, rc = -ENOMEM);
 
        rc = class_attach(lcfg);
        if (rc)
@@ -4082,6 +4084,8 @@ class_detach:
                class_detach(obd, lcfg);
 lcfg_cleanup:
        lustre_cfg_free(lcfg);
+put_profile:
+       class_put_profile(lprof);
 cleanup_mem:
        if (bufs)
                OBD_FREE_PTR(bufs);

diff --git a/lustre/obdclass/class_obd.c b/lustre/obdclass/class_obd.c
index 20ea553..6e4c921 100644 (file)
--- a/lustre/obdclass/class_obd.c
+++ b/lustre/obdclass/class_obd.c
@@ -486,37 +486,37 @@ static int obd_init_checks(void)
 
 static int __init init_obdclass(void)
 {
-        int i, err;
+       int i, err;
 
        spin_lock_init(&obd_stale_export_lock);
        INIT_LIST_HEAD(&obd_stale_exports);
        atomic_set(&obd_stale_export_num, 0);
 
-        LCONSOLE_INFO("Lustre: Build Version: "BUILD_VERSION"\n");
+       LCONSOLE_INFO("Lustre: Build Version: "BUILD_VERSION"\n");
 
        spin_lock_init(&obd_types_lock);
-        obd_zombie_impexp_init();
+       obd_zombie_impexp_init();
 #ifdef CONFIG_PROC_FS
-        obd_memory = lprocfs_alloc_stats(OBD_STATS_NUM,
+       obd_memory = lprocfs_alloc_stats(OBD_STATS_NUM,
                                         LPROCFS_STATS_FLAG_NONE |
                                         LPROCFS_STATS_FLAG_IRQ_SAFE);
-        if (obd_memory == NULL) {
-                CERROR("kmalloc of 'obd_memory' failed\n");
-                RETURN(-ENOMEM);
-        }
+       if (obd_memory == NULL) {
+               CERROR("kmalloc of 'obd_memory' failed\n");
+               RETURN(-ENOMEM);
+       }
 
-        lprocfs_counter_init(obd_memory, OBD_MEMORY_STAT,
-                             LPROCFS_CNTR_AVGMINMAX,
-                             "memused", "bytes");
+       lprocfs_counter_init(obd_memory, OBD_MEMORY_STAT,
+                            LPROCFS_CNTR_AVGMINMAX,
+                            "memused", "bytes");
 #endif
-        err = obd_init_checks();
-        if (err == -EOVERFLOW)
-                return err;
+       err = obd_init_checks();
+       if (err == -EOVERFLOW)
+               return err;
 
-        class_init_uuidlist();
-        err = class_handle_init();
-        if (err)
-                return err;
+       class_init_uuidlist();
+       err = class_handle_init();
+       if (err)
+               return err;
 
        INIT_LIST_HEAD(&obd_types);
 
@@ -526,24 +526,24 @@ static int __init init_obdclass(void)
                return err;
        }
 
-        /* This struct is already zeroed for us (static global) */
-        for (i = 0; i < class_devno_max(); i++)
-                obd_devs[i] = NULL;
+       /* This struct is already zeroed for us (static global) */
+       for (i = 0; i < class_devno_max(); i++)
+               obd_devs[i] = NULL;
 
-        /* Default the dirty page cache cap to 1/2 of system memory.
-         * For clients with less memory, a larger fraction is needed
-         * for other purposes (mostly for BGL). */
+       /* Default the dirty page cache cap to 1/2 of system memory.
+        * For clients with less memory, a larger fraction is needed
+        * for other purposes (mostly for BGL). */
        if (totalram_pages <= 512 << (20 - PAGE_CACHE_SHIFT))
                obd_max_dirty_pages = totalram_pages / 4;
        else
                obd_max_dirty_pages = totalram_pages / 2;
 
-        err = obd_init_caches();
-        if (err)
-                return err;
-        err = class_procfs_init();
-        if (err)
-                return err;
+       err = obd_init_caches();
+       if (err)
+               return err;
+       err = class_procfs_init();
+       if (err)
+               return err;
 
        err = lu_global_init();
        if (err)
@@ -567,9 +567,9 @@ static int __init init_obdclass(void)
        if (err)
                return err;
 
-        err = lustre_register_fs();
+       err = lustre_register_fs();
 
-        return err;
+       return err;
 }
 
 void obd_update_maxusage(void)

diff --git a/lustre/obdclass/obd_config.c b/lustre/obdclass/obd_config.c
index bbedbdc..0b6ff29 100644 (file)
--- a/lustre/obdclass/obd_config.c
+++ b/lustre/obdclass/obd_config.c
@@ -836,18 +836,23 @@ static int class_del_conn(struct obd_device *obd, struct lustre_cfg *lcfg)
 
 static struct list_head lustre_profile_list =
        LIST_HEAD_INIT(lustre_profile_list);
+static DEFINE_SPINLOCK(lustre_profile_list_lock);
 
 struct lustre_profile *class_get_profile(const char * prof)
 {
-        struct lustre_profile *lprof;
+       struct lustre_profile *lprof;
 
-        ENTRY;
+       ENTRY;
+       spin_lock(&lustre_profile_list_lock);
        list_for_each_entry(lprof, &lustre_profile_list, lp_list) {
-                if (!strcmp(lprof->lp_profile, prof)) {
-                        RETURN(lprof);
-                }
-        }
-        RETURN(NULL);
+               if (!strcmp(lprof->lp_profile, prof)) {
+                       lprof->lp_refs++;
+                       spin_unlock(&lustre_profile_list_lock);
+                       RETURN(lprof);
+               }
+       }
+       spin_unlock(&lustre_profile_list_lock);
+       RETURN(NULL);
 }
 EXPORT_SYMBOL(class_get_profile);
 
@@ -889,7 +894,12 @@ static int class_add_profile(int proflen, char *prof, int osclen, char *osc,
                 memcpy(lprof->lp_md, mdc, mdclen);
         }
 
+       spin_lock(&lustre_profile_list_lock);
+       lprof->lp_refs = 1;
+       lprof->lp_list_deleted = false;
+
        list_add(&lprof->lp_list, &lustre_profile_list);
+       spin_unlock(&lustre_profile_list_lock);
         RETURN(err);
 
 out:
@@ -905,39 +915,68 @@ out:
 
 void class_del_profile(const char *prof)
 {
-        struct lustre_profile *lprof;
-        ENTRY;
+       struct lustre_profile *lprof;
+       ENTRY;
 
-        CDEBUG(D_CONFIG, "Del profile %s\n", prof);
+       CDEBUG(D_CONFIG, "Del profile %s\n", prof);
 
-        lprof = class_get_profile(prof);
-        if (lprof) {
+       lprof = class_get_profile(prof);
+       if (lprof) {
+               spin_lock(&lustre_profile_list_lock);
+               /* because get profile increments the ref counter */
+               lprof->lp_refs--;
                list_del(&lprof->lp_list);
-                OBD_FREE(lprof->lp_profile, strlen(lprof->lp_profile) + 1);
-                OBD_FREE(lprof->lp_dt, strlen(lprof->lp_dt) + 1);
-                if (lprof->lp_md)
-                        OBD_FREE(lprof->lp_md, strlen(lprof->lp_md) + 1);
-                OBD_FREE(lprof, sizeof *lprof);
-        }
-        EXIT;
+               lprof->lp_list_deleted = true;
+               spin_unlock(&lustre_profile_list_lock);
+
+               class_put_profile(lprof);
+       }
+       EXIT;
 }
 EXPORT_SYMBOL(class_del_profile);
 
+void class_put_profile(struct lustre_profile *lprof)
+{
+       spin_lock(&lustre_profile_list_lock);
+       if ((--lprof->lp_refs) > 0) {
+               LASSERT(lprof->lp_refs > 0);
+               spin_unlock(&lustre_profile_list_lock);
+               return;
+       }
+       spin_unlock(&lustre_profile_list_lock);
+
+       /* confirm not a negative number */
+       LASSERT(lprof->lp_refs == 0);
+
+       /* At least one class_del_profile/profiles must be called
+        * on the target profile or lustre_profile_list will corrupt */
+       LASSERT(lprof->lp_list_deleted);
+       OBD_FREE(lprof->lp_profile, strlen(lprof->lp_profile) + 1);
+       OBD_FREE(lprof->lp_dt, strlen(lprof->lp_dt) + 1);
+       if (lprof->lp_md != NULL)
+               OBD_FREE(lprof->lp_md, strlen(lprof->lp_md) + 1);
+       OBD_FREE(lprof, sizeof(*lprof));
+}
+EXPORT_SYMBOL(class_put_profile);
+
 /* COMPAT_146 */
 void class_del_profiles(void)
 {
-        struct lustre_profile *lprof, *n;
-        ENTRY;
+       struct lustre_profile *lprof, *n;
+       ENTRY;
 
+       spin_lock(&lustre_profile_list_lock);
        list_for_each_entry_safe(lprof, n, &lustre_profile_list, lp_list) {
                list_del(&lprof->lp_list);
-                OBD_FREE(lprof->lp_profile, strlen(lprof->lp_profile) + 1);
-                OBD_FREE(lprof->lp_dt, strlen(lprof->lp_dt) + 1);
-                if (lprof->lp_md)
-                        OBD_FREE(lprof->lp_md, strlen(lprof->lp_md) + 1);
-                OBD_FREE(lprof, sizeof *lprof);
-        }
-        EXIT;
+               lprof->lp_list_deleted = true;
+               spin_unlock(&lustre_profile_list_lock);
+
+               class_put_profile(lprof);
+
+               spin_lock(&lustre_profile_list_lock);
+       }
+       spin_unlock(&lustre_profile_list_lock);
+       EXIT;
 }
 EXPORT_SYMBOL(class_del_profiles);
 

diff --git a/lustre/obdclass/obd_mount_server.c b/lustre/obdclass/obd_mount_server.c
index a236d16..ef453b0 100644 (file)
--- a/lustre/obdclass/obd_mount_server.c
+++ b/lustre/obdclass/obd_mount_server.c
@@ -1487,9 +1487,13 @@ static void server_put_super(struct super_block *sb)
                   If there are any setup/cleanup errors, save the lov
                   name for safety cleanup later. */
                lprof = class_get_profile(lsi->lsi_svname);
-               if (lprof && lprof->lp_dt) {
-                       OBD_ALLOC(extraname, strlen(lprof->lp_dt) + 1);
-                       strcpy(extraname, lprof->lp_dt);
+               if (lprof != NULL) {
+                       if (lprof->lp_dt != NULL) {
+                               OBD_ALLOC(extraname, strlen(lprof->lp_dt) + 1);
+                               strncpy(extraname, lprof->lp_dt,
+                                       strlen(lprof->lp_dt) + 1);
+                       }
+                       class_put_profile(lprof);
                }
 
                obd = class_name2obd(lsi->lsi_svname);