Whamcloud - gitweb
LU-6142 lod: return pools_hash_params to being static. 70/45070/4
authorMr NeilBrown <neilb@suse.de>
Tue, 28 Sep 2021 06:44:40 +0000 (16:44 +1000)
committerOleg Drokin <green@whamcloud.com>
Sun, 10 Oct 2021 03:30:36 +0000 (03:30 +0000)
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 <neilb@suse.de>
Change-Id: Ieafe2f23fe5cc71d9bdce73cbe7360f5cb540edf
Reviewed-on: https://review.whamcloud.com/45070
Reviewed-by: James Simmons <jsimmons@infradead.org>
Reviewed-by: Alex Zhuravlev <bzzz@whamcloud.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
lustre/lod/lod_internal.h
lustre/lod/lod_pool.c
lustre/lod/lproc_lod.c

index 3060e7a..4d8a0ed 100644 (file)
@@ -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)
index 18bb313..30bebf7 100644 (file)
@@ -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;
 
index 6949eb9..2ef96c7 100644 (file)
@@ -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;