Whamcloud - gitweb
LU-2915 lfsck: LFSCK 1.5 technical debts (3)
authorFan Yong <yong.fan@whamcloud.com>
Sun, 19 May 2013 09:40:39 +0000 (17:40 +0800)
committerOleg Drokin <oleg.drokin@intel.com>
Thu, 13 Jun 2013 17:21:07 +0000 (13:21 -0400)
This patch resolves some LFSCK 1.5 technical debts, including:
1) Check and remove repeated linkea entries.
2) Merge some "goto" branches to make the code more readable.
3) Some comments about object's nlink inconsistency processing.

Test-Parameters: testlist=sanity-scrub,sanity-lfsck
Signed-off-by: Fan Yong <fan.yong@intel.com>
Change-Id: Ia2bf525cab6b94b87b7d88133393f9d2bb13031e
Reviewed-on: http://review.whamcloud.com/6344
Tested-by: Hudson
Tested-by: Maloo <whamcloud.maloo@gmail.com>
Reviewed-by: Alex Zhuravlev <alexey.zhuravlev@intel.com>
Reviewed-by: Andreas Dilger <andreas.dilger@intel.com>
Reviewed-by: Oleg Drokin <oleg.drokin@intel.com>
lustre/include/lustre_linkea.h
lustre/include/obd_support.h
lustre/lfsck/lfsck_namespace.c
lustre/mdd/mdd_dir.c
lustre/obdclass/linkea.c
lustre/tests/sanity-lfsck.sh

index acb8442..1e07f3a 100644 (file)
@@ -52,8 +52,27 @@ void linkea_del_buf(struct linkea_data *ldata, const struct lu_name *lname);
 int linkea_links_find(struct linkea_data *ldata, const struct lu_name *lname,
                      const struct lu_fid  *pfid);
 
