LU-6304 ldlm: crash on umount in cleanup_resource

cfs_hash_for_each_relax() assumes that cfs_hash_put_locked()
doesn't release bd lock, but it isn't true for
ldlm_res_hop_put_locked().

Add recfcount on next hnode in cfs_hash_for_each_relax() and
remove ldlm_res_hop_put_locked()

Change-Id: I275c0d29737119d084205e604c336a42e489bfe7
Xyratex-bug-id: MRP-2352
Signed-off-by: Andriy Skulysh <andriy.skulysh@seagate.com>
Reviewed-by: Vitaly Fertman <vitaly.fertman@seagate.com>
Reviewed-by: Alexander Boyko <alexander.boyko@seagate.com>
Tested-by: Alexander Lezhoev <alexander.lezhoev@seagate.com>
Reviewed-on: http://review.whamcloud.com/13908
Tested-by: Jenkins
Tested-by: Maloo <hpdd-maloo@intel.com>
Reviewed-by: Oleg Drokin <oleg.drokin@intel.com>

diff --git a/libcfs/libcfs/hash.c b/libcfs/libcfs/hash.c
index c3d662f..9c37ee1 100644 (file)
--- a/libcfs/libcfs/hash.c
+++ b/libcfs/libcfs/hash.c
@@ -1038,7 +1038,7 @@ cfs_hash_create(char *name, unsigned cur_bits, unsigned max_bits,
         LASSERT(ops->hs_object);
         LASSERT(ops->hs_keycmp);
         LASSERT(ops->hs_get != NULL);
         LASSERT(ops->hs_object);
         LASSERT(ops->hs_keycmp);
         LASSERT(ops->hs_get != NULL);

-        LASSERT(ops->hs_put_locked != NULL);
+       LASSERT(ops->hs_put != NULL || ops->hs_put_locked != NULL);

 
         if ((flags & CFS_HASH_REHASH) != 0)
                 flags |= CFS_HASH_COUNTER; /* must have counter */
 
         if ((flags & CFS_HASH_REHASH) != 0)
                 flags |= CFS_HASH_COUNTER; /* must have counter */


@@ -1589,18 +1589,19 @@ cfs_hash_for_each_relax(struct cfs_hash *hs, cfs_hash_for_each_cb_t func,
                        void *data, int start)
 {
        struct hlist_node       *hnode;
                        void *data, int start)
 {
        struct hlist_node       *hnode;

-       struct hlist_node       *tmp;
+       struct hlist_node       *next = NULL;

        struct cfs_hash_bd      bd;
        __u32                   version;
        int                     count = 0;
        int                     stop_on_change;
        struct cfs_hash_bd      bd;
        __u32                   version;
        int                     count = 0;
        int                     stop_on_change;

+       int                     has_put_locked;

        int                     rc = 0;
        int                     i, end = -1;
        ENTRY;
 
        stop_on_change = cfs_hash_with_rehash_key(hs) ||
        int                     rc = 0;
        int                     i, end = -1;
        ENTRY;
 
        stop_on_change = cfs_hash_with_rehash_key(hs) ||

-                        !cfs_hash_with_no_itemref(hs) ||
-                        hs->hs_ops->hs_put_locked == NULL;
+                        !cfs_hash_with_no_itemref(hs);
+       has_put_locked = hs->hs_ops->hs_put_locked != NULL;

        cfs_hash_lock(hs, 0);
 again:
        LASSERT(!cfs_hash_is_rehashing(hs));
        cfs_hash_lock(hs, 0);
 again:
        LASSERT(!cfs_hash_is_rehashing(hs));


@@ -1617,38 +1618,52 @@ again:
                 version = cfs_hash_bd_version_get(&bd);
 
                 cfs_hash_bd_for_each_hlist(hs, &bd, hhead) {
                 version = cfs_hash_bd_version_get(&bd);
 
                 cfs_hash_bd_for_each_hlist(hs, &bd, hhead) {

-                        for (hnode = hhead->first; hnode != NULL;) {
-                                cfs_hash_bucket_validate(hs, &bd, hnode);
-                                cfs_hash_get(hs, hnode);
+                       hnode = hhead->first;
+                       if (hnode == NULL)
+                               continue;
+                       cfs_hash_get(hs, hnode);
+                       for (; hnode != NULL; hnode = next) {
+                               cfs_hash_bucket_validate(hs, &bd, hnode);
+                               next = hnode->next;
+                               if (next != NULL)
+                                       cfs_hash_get(hs, next);

                                 cfs_hash_bd_unlock(hs, &bd, 0);
                                 cfs_hash_unlock(hs, 0);
 
                                rc = func(hs, &bd, hnode, data);
                                 cfs_hash_bd_unlock(hs, &bd, 0);
                                 cfs_hash_unlock(hs, 0);
 
                                rc = func(hs, &bd, hnode, data);

-                               if (stop_on_change)
+                               if (stop_on_change || !has_put_locked)

                                        cfs_hash_put(hs, hnode);
                                        cfs_hash_put(hs, hnode);

+

                                cond_resched();
                                count++;
 
                                 cfs_hash_lock(hs, 0);
                                 cfs_hash_bd_lock(hs, &bd, 0);
                                cond_resched();
                                count++;
 
                                 cfs_hash_lock(hs, 0);
                                 cfs_hash_bd_lock(hs, &bd, 0);

-                                if (!stop_on_change) {
-                                        tmp = hnode->next;
-                                        cfs_hash_put_locked(hs, hnode);
-                                        hnode = tmp;
-                                } else { /* bucket changed? */
-                                        if (version !=
-                                            cfs_hash_bd_version_get(&bd))
-                                                break;
-                                        /* safe to continue because no change */
-                                        hnode = hnode->next;
-                                }
+                               if (stop_on_change) {
+                                       if (version !=
+                                           cfs_hash_bd_version_get(&bd))
+                                               rc = -EINTR;
+                               } else if (has_put_locked) {
+                                       cfs_hash_put_locked(hs, hnode);
+                               }

                                 if (rc) /* callback wants to break iteration */
                                         break;
                         }
                                 if (rc) /* callback wants to break iteration */
                                         break;
                         }

-                       if (rc) /* callback wants to break iteration */
+                       if (next != NULL) {
+                               if (has_put_locked) {
+                                       cfs_hash_put_locked(hs, next);
+                                       next = NULL;
+                               }

                                break;
                                break;

+                       } else if (rc != 0) {
+                               break;
+                       }

                 }
                 cfs_hash_bd_unlock(hs, &bd, 0);
                 }
                 cfs_hash_bd_unlock(hs, &bd, 0);

+               if (next != NULL && !has_put_locked) {
+                       cfs_hash_put(hs, next);
+                       next = NULL;
+               }

                if (rc) /* callback wants to break iteration */
                        break;
         }
                if (rc) /* callback wants to break iteration */
                        break;
         }

