LU-15043 lod: check for spilling loops

at setting to avoid possible confusion.

Signed-off-by: Alex Zhuravlev <bzzz@whamcloud.com>
Change-Id: I901b08f614c162607b1b5c6a992aa5b188fd8e75
Reviewed-on: https://review.whamcloud.com/45083
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Patrick Farrell <pfarrell@whamcloud.com>
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

diff --git a/lustre/lod/lod_internal.h b/lustre/lod/lod_internal.h
index aeff630..dbeee1b 100644 (file)
--- a/lustre/lod/lod_internal.h
+++ b/lustre/lod/lod_internal.h
@@ -840,5 +840,6 @@ void lod_check_and_spill_pool(const struct lu_env *env, struct lod_device *lod,
                              char **poolname);
 void lod_spill_target_refresh(const struct lu_env *env, struct lod_device *lod,
                              struct pool_desc *pool);
+struct pool_desc *lod_pool_find(struct lod_device *lod, char *poolname);
 extern struct lprocfs_vars lprocfs_lod_spill_vars[];
 #endif

diff --git a/lustre/lod/lod_pool.c b/lustre/lod/lod_pool.c
index 2d059bc..4b135b4 100644 (file)
--- a/lustre/lod/lod_pool.c
+++ b/lustre/lod/lod_pool.c
@@ -388,7 +388,7 @@ bool lod_pool_exists(struct lod_device *lod, char *poolname)
        return pool != NULL;
 }
 
-static struct pool_desc *lod_pool_find(struct lod_device *lod, char *poolname)
+struct pool_desc *lod_pool_find(struct lod_device *lod, char *poolname)
 {
        struct pool_desc *pool;
 
@@ -775,18 +775,12 @@ out_sem:
 }
 
 /*
- * to prevent infinite loops during spilling, lets limit number of passes
- */
-#define LOD_SPILL_MAX  10
-
-/*
  * XXX: consider a better schema to detect loops
  */
 void lod_check_and_spill_pool(const struct lu_env *env, struct lod_device *lod,
                              char **poolname)
 {
        struct pool_desc *pool;
-       int replaced = 0;
 
        if (!poolname || !*poolname || (*poolname)[0] == '\0')
                return;
@@ -797,15 +791,9 @@ repeat:
 
        lod_spill_target_refresh(env, lod, pool);
        if (pool->pool_spill_is_active) {
-               if (++replaced >= LOD_SPILL_MAX)
-                       CWARN("%s: more than %d levels of pool spill for '%s->%s'\n",
-                             lod2obd(lod)->obd_name, LOD_SPILL_MAX,
-                             *poolname, pool->pool_spill_target);
                lod_set_pool(poolname, pool->pool_spill_target);
                atomic_inc(&pool->pool_spill_hit);
                lod_pool_putref(pool);
-               if (replaced >= LOD_SPILL_MAX)
-                       return;
                goto repeat;
        }
 

diff --git a/lustre/lod/lproc_lod.c b/lustre/lod/lproc_lod.c
index 018c551..de44d74 100644 (file)
--- a/lustre/lod/lproc_lod.c
+++ b/lustre/lod/lproc_lod.c
@@ -1130,8 +1130,10 @@ lod_spill_threshold_pct_seq_write(struct file *file, const char __user *buffer,
 
        pool->pool_spill_threshold_pct = val;
        pool->pool_spill_expire = 0;
-       if (pool->pool_spill_threshold_pct == 0)
+       if (pool->pool_spill_threshold_pct == 0) {
                pool->pool_spill_is_active = false;
+               pool->pool_spill_target[0] = '\0';
+       }
 
        return count;
 }
@@ -1147,6 +1149,40 @@ static int lod_spill_target_seq_show(struct seq_file *m, void *v)
        return 0;
 }
 
+static DEFINE_MUTEX(lod_spill_loop_mutex);
+
+static int lod_spill_check_loop(struct lod_device *lod,
+                               const char *pool,
+                               const char *destarg)
+{
+       char dest[LOV_MAXPOOLNAME + 1];
+       struct pool_desc *tgt;
+       int rc = 0;
+
+       strncpy(dest, destarg, sizeof(dest));
+
+       /*
+        * pool1 -> pool2 -> pool3 ++> pool1
+        * pool1 -> pool2 ++>pool3 -> pool1
+        */
+       while (1) {
+               tgt = lod_pool_find(lod, dest);
+               if (!tgt) {
+                       /* no more steps, we're fine */
+                       break;
+               }
+
+               strncpy(dest, tgt->pool_spill_target, sizeof(dest));
+               lod_pool_putref(tgt);
+
+               if (strcmp(dest, pool) == 0) {
+                       rc = -ELOOP;
+                       break;
+               }
+       }
+       return rc;
+}
+
 static ssize_t
 lod_spill_target_seq_write(struct file *file, const char __user *buffer,
                           size_t count, loff_t *off)
