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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (executable)
--- 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 (file)
--- 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