diff --git a/lustre/ldlm/ldlm_internal.h b/lustre/ldlm/ldlm_internal.h
index c997425..3107b24 100644 (file)
--- a/lustre/ldlm/ldlm_internal.h
+++ b/lustre/ldlm/ldlm_internal.h
@@ -123,7 +123,6 @@ extern struct kmem_cache *ldlm_resource_slab;
 extern struct kmem_cache *ldlm_lock_slab;
 extern struct kmem_cache *ldlm_interval_tree_slab;
 
 extern struct kmem_cache *ldlm_lock_slab;
 extern struct kmem_cache *ldlm_interval_tree_slab;
 

-int ldlm_resource_putref_locked(struct ldlm_resource *res);

 void ldlm_resource_insert_lock_after(struct ldlm_lock *original,
                                      struct ldlm_lock *new);
 
 void ldlm_resource_insert_lock_after(struct ldlm_lock *original,
                                      struct ldlm_lock *new);
 

diff --git a/lustre/ldlm/ldlm_resource.c b/lustre/ldlm/ldlm_resource.c
index 7ed21e3..d8d6db2 100644 (file)
--- a/lustre/ldlm/ldlm_resource.c
+++ b/lustre/ldlm/ldlm_resource.c
@@ -571,16 +571,6 @@ ldlm_res_hop_get_locked(struct cfs_hash *hs, struct hlist_node *hnode)
         ldlm_resource_getref(res);
 }
 
         ldlm_resource_getref(res);
 }
 

