Whamcloud - gitweb
b=22056 hash_add not under spinlock.
authorVitaly Fertman <Vitaly.Fertman@sun.com>
Thu, 15 Apr 2010 05:39:41 +0000 (22:39 -0700)
committerRobert Read <robert.read@oracle.com>
Thu, 15 Apr 2010 05:39:41 +0000 (22:39 -0700)
a refcount is added to the hash, move hash_add from under spinlock with no race against hash_destroy.

i=green
i=tappro

libcfs/include/libcfs/libcfs_hash.h
libcfs/libcfs/hash.c
lustre/ldlm/ldlm_lockd.c
lustre/lov/lov_obd.c
lustre/obdclass/cl_object.c
lustre/obdclass/genops.c
lustre/obdclass/obd_config.c
lustre/ptlrpc/connection.c
lustre/quota/quota_adjust_qunit.c
lustre/quota/quota_context.c

index 73cfe01..82673de 100644 (file)
@@ -141,6 +141,7 @@ typedef struct cfs_hash {
         struct cfs_hash_bucket    **hs_buckets;     /* hash buckets */
         struct cfs_hash_ops        *hs_ops;         /* hash operations */
         cfs_rwlock_t                hs_rwlock;      /* cfs_hash */
+        cfs_atomic_t                hs_refcount;
         char                        hs_name[CFS_MAX_HASH_NAME];
 } cfs_hash_t;
 
@@ -304,7 +305,8 @@ __cfs_hash_bucket_del(cfs_hash_t *hs,
 cfs_hash_t *cfs_hash_create(char *name, unsigned int cur_bits,
                             unsigned int max_bits,
                             cfs_hash_ops_t *ops, int flags);
-void cfs_hash_destroy(cfs_hash_t *hs);
+cfs_hash_t *cfs_hash_getref(cfs_hash_t *hs);
+void cfs_hash_putref(cfs_hash_t *hs);
 
 /* Hash addition functions */
 void cfs_hash_add(cfs_hash_t *hs, void *key,
index 4aca010..e670172 100644 (file)
@@ -57,6 +57,8 @@
 
 #include <libcfs/libcfs.h>
 
+static void cfs_hash_destroy(cfs_hash_t *hs);
+
 static void
 cfs_hash_rlock(cfs_hash_t *hs)
 {
@@ -117,6 +119,7 @@ cfs_hash_create(char *name, unsigned int cur_bits,
         strncpy(hs->hs_name, name, sizeof(hs->hs_name));
         hs->hs_name[sizeof(hs->hs_name) - 1] = '\0';
         cfs_atomic_set(&hs->hs_rehash_count, 0);
+        cfs_atomic_set(&hs->hs_refcount, 1);
         cfs_atomic_set(&hs->hs_count, 0);
         cfs_rwlock_init(&hs->hs_rwlock);
         hs->hs_cur_bits = cur_bits;
@@ -159,7 +162,7 @@ CFS_EXPORT_SYMBOL(cfs_hash_create);
 /**
  * Cleanup libcfs hash @hs.
  */
-void
+static void
 cfs_hash_destroy(cfs_hash_t *hs)
 {
         cfs_hash_bucket_t    *hsb;
@@ -197,7 +200,21 @@ cfs_hash_destroy(cfs_hash_t *hs)
         CFS_FREE_PTR(hs);
         EXIT;
 }
-CFS_EXPORT_SYMBOL(cfs_hash_destroy);
+
+cfs_hash_t *cfs_hash_getref(cfs_hash_t *hs)
+{
+        if (cfs_atomic_inc_not_zero(&hs->hs_refcount))
+                return hs;
+        return NULL;
+}
+CFS_EXPORT_SYMBOL(cfs_hash_getref);
+
+void cfs_hash_putref(cfs_hash_t *hs)
+{
+        if (cfs_atomic_dec_and_test(&hs->hs_refcount))
+                cfs_hash_destroy(hs);
+}
+CFS_EXPORT_SYMBOL(cfs_hash_putref);
 
 static inline unsigned int
 cfs_hash_rehash_bits(cfs_hash_t *hs)
index bf49a20..6268019 100644 (file)
@@ -2299,7 +2299,7 @@ EXPORT_SYMBOL(ldlm_init_export);
 void ldlm_destroy_export(struct obd_export *exp)
 {
         ENTRY;
-        cfs_hash_destroy(exp->exp_lock_hash);
+        cfs_hash_putref(exp->exp_lock_hash);
         exp->exp_lock_hash = NULL;
         EXIT;
 }
index 6d98f6b..53a0343 100644 (file)
@@ -874,7 +874,7 @@ static int lov_cleanup(struct obd_device *obd)
                 CDEBUG(D_INFO, "delete pool %p\n", pool);
                 lov_pool_del(obd, pool->pool_name);
         }
-        cfs_hash_destroy(lov->lov_pools_hash_body);
+        cfs_hash_putref(lov->lov_pools_hash_body);
         lov_ost_pool_free(&(lov->lov_qos.lq_rr.lqr_pool));
         lov_ost_pool_free(&lov->lov_packed);
 
index 1971baa..f8ef9f7 100644 (file)
@@ -1155,7 +1155,7 @@ int cl_global_init(void)
                 }
         }
         if (result)
-                cfs_hash_destroy(cl_env_hash);
+                cfs_hash_putref(cl_env_hash);
         return result;
 }
 
