resolve lots races and use after free.

Branch b1_8
b=HEAD
i=jc.lafoucriere
i=panda

diff --git a/lustre/include/class_hash.h b/lustre/include/class_hash.h
index 6210c7f..37bd8d2 100644 (file)
--- a/lustre/include/class_hash.h
+++ b/lustre/include/class_hash.h
@@ -85,11 +85,9 @@ lh_hash(lustre_hash_t *lh, void *key, unsigned mask)
 {
         LASSERT(lh);
         LASSERT(LHO(lh));
+        LASSERT(LHP(lh, hash));
 
-        if (LHP(lh, hash))
-                return LHP(lh, hash)(lh, key, mask);
-
-        return -EOPNOTSUPP;
+        return LHP(lh, hash)(lh, key, mask);
 }
 
 static inline void *

diff --git a/lustre/lov/lov_internal.h b/lustre/lov/lov_internal.h
index b3a763a..2aaaff4 100644 (file)
--- a/lustre/lov/lov_internal.h
+++ b/lustre/lov/lov_internal.h
@@ -320,5 +320,6 @@ int lov_pool_remove(struct obd_device *obd, char *poolname, char *ostname);
 void lov_dump_pool(int level, struct pool_desc *pool);
 struct pool_desc *lov_find_pool(struct lov_obd *lov, char *poolname);
 int lov_check_index_in_pool(__u32 idx, struct pool_desc *pool);
+void lov_pool_putref(struct pool_desc *pool);
 
 #endif