-static void
-ldlm_res_hop_put_locked(struct cfs_hash *hs, struct hlist_node *hnode)
-{
-        struct ldlm_resource *res;
-
-       res = hlist_entry(hnode, struct ldlm_resource, lr_hash);
-        /* cfs_hash_for_each_nolock is the only chance we call it */
-        ldlm_resource_putref_locked(res);
-}
-

 static void ldlm_res_hop_put(struct cfs_hash *hs, struct hlist_node *hnode)
 {
         struct ldlm_resource *res;
 static void ldlm_res_hop_put(struct cfs_hash *hs, struct hlist_node *hnode)
 {
         struct ldlm_resource *res;


@@ -596,7 +586,6 @@ static struct cfs_hash_ops ldlm_ns_hash_ops = {
         .hs_keycpy      = NULL,
         .hs_object      = ldlm_res_hop_object,
         .hs_get         = ldlm_res_hop_get_locked,
         .hs_keycpy      = NULL,
         .hs_object      = ldlm_res_hop_object,
         .hs_get         = ldlm_res_hop_get_locked,

-        .hs_put_locked  = ldlm_res_hop_put_locked,

         .hs_put         = ldlm_res_hop_put
 };
 
         .hs_put         = ldlm_res_hop_put
 };
 


@@ -607,7 +596,6 @@ static struct cfs_hash_ops ldlm_ns_fid_hash_ops = {
         .hs_keycpy      = NULL,
         .hs_object      = ldlm_res_hop_object,
         .hs_get         = ldlm_res_hop_get_locked,
         .hs_keycpy      = NULL,
         .hs_object      = ldlm_res_hop_object,
         .hs_get         = ldlm_res_hop_get_locked,

-        .hs_put_locked  = ldlm_res_hop_put_locked,

         .hs_put         = ldlm_res_hop_put
 };
 
         .hs_put         = ldlm_res_hop_put
 };
 


@@ -1326,40 +1314,6 @@ int ldlm_resource_putref(struct ldlm_resource *res)
 }
 EXPORT_SYMBOL(ldlm_resource_putref);
 
 }
 EXPORT_SYMBOL(ldlm_resource_putref);
 

-/* Returns 1 if the resource was freed, 0 if it remains. */
-int ldlm_resource_putref_locked(struct ldlm_resource *res)
-{
-       struct ldlm_namespace *ns = ldlm_res_to_ns(res);
-
-       LASSERT_ATOMIC_GT_LT(&res->lr_refcount, 0, LI_POISON);
-       CDEBUG(D_INFO, "putref res: %p count: %d\n",
-              res, atomic_read(&res->lr_refcount) - 1);
-
-       if (atomic_dec_and_test(&res->lr_refcount)) {
-               struct cfs_hash_bd bd;
-
-               cfs_hash_bd_get(ldlm_res_to_ns(res)->ns_rs_hash,
-                               &res->lr_name, &bd);
-               __ldlm_resource_putref_final(&bd, res);
-               cfs_hash_bd_unlock(ns->ns_rs_hash, &bd, 1);
-               /* NB: ns_rs_hash is created with CFS_HASH_NO_ITEMREF,
-                * so we should never be here while calling cfs_hash_del,
-                * cfs_hash_for_each_nolock is the only case we can get
-                * here, which is safe to release cfs_hash_bd_lock.
-                */
-               if (ns->ns_lvbo && ns->ns_lvbo->lvbo_free)
-                       ns->ns_lvbo->lvbo_free(res);
-               if (res->lr_itree != NULL)
-                       OBD_SLAB_FREE(res->lr_itree, ldlm_interval_tree_slab,
-                                     sizeof(*res->lr_itree) * LCK_MODE_NUM);
-               OBD_SLAB_FREE(res, ldlm_resource_slab, sizeof *res);
-
-               cfs_hash_bd_lock(ns->ns_rs_hash, &bd, 1);
-               return 1;
-       }
-       return 0;
-}
-

 /**
  * Add a lock into a given resource into specified lock list.
  */
 /**
  * Add a lock into a given resource into specified lock list.
  */