@@ -1168,5 +1168,5 @@ void cl_global_fini(void)
         cl_page_fini();
         lu_context_key_degister(&cl_key);
         lu_kmem_fini(cl_object_caches);
-        cfs_hash_destroy(cl_env_hash);
+        cfs_hash_putref(cl_env_hash);
 }
index a8c6c34..5012b39 100644 (file)
@@ -772,6 +772,7 @@ struct obd_export *class_new_export(struct obd_device *obd,
                                     struct obd_uuid *cluuid)
 {
         struct obd_export *export;
+        cfs_hash_t *hash;
         int rc = 0;
         ENTRY;
 
@@ -811,11 +812,15 @@ struct obd_export *class_new_export(struct obd_device *obd,
         cfs_spin_lock(&obd->obd_dev_lock);
          /* shouldn't happen, but might race */
         if (obd->obd_stopping)
-                GOTO(exit_err, rc = -ENODEV);
+                GOTO(exit_unlock, rc = -ENODEV);
+
+        hash = cfs_hash_getref(obd->obd_uuid_hash);
+        if (hash == NULL)
+                GOTO(exit_unlock, rc = -ENODEV);
+        cfs_spin_unlock(&obd->obd_dev_lock);
 
         if (!obd_uuid_equals(cluuid, &obd->obd_uuid)) {
-                rc = cfs_hash_add_unique(obd->obd_uuid_hash, cluuid,
-                                         &export->exp_uuid_hash);
+                rc = cfs_hash_add_unique(hash, cluuid, &export->exp_uuid_hash);
                 if (rc != 0) {
                         LCONSOLE_WARN("%s: denying duplicate export for %s, %d\n",
                                       obd->obd_name, cluuid->uuid, rc);
@@ -823,6 +828,12 @@ struct obd_export *class_new_export(struct obd_device *obd,
                 }
         }
 
+        cfs_hash_putref(hash);
+
+        cfs_spin_lock(&obd->obd_dev_lock);
+        if (obd->obd_stopping)
+                GOTO(exit_unlock, rc = -ENODEV);
+
         class_incref(obd, "export", export);
         cfs_list_add(&export->exp_obd_chain, &export->exp_obd->obd_exports);
         cfs_list_add_tail(&export->exp_obd_chain_timed,
@@ -831,8 +842,9 @@ struct obd_export *class_new_export(struct obd_device *obd,
         cfs_spin_unlock(&obd->obd_dev_lock);
         RETURN(export);
 
-exit_err:
+exit_unlock:
         cfs_spin_unlock(&obd->obd_dev_lock);
+exit_err:
         class_handle_unhash(&export->exp_handle);
         LASSERT(cfs_hlist_unhashed(&export->exp_uuid_hash));
         obd_destroy_export(export);
index 93e9ab8..00e672f 100644 (file)
@@ -409,15 +409,15 @@ err_exp:
         }
 err_hash:
         if (obd->obd_uuid_hash) {
-                cfs_hash_destroy(obd->obd_uuid_hash);
+                cfs_hash_putref(obd->obd_uuid_hash);
                 obd->obd_uuid_hash = NULL;
         }
         if (obd->obd_nid_hash) {
-                cfs_hash_destroy(obd->obd_nid_hash);
+                cfs_hash_putref(obd->obd_nid_hash);
                 obd->obd_nid_hash = NULL;
         }
         if (obd->obd_nid_stats_hash) {
-                cfs_hash_destroy(obd->obd_nid_stats_hash);
+                cfs_hash_putref(obd->obd_nid_stats_hash);
                 obd->obd_nid_stats_hash = NULL;
         }
         obd->obd_starting = 0;
@@ -516,19 +516,19 @@ int class_cleanup(struct obd_device *obd, struct lustre_cfg *lcfg)
 
         /* destroy an uuid-export hash body */
         if (obd->obd_uuid_hash) {
-                cfs_hash_destroy(obd->obd_uuid_hash);
+                cfs_hash_putref(obd->obd_uuid_hash);
                 obd->obd_uuid_hash = NULL;
         }
 
         /* destroy a nid-export hash body */
         if (obd->obd_nid_hash) {
-                cfs_hash_destroy(obd->obd_nid_hash);
+                cfs_hash_putref(obd->obd_nid_hash);
                 obd->obd_nid_hash = NULL;
         }
 
         /* destroy a nid-stats hash body */
         if (obd->obd_nid_stats_hash) {
-                cfs_hash_destroy(obd->obd_nid_stats_hash);
+                cfs_hash_putref(obd->obd_nid_stats_hash);
                 obd->obd_nid_stats_hash = NULL;
         }
 
index a41b202..e27c226 100644 (file)
@@ -154,7 +154,7 @@ int ptlrpc_connection_init(void)
 
 void ptlrpc_connection_fini(void) {
         ENTRY;
-        cfs_hash_destroy(conn_hash);
+        cfs_hash_putref(conn_hash);
         EXIT;
 }
 
index 6d0c262..7d82116 100644 (file)
@@ -94,6 +94,7 @@ static struct lustre_qunit_size *
 quota_create_lqs(unsigned long long lqs_key, struct lustre_quota_ctxt *qctxt)
 {
         struct lustre_qunit_size *lqs = NULL;
+        cfs_hash_t *hs = NULL;
         int rc = 0;
 
         OBD_ALLOC_PTR(lqs);
@@ -121,15 +122,20 @@ quota_create_lqs(unsigned long long lqs_key, struct lustre_quota_ctxt *qctxt)
         lqs_initref(lqs);
 
         cfs_spin_lock(&qctxt->lqc_lock);
-        if (!qctxt->lqc_valid)
-                rc = -EBUSY;
-        else
-                rc = cfs_hash_add_unique(qctxt->lqc_lqs_hash,
-                                         &lqs->lqs_key, &lqs->lqs_hash);
+        if (qctxt->lqc_valid)
+                hs = cfs_hash_getref(qctxt->lqc_lqs_hash);
         cfs_spin_unlock(&qctxt->lqc_lock);
 
-        if (!rc)
+        if (hs) {
                 lqs_getref(lqs);
+                rc = cfs_hash_add_unique(qctxt->lqc_lqs_hash,
+                                         &lqs->lqs_key, &lqs->lqs_hash);
+                if (rc)
+                        lqs_putref(lqs);
+                cfs_hash_putref(hs);
+        } else {
+                rc = -EBUSY;
+        }
 
  out:
         if (rc && lqs)
index 859deee..d9d7be0 100644 (file)
@@ -1348,7 +1348,7 @@ void qctxt_cleanup(struct lustre_quota_ctxt *qctxt, int force)
         cfs_hash_for_each_safe(qctxt->lqc_lqs_hash, hash_put_lqs, NULL);
         l_wait_event(qctxt->lqc_lqs_waitq, check_lqs(qctxt), &lwi);
         cfs_down_write(&obt->obt_rwsem);
-        cfs_hash_destroy(qctxt->lqc_lqs_hash);
+        cfs_hash_putref(qctxt->lqc_lqs_hash);
         qctxt->lqc_lqs_hash = NULL;
         cfs_up_write(&obt->obt_rwsem);