LU-14579 flr: mirror unlink and split race

- protect lod_object::ldo_comp_entries during
  lod_obj_for_each_stripe(), since other thread could change the
  ldo_comp_entries at the same time.

- protect LOD in-memory layout during layout change
  layout_{add|set|del} and purge_mirror.

- fix lock-tx order in mdd_unlink: start the transaction and then
  take locks. (introduced in commit
  55d5235354d49aee0a330ad64beef4ed9004a27f)

- Add test case for mirror split and unlink race.

Lustre-commit: bd7a2f9938a7edf09afd133601ca4181e109a7d0
Lustre-change: https://review.whamcloud.com/43369

Fixes: 55d5235354 ("LU-14579 flr: GPF in lod_sub_declare_destroy")
Signed-off-by: Bobi Jam <bobijam@whamcloud.com>
Change-Id: Ic54245c8755f660087fce46d1cad0ef7fa091245
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Mike Pershin <mpershin@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
Reviewed-on: https://review.whamcloud.com/44257
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>

diff --git a/lustre/lod/lod_lov.c b/lustre/lod/lod_lov.c
index 4102be2..1699b95 100644 (file)
--- a/lustre/lod/lod_lov.c
+++ b/lustre/lod/lod_lov.c
@@ -1218,10 +1218,7 @@ int lod_parse_striping(const struct lu_env *env, struct lod_object *lo,
            magic != LOV_MAGIC_SEL)
                GOTO(out, rc = -EINVAL);
 
