From 9ec5e2329bf3d7e38fa899a259221aa58fb48cd4 Mon Sep 17 00:00:00 2001 From: Mr NeilBrown Date: Tue, 28 Sep 2021 16:44:40 +1000 Subject: [PATCH] LU-6142 lod: return pools_hash_params to being static. A recent patch changes pools_hash_params in lod_pool.c to no longer be 'static'. This is not ideal. rhashtable interfaces are mostly 'static inlines' which contain a lot of code which is mostly optimised away providing that the 'params' structure is const and locally visible. When these interfaces are called with a params structure in another file, the code produces is quite inefficient and wasteful. It is generally cleaner to provide accessor functions which can be exported to other compilation units. It is even beneficial to do that within the one file. This patch introduces lod_pool_exists() and lod_pool_find() The first is 'extern' and thus 'pools_hash_params' can not be static. The second is used in several places in lod_pool.c, improving code quality and maintainability. Fixes: 0a998f4723f5 ("LU-14825 lod: pool spilling") Signed-off-by: Mr NeilBrown Change-Id: Ieafe2f23fe5cc71d9bdce73cbe7360f5cb540edf Reviewed-on: https://review.whamcloud.com/45070 Reviewed-by: James Simmons Reviewed-by: Alex Zhuravlev Reviewed-by: Andreas Dilger Tested-by: jenkins Tested-by: Maloo --- lustre/lod/lod_internal.h | 3 ++- lustre/lod/lod_pool.c | 55 ++++++++++++++++++++++++++--------------------- lustre/lod/lproc_lod.c | 9 ++------ 3 files changed, 34 insertions(+), 33 deletions(-) diff --git a/lustre/lod/lod_internal.h b/lustre/lod/lod_internal.h index 3060e7a..4d8a0ed 100644 --- a/lustre/lod/lod_internal.h +++ b/lustre/lod/lod_internal.h @@ -70,9 +70,10 @@ struct pool_desc { char pool_spill_target[LOV_MAXPOOLNAME + 1]; }; +struct lod_device; int lod_pool_hash_init(struct rhashtable *tbl); void lod_pool_hash_destroy(struct rhashtable *tbl); -extern const struct rhashtable_params pools_hash_params; +bool lod_pool_exists(struct lod_device *lod, char *poolname); #define pool_tgt_count(p) ((p)->pool_obds.op_count) #define pool_tgt_array(p) ((p)->pool_obds.op_array) diff --git a/lustre/lod/lod_pool.c b/lustre/lod/lod_pool.c index 18bb313..30bebf7 100644 --- a/lustre/lod/lod_pool.c +++ b/lustre/lod/lod_pool.c @@ -122,7 +122,7 @@ static int pool_cmpfn(struct rhashtable_compare_arg *arg, const void *obj) return strcmp(pool_name, pool->pool_name); } -const struct rhashtable_params pools_hash_params = { +static const struct rhashtable_params pools_hash_params = { .key_len = 1, /* actually variable */ .key_offset = offsetof(struct pool_desc, pool_name), .head_offset = offsetof(struct pool_desc, pool_hash), @@ -376,6 +376,31 @@ void lod_pool_hash_destroy(struct rhashtable *tbl) rhashtable_free_and_destroy(tbl, pools_hash_exit, NULL); } +bool lod_pool_exists(struct lod_device *lod, char *poolname) +{ + struct pool_desc *pool; + + rcu_read_lock(); + pool = rhashtable_lookup(&lod->lod_pools_hash_body, + poolname, + pools_hash_params); + rcu_read_unlock(); + return pool != NULL; +} + +static struct pool_desc *lod_pool_find(struct lod_device *lod, char *poolname) +{ + struct pool_desc *pool; + + rcu_read_lock(); + pool = rhashtable_lookup(&lod->lod_pools_hash_body, + poolname, + pools_hash_params); + if (pool && !atomic_inc_not_zero(&pool->pool_refcount)) + pool = NULL; + rcu_read_unlock(); + return pool; +} /** * Allocate a new pool for the specified device. * @@ -558,12 +583,7 @@ int lod_pool_add(struct obd_device *obd, char *poolname, char *ostname) int rc = -EINVAL; ENTRY; - rcu_read_lock(); - pool = rhashtable_lookup(&lod->lod_pools_hash_body, poolname, - pools_hash_params); - if (pool && !atomic_inc_not_zero(&pool->pool_refcount)) - pool = NULL; - rcu_read_unlock(); + pool = lod_pool_find(lod, poolname); if (!pool) RETURN(-ENOENT); @@ -622,12 +642,7 @@ int lod_pool_remove(struct obd_device *obd, char *poolname, char *ostname) ENTRY; /* lookup and kill hash reference */ - rcu_read_lock(); - pool = rhashtable_lookup(&lod->lod_pools_hash_body, poolname, - pools_hash_params); - if (pool && !atomic_inc_not_zero(&pool->pool_refcount)) - pool = NULL; - rcu_read_unlock(); + pool = lod_pool_find(lod, poolname); if (!pool) RETURN(-ENOENT); @@ -696,12 +711,7 @@ struct pool_desc *lod_find_pool(struct lod_device *lod, char *poolname) pool = NULL; if (poolname[0] != '\0') { - rcu_read_lock(); - pool = rhashtable_lookup(&lod->lod_pools_hash_body, poolname, - pools_hash_params); - if (pool && !atomic_inc_not_zero(&pool->pool_refcount)) - pool = NULL; - rcu_read_unlock(); + pool = lod_pool_find(lod, poolname); if (!pool) CDEBUG(D_CONFIG, "%s: request for an unknown pool (" LOV_POOLNAMEF ")\n", @@ -782,12 +792,7 @@ void lod_check_and_spill_pool(const struct lu_env *env, struct lod_device *lod, if (!poolname || !*poolname || (*poolname)[0] == '\0') return; repeat: - rcu_read_lock(); - pool = rhashtable_lookup(&lod->lod_pools_hash_body, *poolname, - pools_hash_params); - if (pool && !atomic_inc_not_zero(&pool->pool_refcount)) - pool = NULL; - rcu_read_unlock(); + pool = lod_pool_find(lod, *poolname); if (!pool) return; diff --git a/lustre/lod/lproc_lod.c b/lustre/lod/lproc_lod.c index 6949eb9..2ef96c7 100644 --- a/lustre/lod/lproc_lod.c +++ b/lustre/lod/lproc_lod.c @@ -1086,7 +1086,7 @@ lod_spill_target_seq_write(struct file *file, const char __user *buffer, size_t count, loff_t *off) { struct seq_file *m = file->private_data; - struct pool_desc *tgt, *pool = m->private; + struct pool_desc *pool = m->private; struct lod_device *lod; LASSERT(pool != NULL); @@ -1106,12 +1106,7 @@ lod_spill_target_seq_write(struct file *file, const char __user *buffer, pool->pool_spill_target[count] = '\0'; if (strcmp(pool->pool_name, pool->pool_spill_target) == 0) return -ELOOP; - rcu_read_lock(); - tgt = rhashtable_lookup(&lod->lod_pools_hash_body, - pool->pool_spill_target, - pools_hash_params); - rcu_read_unlock(); - if (!tgt) { + if (!lod_pool_exists(lod, pool->pool_spill_target)) { pool->pool_spill_target[0] = '\0'; pool->pool_spill_expire = 0; return -ENODEV; -- 1.8.3.1