-#define LINKEA_NEXT_ENTRY(ldata)       \
-       (struct link_ea_entry *)((char *)ldata.ld_lee + ldata.ld_reclen)
+static inline void linkea_first_entry(struct linkea_data *ldata)
+{
+       LASSERT(ldata != NULL);
+       LASSERT(ldata->ld_leh != NULL);
 
-#define LINKEA_FIRST_ENTRY(ldata)      \
-       (struct link_ea_entry *)(ldata.ld_leh + 1)
+       if (ldata->ld_leh->leh_reccount == 0)
+               ldata->ld_lee = NULL;
+       else
+               ldata->ld_lee = (struct link_ea_entry *)(ldata->ld_leh + 1);
+}
+
+static inline void linkea_next_entry(struct linkea_data *ldata)
+{
+       LASSERT(ldata != NULL);
+       LASSERT(ldata->ld_leh != NULL);
+
+       if (ldata->ld_lee != NULL) {
+               ldata->ld_lee = (struct link_ea_entry *)((char *)ldata->ld_lee +
+                                                        ldata->ld_reclen);
+               if ((char *)ldata->ld_lee >= ((char *)ldata->ld_leh +
+                                             ldata->ld_leh->leh_len))
+                       ldata->ld_lee = NULL;
+       }
+}
index 7ed0586..9739e0e 100644 (file)
@@ -478,6 +478,7 @@ int obd_alloc_fail(const void *ptr, const char *name, const char *type,
 #define OBD_FAIL_LFSCK_DELAY3          0x1602
 #define OBD_FAIL_LFSCK_LINKEA_CRASH    0x1603
 #define OBD_FAIL_LFSCK_LINKEA_MORE     0x1604
+#define OBD_FAIL_LFSCK_LINKEA_MORE2    0x1605
 #define OBD_FAIL_LFSCK_FATAL1          0x1608
 #define OBD_FAIL_LFSCK_FATAL2          0x1609
 #define OBD_FAIL_LFSCK_CRASH           0x160a
index 9d2950c..273daee 100644 (file)
@@ -428,6 +428,38 @@ static int lfsck_links_write(const struct lu_env *env, struct dt_object *obj,
 }
 
 /**
+ * \retval ve: removed entries
+ */
+static int lfsck_linkea_entry_unpack(struct lfsck_instance *lfsck,
+                                    struct linkea_data *ldata,
+                                    struct lu_name *cname,
+                                    struct lu_fid *pfid)
+{
+       struct link_ea_entry    *oldlee;
+       int                      oldlen;
+       int                      removed = 0;
+
+       linkea_entry_unpack(ldata->ld_lee, &ldata->ld_reclen, cname, pfid);
+       oldlee = ldata->ld_lee;
+       oldlen = ldata->ld_reclen;
+       linkea_next_entry(ldata);
+       while (ldata->ld_lee != NULL) {
+               ldata->ld_reclen = (ldata->ld_lee->lee_reclen[0] << 8) |
+                                  ldata->ld_lee->lee_reclen[1];
+               if (unlikely(ldata->ld_reclen == oldlen &&
+                            memcmp(ldata->ld_lee, oldlee, oldlen) == 0)) {
+                       linkea_del_buf(ldata, cname);
+                       removed++;
+               } else {
+                       linkea_next_entry(ldata);
+               }
+       }
+       ldata->ld_lee = oldlee;
+       ldata->ld_reclen = oldlen;
+       return removed;
+}
+
+/**
  * \retval +ve repaired
  * \retval 0   no need to repair
  * \retval -ve error cases
@@ -449,7 +481,6 @@ static int lfsck_namespace_double_scan_one(const struct lu_env *env,
        struct thandle          *handle   = NULL;
        bool                     locked   = false;
        bool                     update   = false;
-       int                      count;
        int                      rc;
        ENTRY;
 
@@ -458,6 +489,7 @@ static int lfsck_namespace_double_scan_one(const struct lu_env *env,
 again:
                LASSERT(!locked);
 
+               update = false;
                com->lc_journal = 1;
                handle = dt_trans_create(env, lfsck->li_next);
                if (IS_ERR(handle))
@@ -491,13 +523,14 @@ again:
                GOTO(stop, rc);
        }
 
-       ldata.ld_lee = LINKEA_FIRST_ENTRY(ldata);
-       count = ldata.ld_leh->leh_reccount;
-       while (count-- > 0) {
+       linkea_first_entry(&ldata);
+       while (ldata.ld_lee != NULL) {
                struct dt_object *parent = NULL;
 
-               linkea_entry_unpack(ldata.ld_lee, &ldata.ld_reclen, cname,
-                                   pfid);
+               rc = lfsck_linkea_entry_unpack(lfsck, &ldata, cname, pfid);
+               if (rc > 0)
+                       update = true;
+
                if (!fid_is_sane(pfid))
                        goto shrink;
 
@@ -514,7 +547,7 @@ again:
                 *      remote object will be processed in LFSCK phase III. */
                if (dt_object_remote(parent)) {
                        lfsck_object_put(env, parent);
-                       ldata.ld_lee = LINKEA_NEXT_ENTRY(ldata);
+                       linkea_next_entry(&ldata);
                        continue;
                }
 
@@ -536,25 +569,29 @@ again:
                if (rc == 0) {
                        if (lu_fid_eq(cfid, lfsck_dto2fid(child))) {
                                lfsck_object_put(env, parent);
-                               ldata.ld_lee = LINKEA_NEXT_ENTRY(ldata);
+                               linkea_next_entry(&ldata);
                                continue;
                        }
 
                        goto shrink;
                }
 
+               /* If there is no name entry in the parent dir and the object
+                * link count is less than the linkea entries count, then the
+                * linkea entry should be removed. */
                if (ldata.ld_leh->leh_reccount > la->la_nlink)
                        goto shrink;
 
-               /* XXX: For the case of there is linkea entry, but without name
-                *      entry pointing to the object, and the object link count
-                *      isn't less than the count of name entries, then add the
-                *      name entry back to namespace.
-                *
-                *      It is out of LFSCK 1.5 scope, will implement it in the
-                *      future. Keep the linkEA entry. */
+               /* XXX: For the case of there is a linkea entry, but without
+                *      name entry pointing to the object and its hard links
+                *      count is not less than the object name entries count,
+                *      then seems we should add the 'missed' name entry back
+                *      to namespace, but before LFSCK phase III finished, we
+                *      do not know whether the object has some inconsistency
+                *      on other MDTs. So now, do NOT add the name entry back
+                *      to the namespace, but keep the linkEA entry. LU-2914 */
                lfsck_object_put(env, parent);
-               ldata.ld_lee = LINKEA_NEXT_ENTRY(ldata);
+               linkea_next_entry(&ldata);
                continue;
 
 shrink:
@@ -582,8 +619,21 @@ shrink:
        GOTO(stop, rc);
 
 stop:
-       if (locked)
+       if (locked) {
+       /* XXX: For the case linkea entries count does not match the object hard
+        *      links count, we cannot update the later one simply. Before LFSCK
+        *      phase III finished, we cannot know whether there are some remote
+        *      name entries to be repaired or not. LU-2914 */
+               if (rc == 0 && !lfsck_is_dead_obj(child) &&
+                   ldata.ld_leh != NULL &&
+                   ldata.ld_leh->leh_reccount != la->la_nlink)
+                       CWARN("%.16s: the object "DFID" linkEA entry count %u "
+                             "may not match its hardlink count %u\n",
+                             lfsck_lfsck2name(lfsck), PFID(cfid),
+                             ldata.ld_leh->leh_reccount, la->la_nlink);
+
                dt_write_unlock(env, child);
+       }
 
        if (handle != NULL)
                dt_trans_stop(env, lfsck->li_next, handle);
@@ -592,6 +642,7 @@ stop:
                ns->ln_objs_nlink_repaired++;
                rc = 1;
        }
