From eba2a78067ef87a56242e47e626c584624fb1e34 Mon Sep 17 00:00:00 2001 From: Hiroya Nozaki Date: Thu, 20 Aug 2015 14:37:23 +0900 Subject: [PATCH] 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 Change-Id: I0d6ef47e9fa3dd9de8bd33d426112242136a9e26 Reviewed-on: http://review.whamcloud.com/14896 Reviewed-by: Andreas Dilger Tested-by: Jenkins Tested-by: Maloo Reviewed-by: Jian Yu Reviewed-by: Oleg Drokin --- lustre/include/obd_class.h | 4 ++ lustre/llite/llite_lib.c | 22 +++++---- lustre/mdt/mdt_handler.c | 8 +++- lustre/obdclass/class_obd.c | 64 ++++++++++++------------- lustre/obdclass/obd_config.c | 95 +++++++++++++++++++++++++++----------- lustre/obdclass/obd_mount_server.c | 10 ++-- 6 files changed, 128 insertions(+), 75 deletions(-) diff --git a/lustre/include/obd_class.h b/lustre/include/obd_class.h index b4b2d6e..4ab8d16 100644 --- 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 --- 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 --- 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 --- 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 --- 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 --- 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); -- 1.8.3.1