diff --git a/lustre/lov/lov_obd.c b/lustre/lov/lov_obd.c
index 11267e7..00ce37d 100644 (file)
--- a/lustre/lov/lov_obd.c
+++ b/lustre/lov/lov_obd.c
@@ -841,11 +841,12 @@ int lov_setup(struct obd_device *obd, struct lustre_cfg *lcfg)
 static int lov_precleanup(struct obd_device *obd, enum obd_cleanup_stage stage)
 {
         int rc = 0;
+        struct lov_obd *lov = &obd->u.lov;
+
         ENTRY;
 
         switch (stage) {
         case OBD_CLEANUP_EARLY: {
-                struct lov_obd *lov = &obd->u.lov;
                 int i;
                 for (i = 0; i < lov->desc.ld_tgt_count; i++) {
                         if (!lov->lov_tgts[i] || !lov->lov_tgts[i]->ltd_active)
@@ -870,22 +871,19 @@ static int lov_cleanup(struct obd_device *obd)
         struct list_head *pos, *tmp;
         struct pool_desc *pool;
 
-        lprocfs_obd_cleanup(obd);
-
-        /* Delete hash entries and kill hash table before freeing pools
-         * and get to use after free issue. */
-        lustre_hash_exit(lov->lov_pools_hash_body);
-
         list_for_each_safe(pos, tmp, &lov->lov_pool_list) {
                 pool = list_entry(pos, struct pool_desc, pool_list);
                 /* free pool structs */
+                CDEBUG(D_INFO, "delete pool %p\n", pool);
                 lov_pool_del(obd, pool->pool_name);
         }
+        lustre_hash_exit(lov->lov_pools_hash_body);
         lov_ost_pool_free(&(lov->lov_qos.lq_rr.lqr_pool));
         lov_ost_pool_free(&lov->lov_packed);
 
         if (lov->lov_tgts) {
                 int i;
+                lov_getref(obd);
                 for (i = 0; i < lov->desc.ld_tgt_count; i++) {
                         if (!lov->lov_tgts[i])
                                 continue;
@@ -902,11 +900,15 @@ static int lov_cleanup(struct obd_device *obd)
                                        atomic_read(&lov->lov_refcount));
                         lov_del_target(obd, i, 0, 0);
                 }
+                lov_putref(obd);
                 OBD_FREE(lov->lov_tgts, sizeof(*lov->lov_tgts) *
                          lov->lov_tgt_size);
                 lov->lov_tgt_size = 0;
         }
 
+        /* clear pools parent proc entry only after all pools is killed */
+        lprocfs_obd_cleanup(obd);
+
         RETURN(0);
 }
 

diff --git a/lustre/lov/lov_pack.c b/lustre/lov/lov_pack.c
index 0165f69..7fd5470 100644 (file)
--- a/lustre/lov/lov_pack.c
+++ b/lustre/lov/lov_pack.c
@@ -484,8 +484,7 @@ static int __lov_setstripe(struct obd_export *exp, struct lov_stripe_md **lsmp,
                         rc = lov_check_index_in_pool(lumv3.lmm_stripe_offset,
                                                      pool);
                         if (rc < 0) {
-                                lh_put(lov->lov_pools_hash_body,
-                                       &pool->pool_hash);
+                                lov_pool_putref(pool);
                                 RETURN(-EINVAL);
                         }
                 }
@@ -493,7 +492,7 @@ static int __lov_setstripe(struct obd_export *exp, struct lov_stripe_md **lsmp,
                 if (stripe_count > pool_tgt_count(pool))
                         stripe_count = pool_tgt_count(pool);
 
-                lh_put(lov->lov_pools_hash_body, &pool->pool_hash);
+                lov_pool_putref(pool);
         }
 
         if ((__u64)lumv1->lmm_stripe_size * stripe_count > ~0UL) {

diff --git a/lustre/lov/lov_pool.c b/lustre/lov/lov_pool.c
index 764a494..3df37a7 100644 (file)
--- a/lustre/lov/lov_pool.c
+++ b/lustre/lov/lov_pool.c
@@ -38,6 +38,8 @@
  * OST pool methods
  *
  * Author: Jacques-Charles LAFOUCRIERE <jc.lafoucriere@cea.fr>
+ * Author: Alex Lyashkov <Alexey.Lyashkov@Sun.COM>
+ * Author: Nathaniel Rutman <Nathan.Rutman@Sun.COM>
  */
 
 #define DEBUG_SUBSYSTEM S_LOV
@@ -51,15 +53,23 @@
 #include <obd.h>
 #include "lov_internal.h"
 
-static void lov_pool_getref(struct pool_desc *pool) {
+static void lov_pool_getref(struct pool_desc *pool)
+{
+        CDEBUG(D_INFO, "pool %p\n", pool);
         atomic_inc(&pool->pool_refcount);
 }
 
-static void lov_pool_putref(struct pool_desc *pool) {
+void lov_pool_putref(struct pool_desc *pool) 
+{
+        CDEBUG(D_INFO, "pool %p\n", pool);
         if (atomic_dec_and_test(&pool->pool_refcount)) {
+                LASSERT(hlist_unhashed(&pool->pool_hash));
+                LASSERT(list_empty(&pool->pool_list));
+                LASSERT(pool->pool_proc_entry == NULL);
                 lov_ost_pool_free(&(pool->pool_rr.lqr_pool));
                 lov_ost_pool_free(&(pool->pool_obds));
                 OBD_FREE_PTR(pool);
+                EXIT;
         }
 }
 
@@ -302,6 +312,8 @@ void lov_dump_pool(int level, struct pool_desc *pool)
 #define LOV_POOL_INIT_COUNT 2
 int lov_ost_pool_init(struct ost_pool *op, unsigned int count)
 {
+        ENTRY;
+
         if (count == 0)
                 count = LOV_POOL_INIT_COUNT;
         op->op_array = NULL;
@@ -311,8 +323,9 @@ int lov_ost_pool_init(struct ost_pool *op, unsigned int count)
         OBD_ALLOC(op->op_array, op->op_size * sizeof(op->op_array[0]));
         if (op->op_array == NULL) {
                 op->op_size = 0;
-                return -ENOMEM;
+                RETURN(-ENOMEM);
         }
+        EXIT;
         return 0;
 }
 
@@ -359,6 +372,7 @@ int lov_ost_pool_add(struct ost_pool *op, __u32 idx, unsigned int min_count)
         /* ost not found we add it */
         op->op_array[op->op_count] = idx;
         op->op_count++;
+        EXIT;
 out:
         up_write(&op->op_rw_sem);
         return rc;
@@ -367,6 +381,7 @@ out:
 int lov_ost_pool_remove(struct ost_pool *op, __u32 idx)
 {
         int i;
+        ENTRY;
 
         down_write(&op->op_rw_sem);
 
@@ -376,18 +391,21 @@ int lov_ost_pool_remove(struct ost_pool *op, __u32 idx)
                                 (op->op_count - i - 1) * sizeof(op->op_array[0]));
                         op->op_count--;
                         up_write(&op->op_rw_sem);
+                        EXIT;
                         return 0;
                 }
         }
 
         up_write(&op->op_rw_sem);
-        return -EINVAL;
+        RETURN(-EINVAL);
 }
 
 int lov_ost_pool_free(struct ost_pool *op)
 {
+        ENTRY;
+
         if (op->op_size == 0)
-                return 0;
+                RETURN(0);
 
         down_write(&op->op_rw_sem);
 
@@ -397,7 +415,7 @@ int lov_ost_pool_free(struct ost_pool *op)
         op->op_size = 0;
 
         up_write(&op->op_rw_sem);
-        return 0;
+        RETURN(0);
 }
 
 
@@ -430,48 +448,54 @@ int lov_pool_new(struct obd_device *obd, char *poolname)
 
         memset(&(new_pool->pool_rr), 0, sizeof(struct lov_qos_rr));
         rc = lov_ost_pool_init(&new_pool->pool_rr.lqr_pool, 0);
-        if (rc) {
-                lov_ost_pool_free(&new_pool->pool_obds);
-                GOTO(out_err, rc);
-        }
+        if (rc)
+                GOTO(out_free_pool_obds, rc);
 
         INIT_HLIST_NODE(&new_pool->pool_hash);
-        rc = lustre_hash_add_unique(lov->lov_pools_hash_body, poolname,
-                                    &new_pool->pool_hash);
-        if (rc) {
-                lov_ost_pool_free(&new_pool->pool_rr.lqr_pool);
-                lov_ost_pool_free(&new_pool->pool_obds);
-                GOTO(out_err, rc = -EEXIST);
-        }
-
-        spin_lock(&obd->obd_dev_lock);
-        list_add_tail(&new_pool->pool_list, &lov->lov_pool_list);
-        lov->lov_pool_count++;
-
-        spin_unlock(&obd->obd_dev_lock);
-
-        CDEBUG(D_CONFIG, LOV_POOLNAMEF" is pool #%d\n",
-               poolname, lov->lov_pool_count);
 
 #ifdef LPROCFS
-        /* ifdef needed for liblustre */
+        /* we need this assert seq_file is not implementated for liblustre */
         /* get ref for /proc file */
         lov_pool_getref(new_pool);
         new_pool->pool_proc_entry = lprocfs_add_simple(lov->lov_pool_proc_entry,
                                                        poolname, NULL, NULL,
                                                        new_pool,
                                                        &pool_proc_operations);
-#endif
-
         if (IS_ERR(new_pool->pool_proc_entry)) {
                 CWARN("Cannot add proc pool entry "LOV_POOLNAMEF"\n", poolname);
                 new_pool->pool_proc_entry = NULL;
                 lov_pool_putref(new_pool);
         }
+        CDEBUG(D_INFO, "pool %p - proc %p\n", new_pool, new_pool->pool_proc_entry);
+#endif
+
+        spin_lock(&obd->obd_dev_lock);
+        list_add_tail(&new_pool->pool_list, &lov->lov_pool_list);
+        lov->lov_pool_count++;
+        spin_unlock(&obd->obd_dev_lock);
+
+        /* add to find only when it fully ready  */
+        rc = lustre_hash_add_unique(lov->lov_pools_hash_body, poolname,
+                                    &new_pool->pool_hash);
+        if (rc)
+                GOTO(out_err, rc = -EEXIST);
+
+        CDEBUG(D_CONFIG, LOV_POOLNAMEF" is pool #%d\n",
+               poolname, lov->lov_pool_count);
 
         RETURN(0);
 
 out_err:
+        spin_lock(&obd->obd_dev_lock);
+        list_del_init(&new_pool->pool_list);
+        lov->lov_pool_count--;
+        spin_unlock(&obd->obd_dev_lock);
+
+        lprocfs_remove(&new_pool->pool_proc_entry);
+
+        lov_ost_pool_free(&new_pool->pool_rr.lqr_pool);
+out_free_pool_obds:
+        lov_ost_pool_free(&new_pool->pool_obds);
         OBD_FREE_PTR(new_pool);
         return rc;
 }
@@ -484,33 +508,23 @@ int lov_pool_del(struct obd_device *obd, char *poolname)
 
         lov = &(obd->u.lov);
 
-        spin_lock(&obd->obd_dev_lock);
-
-        pool = lustre_hash_lookup(lov->lov_pools_hash_body, poolname);
-        if (pool == NULL) {
-                spin_unlock(&obd->obd_dev_lock);
+        /* lookup and kill hash reference */
+        pool = lustre_hash_del_key(lov->lov_pools_hash_body, poolname);
+        if (pool == NULL)
                 RETURN(-ENOENT);
-        }
 
-#ifdef LPROCFS
         if (pool->pool_proc_entry != NULL) {
-                remove_proc_entry(pool->pool_proc_entry->name,
-                                  pool->pool_proc_entry->parent);
-                /* remove ref for /proc file */
+                CDEBUG(D_INFO, "proc entry %p\n", pool->pool_proc_entry);
+                lprocfs_remove(&pool->pool_proc_entry);
                 lov_pool_putref(pool);
         }
-#endif
 
-        lustre_hash_del_key(lov->lov_pools_hash_body, poolname);
+        spin_lock(&obd->obd_dev_lock);
         list_del_init(&pool->pool_list);
-
         lov->lov_pool_count--;
-        lh_put(lov->lov_pools_hash_body, &pool->pool_hash);
         spin_unlock(&obd->obd_dev_lock);
 
-        /* remove ref got when pool was created in memory
-         * pool will be freed when refount will reach 0
-         */
+        /* release last reference */
         lov_pool_putref(pool);
 
         RETURN(0);
@@ -522,7 +536,7 @@ int lov_pool_add(struct obd_device *obd, char *poolname, char *ostname)
         struct obd_uuid ost_uuid;
         struct lov_obd *lov;
         struct pool_desc *pool;
-        unsigned int i, lov_idx;
+        unsigned int lov_idx;
         int rc;
         ENTRY;
 
@@ -536,22 +550,17 @@ int lov_pool_add(struct obd_device *obd, char *poolname, char *ostname)
 
 
         /* search ost in lov array */
-        mutex_down(&lov->lov_lock);
-        for (i = 0; i < lov->desc.ld_tgt_count; i++) {
-                if (!lov->lov_tgts[i])
+        lov_getref(obd);
+        for (lov_idx = 0; lov_idx < lov->desc.ld_tgt_count; lov_idx++) {
+                if (!lov->lov_tgts[lov_idx])
                         continue;
-                if (obd_uuid_equals(&ost_uuid, &(lov->lov_tgts[i]->ltd_uuid)))
+                if (obd_uuid_equals(&ost_uuid,
+                                    &(lov->lov_tgts[lov_idx]->ltd_uuid)))
                         break;
         }
-
         /* test if ost found in lov */
-        if (i == lov->desc.ld_tgt_count) {
-                mutex_up(&lov->lov_lock);
+        if (lov_idx == lov->desc.ld_tgt_count)
                 GOTO(out, rc = -EINVAL);
-        }
-        mutex_up(&lov->lov_lock);
-
-        lov_idx = i;
 
         rc = lov_ost_pool_add(&pool->pool_obds, lov_idx, lov->lov_tgt_size);
         if (rc)
@@ -564,7 +573,8 @@ int lov_pool_add(struct obd_device *obd, char *poolname, char *ostname)
 
         EXIT;
 out:
-        lh_put(lov->lov_pools_hash_body, &pool->pool_hash);
+        lov_putref(obd);
+        lov_pool_putref(pool);
         return rc;
 }
 
@@ -573,39 +583,32 @@ int lov_pool_remove(struct obd_device *obd, char *poolname, char *ostname)
         struct obd_uuid ost_uuid;
         struct lov_obd *lov;
         struct pool_desc *pool;
-        unsigned int i, lov_idx;
+        unsigned int lov_idx;
         int rc = 0;
         ENTRY;
 
         lov = &(obd->u.lov);
 
-        spin_lock(&obd->obd_dev_lock);
         pool = lustre_hash_lookup(lov->lov_pools_hash_body, poolname);
-        if (pool == NULL) {
-                spin_unlock(&obd->obd_dev_lock);
+        if (pool == NULL)
                 RETURN(-ENOENT);
-        }
 
         obd_str2uuid(&ost_uuid, ostname);
 
+        lov_getref(obd);
         /* search ost in lov array, to get index */
-        for (i = 0; i < lov->desc.ld_tgt_count; i++) {
-                if (!lov->lov_tgts[i])
+        for (lov_idx = 0; lov_idx < lov->desc.ld_tgt_count; lov_idx++) {
+                if (!lov->lov_tgts[lov_idx])
                         continue;
 
-                if (obd_uuid_equals(&ost_uuid, &(lov->lov_tgts[i]->ltd_uuid)))
+                if (obd_uuid_equals(&ost_uuid,
+                                    &(lov->lov_tgts[lov_idx]->ltd_uuid)))
                         break;
         }
 
         /* test if ost found in lov */
-        if (i == lov->desc.ld_tgt_count) {
-                spin_unlock(&obd->obd_dev_lock);
+        if (lov_idx == lov->desc.ld_tgt_count)
                 GOTO(out, rc = -EINVAL);
-        }
-
-        spin_unlock(&obd->obd_dev_lock);
-
-        lov_idx = i;
 
         lov_ost_pool_remove(&pool->pool_obds, lov_idx);
 
@@ -616,7 +619,8 @@ int lov_pool_remove(struct obd_device *obd, char *poolname, char *ostname)
 
         EXIT;
 out:
-        lh_put(lov->lov_pools_hash_body, &pool->pool_hash);
+        lov_putref(obd);
+        lov_pool_putref(pool);
         return rc;
 }
 
@@ -660,7 +664,7 @@ struct pool_desc *lov_find_pool(struct lov_obd *lov, char *poolname)
                         CWARN("Request for an empty pool ("LOV_POOLNAMEF")\n",
                                poolname);
                         /* pool is ignored, so we remove ref on it */
-                        lh_put(lov->lov_pools_hash_body, &pool->pool_hash);
+                        lov_pool_putref(pool);
                         pool = NULL;
                 }
         }