+
        return rc;
 }
 
@@ -794,6 +845,8 @@ static int lfsck_namespace_exec_dir(const struct lu_env *env,
        struct thandle             *handle   = NULL;
        bool                        repaired = false;
        bool                        locked   = false;
+       bool                        remove;
+       bool                        newdata;
        int                         count    = 0;
        int                         rc;
        ENTRY;
@@ -847,75 +900,61 @@ again:
        if (rc == 0) {
                count = ldata.ld_leh->leh_reccount;
                rc = linkea_links_find(&ldata, cname, pfid);
-               if (rc == 0) {
-                       /* For dir, if there are more than one linkea entries,
-                        * then remove all the other redundant linkea entries.*/
-                       if (unlikely(count > 1 &&
-                                    S_ISDIR(lfsck_object_type(obj))))
-                               goto unmatch;
-
+               if ((rc == 0) &&
+                   (count == 1 || !S_ISDIR(lfsck_object_type(obj))))
                        goto record;
-               } else {
-
-unmatch:
-                       ns->ln_flags |= LF_INCONSISTENT;
-                       if (bk->lb_param & LPF_DRYRUN) {
-                               repaired = true;
-                               goto record;
-                       }
 
-                       /*For dir, remove the unmatched linkea entry directly.*/
-                       if (S_ISDIR(lfsck_object_type(obj))) {
-                               if (!com->lc_journal)
-                                       goto again;
-
-                               rc = dt_xattr_del(env, obj, XATTR_NAME_LINK,
-                                                 handle, BYPASS_CAPA);
-                               if (rc != 0)
-                                       GOTO(stop, rc);
-
-                               goto nodata;
-                       } else {
-                               goto add;
-                       }
+               ns->ln_flags |= LF_INCONSISTENT;
+               /* For dir, if there are more than one linkea entries, or the
+                * linkea entry does not match the name entry, then remove all
+                * and add the correct one. */
+               if (S_ISDIR(lfsck_object_type(obj))) {
+                       remove = true;
+                       newdata = true;
+               } else {
+                       remove = false;
+                       newdata = false;
                }
+               goto nodata;
        } else if (unlikely(rc == -EINVAL)) {
+               count = 1;
                ns->ln_flags |= LF_INCONSISTENT;
-               if (bk->lb_param & LPF_DRYRUN) {
-                       count = 1;
-                       repaired = true;
-                       goto record;
-               }
-
-               if (!com->lc_journal)
-                       goto again;
-
                /* The magic crashed, we are not sure whether there are more
                 * corrupt data in the linkea, so remove all linkea entries. */
-               rc = dt_xattr_del(env, obj, XATTR_NAME_LINK, handle,
-                                 BYPASS_CAPA);
-               if (rc != 0)
-                       GOTO(stop, rc);
-
+               remove = true;
+               newdata = true;
                goto nodata;
        } else if (rc == -ENODATA) {
+               count = 1;
                ns->ln_flags |= LF_UPGRADE;
+               remove = false;
+               newdata = true;
+
+nodata:
                if (bk->lb_param & LPF_DRYRUN) {
-                       count = 1;
                        repaired = true;
                        goto record;
                }
 
-nodata:
-               rc = linkea_data_new(&ldata,
-                                    &lfsck_env_info(env)->lti_linkea_buf);
-               if (rc != 0)
-                       GOTO(stop, rc);
-
-add:
                if (!com->lc_journal)
                        goto again;
 
+               if (remove) {
+                       LASSERT(newdata);
+
+                       rc = dt_xattr_del(env, obj, XATTR_NAME_LINK, handle,
+                                         BYPASS_CAPA);
+                       if (rc != 0)
+                               GOTO(stop, rc);
+               }
+
+               if (newdata) {
+                       rc = linkea_data_new(&ldata,
+                                       &lfsck_env_info(env)->lti_linkea_buf);
+                       if (rc != 0)
+                               GOTO(stop, rc);
+               }
+
                rc = linkea_add_buf(&ldata, cname, pfid);
                if (rc != 0)
                        GOTO(stop, rc);
@@ -1366,15 +1405,13 @@ static int lfsck_namespace_double_scan(const struct lu_env *env,
 
                /* XXX: Currently, skip remote object, the consistency for
                 *      remote object will be processed in LFSCK phase III. */
-               if (!dt_object_exists(target) || dt_object_remote(target))
-                       goto obj_put;
-
-               rc = iops->rec(env, di, (struct dt_rec *)&flags, 0);
-               if (rc == 0)
-                       rc = lfsck_namespace_double_scan_one(env, com,
-                                                            target, flags);
+               if (dt_object_exists(target) && !dt_object_remote(target)) {
+                       rc = iops->rec(env, di, (struct dt_rec *)&flags, 0);
+                       if (rc == 0)
+                               rc = lfsck_namespace_double_scan_one(env, com,
+                                                               target, flags);
+               }
 
-obj_put:
                lfsck_object_put(env, target);
 
 checkpoint:
@@ -1397,27 +1434,27 @@ checkpoint:
                if (rc < 0 && bk->lb_param & LPF_FAILOUT)
                        GOTO(put, rc);
 
-               if (likely(cfs_time_beforeq(cfs_time_current(),
-                                           lfsck->li_time_next_checkpoint)) ||
-                   com->lc_new_checked == 0)
-                       goto speed;
-
-               down_write(&com->lc_sem);
-               ns->ln_run_time_phase2 += cfs_duration_sec(cfs_time_current() +
+               if (unlikely(cfs_time_beforeq(lfsck->li_time_next_checkpoint,
+                                             cfs_time_current())) &&
+                   com->lc_new_checked != 0) {
+                       down_write(&com->lc_sem);
+                       ns->ln_run_time_phase2 +=
+                               cfs_duration_sec(cfs_time_current() +
                                HALF_SEC - lfsck->li_time_last_checkpoint);
-               ns->ln_time_last_checkpoint = cfs_time_current_sec();
-               ns->ln_objs_checked_phase2 += com->lc_new_checked;
-               com->lc_new_checked = 0;
-               rc = lfsck_namespace_store(env, com, false);
-               up_write(&com->lc_sem);
-               if (rc != 0)
-                       GOTO(put, rc);
-
-               lfsck->li_time_last_checkpoint = cfs_time_current();
-               lfsck->li_time_next_checkpoint = lfsck->li_time_last_checkpoint +
+                       ns->ln_time_last_checkpoint = cfs_time_current_sec();
+                       ns->ln_objs_checked_phase2 += com->lc_new_checked;
+                       com->lc_new_checked = 0;
+                       rc = lfsck_namespace_store(env, com, false);
+                       up_write(&com->lc_sem);
+                       if (rc != 0)
+                               GOTO(put, rc);
+
+                       lfsck->li_time_last_checkpoint = cfs_time_current();
+                       lfsck->li_time_next_checkpoint =
+                               lfsck->li_time_last_checkpoint +
                                cfs_time_seconds(LFSCK_CHECKPOINT_INTERVAL);
+               }
 
-speed:
                lfsck_control_speed(lfsck);
                if (unlikely(!thread_is_running(thread)))
                        GOTO(put, rc = 0);
@@ -1543,7 +1580,7 @@ int lfsck_namespace_setup(const struct lu_env *env,
                cfs_list_add_tail(&com->lc_link, &lfsck->li_list_idle);
                break;
        default:
-               CERROR("%s: unknown status: %u\n",
+               CERROR("%s: unknown lfsck_namespace status: %u\n",
                       lfsck_lfsck2name(lfsck), ns->ln_status);
                /* fall through */
        case LS_SCANNING_PHASE1:
index 398f146..9f3c333 100644 (file)
@@ -876,6 +876,9 @@ static int __mdd_links_add(const struct lu_env *env,
                linkea_add_buf(ldata, lname, tfid);
        }
 
+       if (OBD_FAIL_CHECK(OBD_FAIL_LFSCK_LINKEA_MORE2))
+               linkea_add_buf(ldata, lname, pfid);
+
        return linkea_add_buf(ldata, lname, pfid);
 }
 
@@ -919,9 +922,12 @@ static int mdd_linkea_prepare(const struct lu_env *env,
 
        LASSERT(oldpfid != NULL || newpfid != NULL);
 
-       if (mdd_obj->mod_flags & DEAD_OBJ)
+       if (mdd_obj->mod_flags & DEAD_OBJ) {
+               /* Prevent linkea to be updated which is NOT necessary. */
+               ldata->ld_reclen = 0;
                /* No more links, don't bother */
                RETURN(0);
+       }
 
        if (oldpfid != NULL) {
                rc = __mdd_links_del(env, mdd_obj, ldata, oldlname, oldpfid);
@@ -973,7 +979,7 @@ int mdd_links_rename(const struct lu_env *env,
                        GOTO(out, rc);
        }
 
-       if (ldata->ld_lee != NULL)
+       if (ldata->ld_reclen != 0)
                rc = mdd_links_write(env, mdd_obj, ldata, handle);
        EXIT;
 out:
index 8c580f0..8fd9076 100644 (file)
@@ -145,6 +145,10 @@ void linkea_del_buf(struct linkea_data *ldata, const struct lu_name *lname)
                (char *)ldata->ld_lee);
        CDEBUG(D_INODE, "Old link_ea name '%.*s' is removed\n",
               lname->ln_namelen, lname->ln_name);
+
+       if ((char *)ldata->ld_lee >= ((char *)ldata->ld_leh +
+                                     ldata->ld_leh->leh_len))
+               ldata->ld_lee = NULL;
 }
 EXPORT_SYMBOL(linkea_del_buf);
 
@@ -187,6 +191,7 @@ int linkea_links_find(struct linkea_data *ldata, const struct lu_name *lname,
                CDEBUG(D_INODE, "Old link_ea name '%.*s' not found\n",
                       lname->ln_namelen, lname->ln_name);
                ldata->ld_lee = NULL;
+               ldata->ld_reclen = 0;
                return -ENOENT;
        }
        return 0;
index 0cc7474..81cea0f 100644 (file)
@@ -284,6 +284,45 @@ test_2b()
 }
 run_test 2b "LFSCK can find out and remove invalid linkEA entry"
 
+test_2c()
+{
+       lfsck_prep 1 1
+       echo "start $SINGLEMDS"
+       start $SINGLEMDS $MDT_DEVNAME $MOUNT_OPTS_SCRUB > /dev/null ||
+               error "(1) Fail to start MDS!"
+
+       mount_client $MOUNT || error "(2) Fail to start client!"
+
+       #define OBD_FAIL_LFSCK_LINKEA_MORE2     0x1605
+       do_facet $SINGLEMDS $LCTL set_param fail_loc=0x1605
+       touch $DIR/$tdir/dummy
+
+       do_facet $SINGLEMDS $LCTL set_param fail_loc=0
+       umount_client $MOUNT
+       $START_NAMESPACE || error "(3) Fail to start LFSCK for namespace!"
+
+       sleep 3
+       local STATUS=$($SHOW_NAMESPACE | awk '/^status/ { print $2 }')
+       [ "$STATUS" == "completed" ] ||
+               error "(4) Expect 'completed', but got '$STATUS'"
+
+       local repaired=$($SHOW_NAMESPACE |
+                        awk '/^updated_phase2/ { print $2 }')
+       [ $repaired -eq 1 ] ||
+               error "(5) Fail to repair crashed linkEA: $repaired"
+
+       mount_client $MOUNT || error "(6) Fail to start client!"
+
+       stat $DIR/$tdir/dummy | grep "Links: 1" > /dev/null ||
+               error "(7) Fail to stat $DIR/$tdir/dummy"
+
+       local dummyfid=$($LFS path2fid $DIR/$tdir/dummy)
+       local dummyname=$($LFS fid2path $DIR $dummyfid)
+       [ "$dummyname" == "$DIR/$tdir/dummy" ] ||
+               error "(8) Fail to repair linkEA: $dummyfid $dummyname"
+}
+run_test 2c "LFSCK can find out and remove repeated linkEA entry"
+
 test_4()
 {
        lfsck_prep 3 3