@@ -1154,6 +1190,9 @@ lod_spill_target_seq_write(struct file *file, const char __user *buffer,
        struct seq_file *m = file->private_data;
        struct pool_desc *pool = m->private;
        struct lod_device *lod;
+       char tgt_name[LOV_MAXPOOLNAME + 1];
+       int rc;
+
 
        LASSERT(pool != NULL);
        lod = lu2lod_dev(pool->pool_lobd->obd_lu_dev);
@@ -1164,21 +1203,33 @@ lod_spill_target_seq_write(struct file *file, const char __user *buffer,
                return count;
        }
 
-       if (count > LOV_MAXPOOLNAME - 1)
+       if (count > sizeof(tgt_name) - 1)
                return -E2BIG;
-       if (copy_from_user(pool->pool_spill_target, buffer, count))
+       if (copy_from_user(tgt_name, buffer, count))
                return -EFAULT;
+       tgt_name[count] = '\0';
 
-       pool->pool_spill_target[count] = '\0';
-       if (strcmp(pool->pool_name, pool->pool_spill_target) == 0)
+       if (strcmp(pool->pool_name, tgt_name) == 0)
                return -ELOOP;
-       if (!lod_pool_exists(lod, pool->pool_spill_target)) {
+       if (!lod_pool_exists(lod, tgt_name)) {
                pool->pool_spill_target[0] = '\0';
                pool->pool_spill_expire = 0;
                return -ENODEV;
        }
 
-       return count;
+       /* serialize all checks to protect against racing settings */
+       mutex_lock(&lod_spill_loop_mutex);
+
+       rc = lod_spill_check_loop(lod, pool->pool_name, tgt_name);
+       if (rc == 0) {
+               strncpy(pool->pool_spill_target, tgt_name,
+                       sizeof(pool->pool_spill_target));
+               rc = count;
+       }
+
+       mutex_unlock(&lod_spill_loop_mutex);
+
+       return rc;
 }
 LPROC_SEQ_FOPS(lod_spill_target);
 

diff --git a/lustre/tests/ost-pools.sh b/lustre/tests/ost-pools.sh
index 17bebb5..6def5e4 100755 (executable)
--- a/lustre/tests/ost-pools.sh
+++ b/lustre/tests/ost-pools.sh
@@ -1747,8 +1747,10 @@ test_30() {
 run_test 30 "persistent OST pool spilling"
 
 test_31() {
+       local prefix="lod.$FSNAME-*.pool."
        local MDT_DEV=$(mdsdevname mds1)
        local mdts=$(comma_list $(mdts_nodes))
+       local do_mdts="do_nodes $mdts $LCTL"
        local pool1=${TESTNAME}-1
        local pool2=${TESTNAME}-2
        local pool3=${TESTNAME}-3
@@ -1781,16 +1783,18 @@ test_31() {
                      awk '{ print $1 * 2; exit; }')
        sleep $((delay + 1))
 
-       do_nodes $mdts $LCTL set_param lod.*.pool.$pool1.spill_target="$pool2"
-       do_nodes $mdts $LCTL set_param lod.*.pool.$pool1.spill_threshold_pct="$threshold"
+       stack_trap "$do_mdts set_param lod.*.pool.*.spill_threshold_pct=0"
 
-       do_nodes $mdts $LCTL set_param lod.*.pool.$pool2.spill_target="$pool3"
-       do_nodes $mdts $LCTL set_param lod.*.pool.$pool2.spill_threshold_pct="$threshold"
+       $do_mdts set_param lod.*.pool.$pool1.spill_target="$pool2"
+       $do_mdts set_param lod.*.pool.$pool1.spill_threshold_pct="$threshold"
 
-       do_nodes $mdts $LCTL set_param lod.*.pool.$pool3.spill_target="$pool4"
-       do_nodes $mdts $LCTL set_param lod.*.pool.$pool3.spill_threshold_pct="$threshold"
+       $do_mdts set_param lod.*.pool.$pool2.spill_target="$pool3"
+       $do_mdts set_param lod.*.pool.$pool2.spill_threshold_pct="$threshold"
 
-       do_nodes $mdts $LCTL get_param lod.*.pool.*.spill*
+       $do_mdts set_param lod.*.pool.$pool3.spill_target="$pool4"
+       $do_mdts set_param lod.*.pool.$pool3.spill_threshold_pct="$threshold"
+
+       $do_mdts get_param lod.*.pool.*.spill*
 
        $LFS setstripe -p $pool1 $DIR/$tdir || error "can't set default layout"
        local tmpfile=$DIR/$tdir/$tfile-2
@@ -1799,6 +1803,36 @@ test_31() {
                $LFS getstripe $tmpfile
                error "old pool is not $pool4"
        }
+
+       # check for loops
+
+       # reset all loop spilling
+       $do_mdts set_param lod.*.pool.*.spill_threshold_pct="0"
+
+       # pool1->pool2->pool3, then pool4->pool1 fails
+       $do_mdts set_param lod.*.pool.$pool1.spill_target="$pool2" ||
+               error "can't set spill target for $pool1"
+       $do_mdts set_param lod.*.pool.$pool2.spill_target="$pool3" ||
+               error "can't set spill target for $pool1"
+       $do_mdts set_param lod.*.pool.$pool3.spill_target="$pool4" ||
+               error "can't set spill target for $pool1"
+       $do_mdts set_param lod.*.pool.$pool4.spill_target="$pool1" &&
+               error "loop succeed"
+
+       # reset all loop spilling
+       $do_mdts set_param lod.*.pool.*.spill_threshold_pct="0"
+
+       # pool1->pool2,pool4->pool1, then pool3->pool4 fails
+       $do_mdts set_param lod.*.pool.$pool1.spill_target="$pool2" ||
+               error "can't set spill target for $pool1"
+       $do_mdts set_param lod.*.pool.$pool2.spill_target="$pool3" ||
+               error "can't set spill target for $pool1"
+       $do_mdts set_param lod.*.pool.$pool4.spill_target="$pool1" ||
+               error "can't set spill target for $pool4"
+       $do_mdts set_param lod.*.pool.$pool3.spill_target="$pool4" &&
+               error "loop succeed"
+
+       return 0
 }
 run_test 31 "OST pool spilling chained"