From: Rahul Deshmukh Date: Wed, 26 May 2010 08:48:17 +0000 (+0530) Subject: b=21815 per-nid stats should not access lustre hash internal structures directly X-Git-Tag: v1_10_0_43~19 X-Git-Url: https://git.whamcloud.com/?p=fs%2Flustre-release.git;a=commitdiff_plain;h=45e8c410e3cbe3d1d38db8976db7c3c0f2f7e645 b=21815 per-nid stats should not access lustre hash internal structures directly Fixed Yang Sheng's patch as per Robert's comment. i=rread --- diff --git a/libcfs/include/libcfs/libcfs_hash.h b/libcfs/include/libcfs/libcfs_hash.h index 98f4f78..9011de7 100644 --- a/libcfs/include/libcfs/libcfs_hash.h +++ b/libcfs/include/libcfs/libcfs_hash.h @@ -286,6 +286,8 @@ void cfs_hash_for_each_safe(cfs_hash_t *hs, cfs_hash_for_each_cb_t, void *data); void cfs_hash_for_each_empty(cfs_hash_t *hs, cfs_hash_for_each_cb_t, void *data); void cfs_hash_for_each_key(cfs_hash_t *hs, void *key, cfs_hash_for_each_cb_t, void *data); +typedef int (*cfs_hash_cond_opt_cb_t)(void *obj, void *data); +void cfs_hash_cond_del(cfs_hash_t *hs, cfs_hash_cond_opt_cb_t, void *data); /* * Rehash - Theta is calculated to be the average chained diff --git a/libcfs/libcfs/hash.c b/libcfs/libcfs/hash.c index ae2f640..cba1d37 100644 --- a/libcfs/libcfs/hash.c +++ b/libcfs/libcfs/hash.c @@ -383,6 +383,37 @@ cfs_hash_del(cfs_hash_t *hs, void *key, cfs_hlist_node_t *hnode) CFS_EXPORT_SYMBOL(cfs_hash_del); /** + * Delete item from the libcfs hash @hs when @func return true. + * The write lock being hold during loop for each bucket to avoid + * any object be reference. + */ +void +cfs_hash_cond_del(cfs_hash_t *hs, cfs_hash_cond_opt_cb_t func, void *data) +{ + cfs_hlist_node_t *hnode; + cfs_hlist_node_t *pos; + cfs_hash_bucket_t *hsb; + int i; + ENTRY; + + cfs_hash_wlock(hs); + cfs_hash_for_each_bucket(hs, hsb, i) { + cfs_write_lock(&hsb->hsb_rwlock); + cfs_hlist_for_each_safe(hnode, pos, &(hsb->hsb_head)) { + __cfs_hash_bucket_validate(hs, hsb, hnode); + if (func(cfs_hash_get(hs, hnode), data)) + __cfs_hash_bucket_del(hs, hsb, hnode); + (void)cfs_hash_put(hs, hnode); + } + cfs_write_unlock(&hsb->hsb_rwlock); + } + cfs_hash_wunlock(hs); + + EXIT; +} +CFS_EXPORT_SYMBOL(cfs_hash_cond_del); + +/** * Delete item given @key in libcfs hash @hs. The first @key found in * the hash will be removed, if the key exists multiple times in the hash * @hs this function must be called once per key. The removed object @@ -765,7 +796,7 @@ void cfs_hash_rehash_key(cfs_hash_t *hs, void *old_key, void *new_key, cfs_write_lock(&new_hsb->hsb_rwlock); cfs_write_lock(&old_hsb->hsb_rwlock); } else { /* do nothing */ - cfs_read_unlock(&hs->hs_rwlock); + cfs_hash_runlock(hs); EXIT; return; } diff --git a/lustre/include/obd_class.h b/lustre/include/obd_class.h index 5b354e1..1376b75 100644 --- a/lustre/include/obd_class.h +++ b/lustre/include/obd_class.h @@ -381,6 +381,19 @@ do { \ #define EXP_MD_COUNTER_INCREMENT(exp, op) #endif +static inline int lprocfs_nid_ldlm_stats_init(struct nid_stat* tmp) { + /* Always add in ldlm_stats */ + tmp->nid_ldlm_stats = lprocfs_alloc_stats(LDLM_LAST_OPC - LDLM_FIRST_OPC + ,LPROCFS_STATS_FLAG_NOPERCPU); + if (tmp->nid_ldlm_stats == NULL) + return -ENOMEM; + + lprocfs_init_ldlm_stats(tmp->nid_ldlm_stats); + + return lprocfs_register_stats(tmp->nid_proc, "ldlm_stats", + tmp->nid_ldlm_stats); +} + #define OBD_CHECK_MD_OP(obd, op, err) \ do { \ if (!OBT(obd) || !MDP((obd), op)) { \ diff --git a/lustre/mdt/mdt_fs.c b/lustre/mdt/mdt_fs.c index 2c38182..599881e 100644 --- a/lustre/mdt/mdt_fs.c +++ b/lustre/mdt/mdt_fs.c @@ -61,16 +61,7 @@ int mdt_export_stats_init(struct obd_device *obd, RETURN(rc); } if (newnid) { - /* Always add in ldlm_stats */ - exp->exp_nid_stats->nid_ldlm_stats = - lprocfs_alloc_stats(LDLM_LAST_OPC - LDLM_FIRST_OPC, - LPROCFS_STATS_FLAG_NOPERCPU); - if (exp->exp_nid_stats->nid_ldlm_stats == NULL) - GOTO(clean, rc = -ENOMEM); - lprocfs_init_ldlm_stats(exp->exp_nid_stats->nid_ldlm_stats); - rc = lprocfs_register_stats(exp->exp_nid_stats->nid_proc, - "ldlm_stats", - exp->exp_nid_stats->nid_ldlm_stats); + rc = lprocfs_nid_ldlm_stats_init(exp->exp_nid_stats); if (rc) GOTO(clean, rc); } diff --git a/lustre/mgs/mgs_fs.c b/lustre/mgs/mgs_fs.c index ff708e0..bd69bca 100644 --- a/lustre/mgs/mgs_fs.c +++ b/lustre/mgs/mgs_fs.c @@ -75,16 +75,7 @@ static int mgs_export_stats_init(struct obd_device *obd, struct obd_export *exp, } if (newnid) { - /* Always add in ldlm_stats */ - exp->exp_nid_stats->nid_ldlm_stats = - lprocfs_alloc_stats(LDLM_LAST_OPC - LDLM_FIRST_OPC, - LPROCFS_STATS_FLAG_NOPERCPU); - if (exp->exp_nid_stats->nid_ldlm_stats == NULL) - GOTO(clean, rc = -ENOMEM); - lprocfs_init_ldlm_stats(exp->exp_nid_stats->nid_ldlm_stats); - rc = lprocfs_register_stats(exp->exp_nid_stats->nid_proc, - "ldlm_stats", - exp->exp_nid_stats->nid_ldlm_stats); + rc = lprocfs_nid_ldlm_stats_init(exp->exp_nid_stats); if (rc) GOTO(clean, rc); } diff --git a/lustre/obdclass/lprocfs_status.c b/lustre/obdclass/lprocfs_status.c index c0fefe7..12d91e3 100644 --- a/lustre/obdclass/lprocfs_status.c +++ b/lustre/obdclass/lprocfs_status.c @@ -1695,7 +1695,7 @@ int lprocfs_nid_stats_clear_read(char *page, char **start, off_t off, } EXPORT_SYMBOL(lprocfs_nid_stats_clear_read); -void lprocfs_nid_stats_clear_write_cb(void *obj, void *data) +int lprocfs_nid_stats_clear_write_cb(void *obj, void *data) { struct nid_stat *stat = obj; int i; @@ -1704,13 +1704,10 @@ void lprocfs_nid_stats_clear_write_cb(void *obj, void *data) * add/delete blocked by hash bucket lock */ CDEBUG(D_INFO,"refcnt %d\n", cfs_atomic_read(&stat->nid_exp_ref_count)); if (cfs_atomic_read(&stat->nid_exp_ref_count) == 2) { - cfs_hlist_del_init(&stat->nid_hash); - nidstat_putref(stat); cfs_spin_lock(&stat->nid_obd->obd_nid_lock); cfs_list_move(&stat->nid_list, data); cfs_spin_unlock(&stat->nid_obd->obd_nid_lock); - EXIT; - return; + RETURN(1); } /* we has reference to object - only clear data*/ if (stat->nid_stats) @@ -1720,8 +1717,7 @@ void lprocfs_nid_stats_clear_write_cb(void *obj, void *data) for (i = 0; i < BRW_LAST; i++) lprocfs_oh_clear(&stat->nid_brw_stats->hist[i]); } - EXIT; - return; + RETURN(0); } int lprocfs_nid_stats_clear_write(struct file *file, const char *buffer, @@ -1731,7 +1727,7 @@ int lprocfs_nid_stats_clear_write(struct file *file, const char *buffer, struct nid_stat *client_stat; CFS_LIST_HEAD(free_list); - cfs_hash_for_each(obd->obd_nid_stats_hash, + cfs_hash_cond_del(obd->obd_nid_stats_hash, lprocfs_nid_stats_clear_write_cb, &free_list); while (!cfs_list_empty(&free_list)) { diff --git a/lustre/obdfilter/filter.c b/lustre/obdfilter/filter.c index 1b9078a..d651a0c 100644 --- a/lustre/obdfilter/filter.c +++ b/lustre/obdfilter/filter.c @@ -282,16 +282,7 @@ static int filter_export_stats_init(struct obd_device *obd, tmp->nid_stats); if (rc) GOTO(clean, rc); - /* Always add in ldlm_stats */ - tmp->nid_ldlm_stats = - lprocfs_alloc_stats(LDLM_LAST_OPC - LDLM_FIRST_OPC, - LPROCFS_STATS_FLAG_NOPERCPU); - if (tmp->nid_ldlm_stats == NULL) - GOTO(clean, rc = -ENOMEM); - - lprocfs_init_ldlm_stats(tmp->nid_ldlm_stats); - rc = lprocfs_register_stats(tmp->nid_proc, "ldlm_stats", - tmp->nid_ldlm_stats); + rc = lprocfs_nid_ldlm_stats_init(tmp); if (rc) GOTO(clean, rc); } diff --git a/lustre/tests/replay-single.sh b/lustre/tests/replay-single.sh index 8ca4206..0b2900c 100755 --- a/lustre/tests/replay-single.sh +++ b/lustre/tests/replay-single.sh @@ -2063,6 +2063,13 @@ test_85() { # bug 22190 } run_test 85 "ensure there is no reply on bulk write if obd is in rdonly mode" +test_86() { + umount $MOUNT + do_facet $SINGLEMDS lctl set_param mdt.${FSNAME}-MDT*.exports.clear=0 + remount_facet $SINGLEMDS +} +run_test 86 "umount server after clear nid_stats should not hit LBUG" + equals_msg `basename $0`: test complete, cleaning up check_and_cleanup_lustre [ -f "$TESTSUITELOG" ] && cat $TESTSUITELOG && grep -q FAIL $TESTSUITELOG && exit 1 || true diff --git a/lustre/tests/test-framework.sh b/lustre/tests/test-framework.sh index f41aa76..2c31167 100644 --- a/lustre/tests/test-framework.sh +++ b/lustre/tests/test-framework.sh @@ -878,6 +878,13 @@ shutdown_facet() { fi } +remount_facet() { + local facet=$1 + + stop $facet + mount_facet $facet +} + reboot_facet() { facet=$1 if [ "$FAILURE_MODE" = HARD ]; then