-       if (lo->ldo_is_foreign)
-               lod_free_foreign_lov(lo);
-       else
-               lod_free_comp_entries(lo);
+       lod_striping_free_nolock(env, lo);
 
        if (magic == LOV_MAGIC_COMP_V1 || magic == LOV_MAGIC_SEL) {
                comp_v1 = (struct lov_comp_md_v1 *)lmm;
@@ -1564,7 +1561,6 @@ int lod_striping_reload(const struct lu_env *env, struct lod_object *lo,
        ENTRY;
 
        mutex_lock(&lo->ldo_layout_mutex);
-       lod_striping_free_nolock(env, lo);
        rc = lod_parse_striping(env, lo, buf);
        mutex_unlock(&lo->ldo_layout_mutex);
 

diff --git a/lustre/lod/lod_object.c b/lustre/lod/lod_object.c
index 16ea21e..52681dc 100644 (file)
--- a/lustre/lod/lod_object.c
+++ b/lustre/lod/lod_object.c
@@ -1175,10 +1175,10 @@ int lod_obj_for_each_stripe(const struct lu_env *env, struct lod_object *lo,
                            struct lod_obj_stripe_cb_data *data)
 {
        struct lod_layout_component *lod_comp;
-       int i, j, rc;
+       int i, j, rc = 0;
        ENTRY;
 
-       LASSERT(lo->ldo_comp_cnt != 0 && lo->ldo_comp_entries != NULL);
+       mutex_lock(&lo->ldo_layout_mutex);
        for (i = 0; i < lo->ldo_comp_cnt; i++) {
                lod_comp = &lo->ldo_comp_entries[i];
 
@@ -1201,7 +1201,7 @@ int lod_obj_for_each_stripe(const struct lu_env *env, struct lod_object *lo,
                if (data->locd_comp_cb) {
                        rc = data->locd_comp_cb(env, lo, i, data);
                        if (rc)
-                               RETURN(rc);
+                               GOTO(unlock, rc);
                }
 
                /* could used just to do sth about component, not each
@@ -1218,10 +1218,12 @@ int lod_obj_for_each_stripe(const struct lu_env *env, struct lod_object *lo,
                                continue;
                        rc = data->locd_stripe_cb(env, lo, dt, th, i, j, data);
                        if (rc != 0)
-                               RETURN(rc);
+                               GOTO(unlock, rc);
                }
        }
-       RETURN(0);
+unlock:
+       mutex_unlock(&lo->ldo_layout_mutex);
+       RETURN(rc);
 }
 
 static bool lod_obj_attr_set_comp_skip_cb(const struct lu_env *env,
@@ -2757,10 +2759,14 @@ static int lod_declare_layout_add(const struct lu_env *env,
        if (magic != LOV_USER_MAGIC_COMP_V1)
                RETURN(-EINVAL);
 
+       mutex_lock(&lo->ldo_layout_mutex);
+
        array_cnt = lo->ldo_comp_cnt + comp_v1->lcm_entry_count;
        OBD_ALLOC_PTR_ARRAY(comp_array, array_cnt);
-       if (comp_array == NULL)
+       if (comp_array == NULL) {
+               mutex_unlock(&lo->ldo_layout_mutex);
                RETURN(-ENOMEM);
+       }
 
        memcpy(comp_array, lo->ldo_comp_entries,
               sizeof(*comp_array) * lo->ldo_comp_cnt);
@@ -2817,6 +2823,8 @@ static int lod_declare_layout_add(const struct lu_env *env,
        LASSERT(lo->ldo_mirror_count == 1);
        lo->ldo_mirrors[0].lme_end = array_cnt - 1;
 
+       mutex_unlock(&lo->ldo_layout_mutex);
+
        RETURN(0);
 
 error:
@@ -2829,6 +2837,8 @@ error:
                }
        }
        OBD_FREE_PTR_ARRAY(comp_array, array_cnt);
+       mutex_unlock(&lo->ldo_layout_mutex);
+
        RETURN(rc);
 }
 
@@ -2924,6 +2934,7 @@ static int lod_declare_layout_set(const struct lu_env *env,
                RETURN(-EINVAL);
        }
 
+       mutex_lock(&lo->ldo_layout_mutex);
        for (i = 0; i < comp_v1->lcm_entry_count; i++) {
                __u32 id = comp_v1->lcm_entries[i].lcme_id;
                __u32 flags = comp_v1->lcm_entries[i].lcme_flags;
@@ -2933,7 +2944,8 @@ static int lod_declare_layout_set(const struct lu_env *env,
 
                if (flags & LCME_FL_INIT) {
                        if (changed)
-                               lod_striping_free(env, lo);
+                               lod_striping_free_nolock(env, lo);
+                       mutex_unlock(&lo->ldo_layout_mutex);
                        RETURN(-EINVAL);
                }
 
@@ -2956,8 +2968,11 @@ static int lod_declare_layout_set(const struct lu_env *env,
                                if (flags) {
                                        if ((flags & LCME_FL_STALE) &&
                                            lod_last_non_stale_mirror(mirror_id,
-                                                                     lo))
+                                                                     lo)) {
+                                               mutex_unlock(
+                                                       &lo->ldo_layout_mutex);
                                                RETURN(-EUCLEAN);
+                                       }
                                        lod_comp->llc_flags |= flags;
                                }
                                if (mirror_flag) {
@@ -2970,6 +2985,7 @@ static int lod_declare_layout_set(const struct lu_env *env,
                        changed = true;
                }
        }
+       mutex_unlock(&lo->ldo_layout_mutex);
 
        if (!changed) {
                CDEBUG(D_LAYOUT, "%s: requested component(s) not found.\n",
@@ -3052,9 +3068,13 @@ static int lod_declare_layout_del(const struct lu_env *env,
                flags = 0;
        }
 
+       mutex_lock(&lo->ldo_layout_mutex);
+
        left = lo->ldo_comp_cnt;
-       if (left <= 0)
+       if (left <= 0) {
+               mutex_unlock(&lo->ldo_layout_mutex);
                RETURN(-EINVAL);
+       }
 
        for (i = (lo->ldo_comp_cnt - 1); i >= 0; i--) {
                struct lod_layout_component *lod_comp;
@@ -3071,6 +3091,7 @@ static int lod_declare_layout_del(const struct lu_env *env,
                if (left != (i + 1)) {
                        CDEBUG(D_LAYOUT, "%s: this deletion will create "
                               "a hole.\n", lod2obd(d)->obd_name);
+                       mutex_unlock(&lo->ldo_layout_mutex);
                        RETURN(-EINVAL);
                }
                left--;
@@ -3089,8 +3110,10 @@ static int lod_declare_layout_del(const struct lu_env *env,
                        if (obj == NULL)
                                continue;
                        rc = lod_sub_declare_destroy(env, obj, th);
-                       if (rc)
+                       if (rc) {
+                               mutex_unlock(&lo->ldo_layout_mutex);
                                RETURN(rc);
+                       }
                }
        }
 
@@ -3098,9 +3121,12 @@ static int lod_declare_layout_del(const struct lu_env *env,
        if (left == lo->ldo_comp_cnt) {
                CDEBUG(D_LAYOUT, "%s: requested component id:%#x not found\n",
                       lod2obd(d)->obd_name, id);
+               mutex_unlock(&lo->ldo_layout_mutex);
                RETURN(-EINVAL);
        }
 
+       mutex_unlock(&lo->ldo_layout_mutex);
+
        memset(attr, 0, sizeof(*attr));
        attr->la_valid = LA_SIZE;
        rc = lod_sub_declare_attr_set(env, next, attr, th);
@@ -4167,7 +4193,7 @@ static int lod_generate_and_set_lovea(const struct lu_env *env,
        LASSERT(lo);
 
        if (lo->ldo_comp_cnt == 0 && !lo->ldo_is_foreign) {
-               lod_striping_free(env, lo);
+               lod_striping_free_nolock(env, lo);
                rc = lod_sub_xattr_del(env, next, XATTR_NAME_LOV, th);
                RETURN(rc);
        }
@@ -4466,6 +4492,8 @@ static int lod_layout_del(const struct lu_env *env, struct dt_object *dt,
 
        LASSERT(lo->ldo_mirror_count == 1);
 
+       mutex_lock(&lo->ldo_layout_mutex);
+
        rc = lod_layout_del_prep_layout(env, lo, th);
        if (rc < 0)
                GOTO(out, rc);
@@ -4493,7 +4521,10 @@ static int lod_layout_del(const struct lu_env *env, struct dt_object *dt,
        EXIT;
 out:
        if (rc)
-               lod_striping_free(env, lo);
+               lod_striping_free_nolock(env, lo);
+
+       mutex_unlock(&lo->ldo_layout_mutex);
+
        return rc;
 }
 
@@ -5763,6 +5794,8 @@ int lod_striped_create(const struct lu_env *env, struct dt_object *dt,
        int     rc = 0, i, j;
        ENTRY;
 
+       mutex_lock(&lo->ldo_layout_mutex);
+
        LASSERT((lo->ldo_comp_cnt != 0 && lo->ldo_comp_entries != NULL) ||
                lo->ldo_is_foreign);
 
@@ -5826,15 +5859,20 @@ int lod_striped_create(const struct lu_env *env, struct dt_object *dt,
        if (rc)
                GOTO(out, rc);
 
+       lo->ldo_comp_cached = 1;
+
        rc = lod_generate_and_set_lovea(env, lo, th);
        if (rc)
                GOTO(out, rc);
 
-       lo->ldo_comp_cached = 1;
+       mutex_unlock(&lo->ldo_layout_mutex);
+
        RETURN(0);
 
 out:
-       lod_striping_free(env, lo);
+       lod_striping_free_nolock(env, lo);
+       mutex_unlock(&lo->ldo_layout_mutex);
+
        RETURN(rc);
 }
 
@@ -5894,11 +5932,12 @@ lod_obj_stripe_destroy_cb(const struct lu_env *env, struct lod_object *lo,
 {
        if (data->locd_declare)
                return lod_sub_declare_destroy(env, dt, th);
-       else if (!OBD_FAIL_CHECK(OBD_FAIL_LFSCK_LOST_SPEOBJ) ||
-                stripe_idx == cfs_fail_val)
+
+       if (!OBD_FAIL_CHECK(OBD_FAIL_LFSCK_LOST_SPEOBJ) ||
+           stripe_idx == cfs_fail_val)
                return lod_sub_destroy(env, dt, th);
-       else
-               return 0;
+
+       return 0;
 }
 
 /**

diff --git a/lustre/mdd/mdd_dir.c b/lustre/mdd/mdd_dir.c
index b0c12d8..ac3aa25 100644 (file)
--- a/lustre/mdd/mdd_dir.c
+++ b/lustre/mdd/mdd_dir.c
@@ -1792,7 +1792,7 @@ static int mdd_unlink(const struct lu_env *env, struct md_object *pobj,
 
        /* Enough for only unlink the entry */
        if (unlikely(mdd_cobj == NULL))
-               GOTO(stop, rc);
+               GOTO(cleanup, rc);
 
        if (cattr->la_nlink > 0 || mdd_cobj->mod_count > 0) {
                /* update ctime of an unlinked file only if it is still

diff --git a/lustre/mdd/mdd_object.c b/lustre/mdd/mdd_object.c
index 2e63862..fb34810 100644 (file)
--- a/lustre/mdd/mdd_object.c
+++ b/lustre/mdd/mdd_object.c
@@ -1776,36 +1776,29 @@ static int mdd_xattr_split(const struct lu_env *env, struct md_object *md_obj,
        struct lu_buf *buf_vic = &mdd_env_info(env)->mti_buf[2];
        struct lov_comp_md_v1 *lcm;
        struct thandle *handle;
+       int lock_order;
        int rc;
        bool dom_stripe = false;
 
        ENTRY;
 
-       rc = lu_fid_cmp(mdd_object_fid(obj), mdd_object_fid(vic));
-       if (rc == 0) /* same fid */
+       lock_order = lu_fid_cmp(mdd_object_fid(obj), mdd_object_fid(vic));
+       if (lock_order == 0) /* same fid */
                RETURN(-EPERM);
 
        handle = mdd_trans_create(env, mdd);
        if (IS_ERR(handle))
                RETURN(PTR_ERR(handle));
 
-       if (rc > 0) {
-               mdd_write_lock(env, obj, DT_TGT_CHILD);
-               mdd_write_lock(env, vic, DT_TGT_CHILD);
-       } else {
-               mdd_write_lock(env, vic, DT_TGT_CHILD);
-               mdd_write_lock(env, obj, DT_TGT_CHILD);
-       }
-
        /* get EA of mirrored file */
        memset(buf_save, 0, sizeof(*buf));
        rc = mdd_stripe_get(env, obj, buf_save, XATTR_NAME_LOV);
        if (rc < 0)
-               GOTO(out, rc);
+               GOTO(stop, rc);
 
        lcm = buf_save->lb_buf;
        if (le32_to_cpu(lcm->lcm_magic) != LOV_MAGIC_COMP_V1)
-               GOTO(out, rc = -EINVAL);
+               GOTO(stop, rc = -EINVAL);
 
        /**
         * Extract the mirror with specified mirror id, and store the splitted
@@ -1815,27 +1808,39 @@ static int mdd_xattr_split(const struct lu_env *env, struct md_object *md_obj,
        memset(buf_vic, 0, sizeof(*buf_vic));
        rc = mdd_split_ea(lcm, mrd->mrd_mirror_id, buf, buf_vic);
        if (rc < 0)
-               GOTO(out, rc);
+               GOTO(stop, rc);
 
+       /**
+        * @buf stores layout w/o the specified mirror, @buf_vic stores the
+        * splitted mirror
+        */
        dom_stripe = mdd_lmm_dom_size(buf_vic->lb_buf) > 0;
 
        rc = mdd_declare_xattr_set(env, mdd, obj, buf, XATTR_NAME_LOV,
                                   LU_XATTR_SPLIT, handle);
        if (rc)
-               GOTO(out, rc);
+               GOTO(stop, rc);
        rc = mdd_declare_xattr_set(env, mdd, vic, buf_vic, XATTR_NAME_LOV,
                                   LU_XATTR_SPLIT, handle);
        if (rc)
-               GOTO(out, rc);
+               GOTO(stop, rc);
 
        rc = mdd_trans_start(env, mdd, handle);
        if (rc)
-               GOTO(out, rc);
+               GOTO(stop, rc);
+
+       if (lock_order > 0) {
+               mdd_write_lock(env, obj, DT_TGT_CHILD);
+               mdd_write_lock(env, vic, DT_TGT_CHILD);
+       } else {
+               mdd_write_lock(env, vic, DT_TGT_CHILD);
+               mdd_write_lock(env, obj, DT_TGT_CHILD);
+       }
 
        rc = mdo_xattr_set(env, obj, buf, XATTR_NAME_LOV, LU_XATTR_REPLACE,
                           handle);
        if (rc)
-               GOTO(out, rc);
+               GOTO(unlock, rc);
 
        rc = mdo_xattr_set(env, vic, buf_vic, XATTR_NAME_LOV, LU_XATTR_CREATE,
                           handle);
@@ -1845,12 +1850,12 @@ static int mdd_xattr_split(const struct lu_env *env, struct md_object *md_obj,
        rc = mdd_changelog_data_store(env, mdd, CL_LAYOUT, 0, obj, handle,
                                      NULL);
        if (rc)
-               GOTO(out, rc);
+               GOTO(out_restore, rc);
 
        rc = mdd_changelog_data_store(env, mdd, CL_LAYOUT, 0, vic, handle,
                                      NULL);
        if (rc)
-               GOTO(out, rc);
+               GOTO(out_restore, rc);
        EXIT;
 
 out_restore:
@@ -1863,15 +1868,17 @@ out_restore:
                               mdd_obj_dev_name(obj),
                               PFID(mdd_object_fid(obj)), rc2);
        }
-out:
+
+unlock:
+       mdd_write_unlock(env, obj);
+       mdd_write_unlock(env, vic);
+stop:
        rc = mdd_trans_stop(env, mdd, rc, handle);
 
        /* Truncate local DOM data if all went well */
        if (!rc && dom_stripe)
                mdd_dom_data_truncate(env, mdd, obj);
 
-       mdd_write_unlock(env, obj);
-       mdd_write_unlock(env, vic);
        lu_buf_free(buf_save);
        lu_buf_free(buf);
        lu_buf_free(buf_vic);

diff --git a/lustre/tests/sanity-flr.sh b/lustre/tests/sanity-flr.sh
index 0c4e6b4..afb8462 100644 (file)
--- a/lustre/tests/sanity-flr.sh
+++ b/lustre/tests/sanity-flr.sh
@@ -2606,6 +2606,35 @@ test_60b() {
 }
 run_test 60b "mirror merge/split cancel client's in-memory layout gen"
 
+test_70() {
+       local tf=$DIR/$tdir/$tfile
+
+       test_mkdir $DIR/$tdir
+
+       while true; do
+               rm -f $tf
+               $LFS mirror create -N -E 1M -c -1 -E eof -N $tf
+               echo xxxx > $tf
+       done &
+       c_pid=$!
+       echo "mirror create pid $c_pid"
+
+       while true; do
+               $LFS mirror split -d --mirror-id=1 $tf &> /dev/null
+       done &
+       s_pid=$!
+       echo "mirror split pid $s_pid"
+
+       echo "mirror create and split race for 60 seconds, should not crash"
+       sleep 60
+       kill -9 $c_pid &> /dev/null
+       kill -9 $s_pid &> /dev/null
+
+       rm -f $tf
+       true
+}
+run_test 70 "mirror create and split race"
+
 ctrl_file=$(mktemp /tmp/CTRL.XXXXXX)
 lock_file=$(mktemp /var/lock/FLR.XXXXXX)