From: Vitaly Fertman Date: Thu, 15 Apr 2010 05:39:41 +0000 (-0700) Subject: b=22056 hash_add not under spinlock. X-Git-Tag: 1.10.0.41~48 X-Git-Url: https://git.whamcloud.com/gitweb?a=commitdiff_plain;h=589bc6c478b91d2d0e4cdd2aefd83dd45be2ef51;p=fs%2Flustre-release.git b=22056 hash_add not under spinlock. a refcount is added to the hash, move hash_add from under spinlock with no race against hash_destroy. i=green i=tappro --- diff --git a/libcfs/include/libcfs/libcfs_hash.h b/libcfs/include/libcfs/libcfs_hash.h index 73cfe01..82673de 100644 --- a/libcfs/include/libcfs/libcfs_hash.h +++ b/libcfs/include/libcfs/libcfs_hash.h @@ -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, diff --git a/libcfs/libcfs/hash.c b/libcfs/libcfs/hash.c index 4aca010..e670172 100644 --- a/libcfs/libcfs/hash.c +++ b/libcfs/libcfs/hash.c @@ -57,6 +57,8 @@ #include +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) diff --git a/lustre/ldlm/ldlm_lockd.c b/lustre/ldlm/ldlm_lockd.c index bf49a20..6268019 100644 --- a/lustre/ldlm/ldlm_lockd.c +++ b/lustre/ldlm/ldlm_lockd.c @@ -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; } diff --git a/lustre/lov/lov_obd.c b/lustre/lov/lov_obd.c index 6d98f6b..53a0343 100644 --- a/lustre/lov/lov_obd.c +++ b/lustre/lov/lov_obd.c @@ -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); diff --git a/lustre/obdclass/cl_object.c b/lustre/obdclass/cl_object.c index 1971baa..f8ef9f7 100644 --- a/lustre/obdclass/cl_object.c +++ b/lustre/obdclass/cl_object.c @@ -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); } diff --git a/lustre/obdclass/genops.c b/lustre/obdclass/genops.c index a8c6c34..5012b39 100644 --- a/lustre/obdclass/genops.c +++ b/lustre/obdclass/genops.c @@ -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); diff --git a/lustre/obdclass/obd_config.c b/lustre/obdclass/obd_config.c index 93e9ab8..00e672f 100644 --- a/lustre/obdclass/obd_config.c +++ b/lustre/obdclass/obd_config.c @@ -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; } diff --git a/lustre/ptlrpc/connection.c b/lustre/ptlrpc/connection.c index a41b202..e27c226 100644 --- a/lustre/ptlrpc/connection.c +++ b/lustre/ptlrpc/connection.c @@ -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; } diff --git a/lustre/quota/quota_adjust_qunit.c b/lustre/quota/quota_adjust_qunit.c index 6d0c262..7d82116 100644 --- a/lustre/quota/quota_adjust_qunit.c +++ b/lustre/quota/quota_adjust_qunit.c @@ -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) diff --git a/lustre/quota/quota_context.c b/lustre/quota/quota_context.c index 859deee..d9d7be0 100644 --- a/lustre/quota/quota_context.c +++ b/lustre/quota/quota_context.c @@ -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);