diff --git a/lustre/lov/lov_qos.c b/lustre/lov/lov_qos.c
index d020dd7..94539b3 100644 (file)
--- a/lustre/lov/lov_qos.c
+++ b/lustre/lov/lov_qos.c
@@ -637,7 +637,7 @@ out:
         if (pool != NULL) {
                 up_read(&pool_tgt_rw_sem(pool));
                 /* put back ref got by lov_find_pool() */
-                lh_put(lov->lov_pools_hash_body, &pool->pool_hash);
+                lov_pool_putref(pool);
         }
 
         RETURN(rc);
@@ -729,7 +729,7 @@ out:
         if (pool != NULL) {
                 up_read(&pool_tgt_rw_sem(pool));
                 /* put back ref got by lov_find_pool() */
-                lh_put(lov->lov_pools_hash_body, &pool->pool_hash);
+                lov_pool_putref(pool);
         }
 
         RETURN(rc);
@@ -924,7 +924,7 @@ out_nolock:
         if (pool != NULL) {
                 up_read(&pool_tgt_rw_sem(pool));
                 /* put back ref got by lov_find_pool() */
-                lh_put(lov->lov_pools_hash_body, &pool->pool_hash);
+                lov_pool_putref(pool);
         }
 
         if (rc == -EAGAIN)

diff --git a/lustre/ptlrpc/recov_thread.c b/lustre/ptlrpc/recov_thread.c
index dc4b13b..e90142b 100644 (file)
--- a/lustre/ptlrpc/recov_thread.c
+++ b/lustre/ptlrpc/recov_thread.c
@@ -591,6 +591,7 @@ int llog_obd_repl_cancel(struct llog_ctxt *ctxt,
 
         mutex_down(&ctxt->loc_sem);
         lcm = ctxt->loc_lcm;
+        CDEBUG(D_INFO, "cancel on lsm %p\n", lcm);
 
         /*
          * Let's check if we have all structures alive. We also check for