LU-11418 llog: refresh remote llog upon -ESTALE

If a distributed transaction is aborted, it will invalidate all
objects involved, which include remote catalog and update logs.

So llog_cat_declare_add_rec() will refresh remote llog upon -ESTALE,
but it only does this for chd_current_log, not chd_next_log.

If an aborted transaction happens to invalidate catalog only, and
leave chd_current_log valid, which will cause subsequent operations
fail to create chd_next_log.

This patch prepares both current and next log of catalog before
declare_write in llog_cat_declare_add_rec().

Add sanity.sh 60g.

Signed-off-by: Lai Siyao <lai.siyao@whamcloud.com>
Change-Id: Ie2d37686e910676587baefa1687ebb607be3c3a1
Reviewed-on: https://review.whamcloud.com/33401
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Tested-by: Jenkins
Tested-by: Maloo <hpdd-maloo@intel.com>
Reviewed-by: Mike Pershin <mpershin@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

diff --git a/lustre/include/lustre_log.h b/lustre/include/lustre_log.h
index ec4d1f9..aa0a0b5 100644 (file)
--- a/lustre/include/lustre_log.h
+++ b/lustre/include/lustre_log.h
@@ -280,7 +280,6 @@ struct llog_handle {
        atomic_t                 lgh_refcount;
 
        int                     lgh_max_size;
-       __u32                   lgh_stale:1;
 };
 
 /* llog_osd.c */

diff --git a/lustre/obdclass/llog_cat.c b/lustre/obdclass/llog_cat.c
index 59a0950..e489f23 100644 (file)
--- a/lustre/obdclass/llog_cat.c
+++ b/lustre/obdclass/llog_cat.c
@@ -220,6 +220,110 @@ out_destroy:
        RETURN(rc);
 }
 
+static int llog_cat_refresh(const struct lu_env *env,
+                           struct llog_handle *cathandle)
+{
+       struct llog_handle *loghandle;
+       int rc;
+
+       down_write(&cathandle->lgh_lock);
+       list_for_each_entry(loghandle, &cathandle->u.chd.chd_head,
+                           u.phd.phd_entry) {
+               if (!llog_exist(loghandle))
+                       continue;
+
+               rc = llog_read_header(env, loghandle, NULL);
+               if (rc)
+                       goto unlock;
+       }
+
+       rc = llog_read_header(env, cathandle, NULL);
+unlock:
+       up_write(&loghandle->lgh_lock);
+
+       return rc;
+}
+
+/*
+ * prepare current/next log for catalog.
+ *
+ * if log is NULL, open it, and declare create, NB, if log is remote, create it
+ * synchronously here, see comments below.
+ */
+static int llog_cat_prep_log(const struct lu_env *env,
+                            struct llog_handle *cathandle,
+                            struct llog_handle **ploghandle,
+                            struct thandle *th)
+{
+       int rc = 0;
+
+       if (IS_ERR_OR_NULL(*ploghandle)) {
+               down_write(&cathandle->lgh_lock);
+               if (IS_ERR_OR_NULL(*ploghandle)) {
+                       struct llog_handle *loghandle;
+
+                       rc = llog_open(env, cathandle->lgh_ctxt, &loghandle,
+                                      NULL, NULL, LLOG_OPEN_NEW);
+                       if (!rc) {
+                               *ploghandle = loghandle;
+                               list_add_tail(&loghandle->u.phd.phd_entry,
+                                             &cathandle->u.chd.chd_head);
+                       }
+               }
+               up_write(&cathandle->lgh_lock);
+
+               if (rc)
+                       return rc;
+       }
+
+       if (llog_exist(*ploghandle))
+               return 0;
+
+       if (dt_object_remote(cathandle->lgh_obj)) {
+create_remote_log:
+               down_read_nested(&cathandle->lgh_lock, LLOGH_CAT);
+               down_write_nested(&(*ploghandle)->lgh_lock, LLOGH_LOG);
+               if (!llog_exist(*ploghandle)) {
+                       /* For remote operation, if we put the llog object
+                        * creation in the current transaction, then the
+                        * llog object will not be created on the remote
+                        * target until the transaction stop, if other
+                        * operations start before the transaction stop,
+                        * and use the same llog object, will be dependent
+                        * on the success of this transaction. So let's
+                        * create the llog object synchronously here to
+                        * remove the dependency. */
+                       rc = llog_cat_new_log(env, cathandle, *ploghandle,
+                                             NULL);
+                       if (rc == -ESTALE) {
+                               up_write(&(*ploghandle)->lgh_lock);
+                               up_read(&cathandle->lgh_lock);
+
+                               rc = llog_cat_refresh(env, cathandle);
+                               if (!rc)
+                                       goto create_remote_log;
+
+                               return rc;
+                       }
+               }
+               up_write(&(*ploghandle)->lgh_lock);
+               up_read(&cathandle->lgh_lock);
+       } else {
+               struct llog_thread_info *lgi = llog_info(env);
+               struct llog_logid_rec *lirec = &lgi->lgi_logid;
+
+               rc = llog_declare_create(env, *ploghandle, th);
+               if (rc)
+                       return rc;
+
+               lirec->lid_hdr.lrh_len = sizeof(*lirec);
+               rc = llog_declare_write_rec(env, cathandle, &lirec->lid_hdr, -1,
+                                           th);
+       }
+
+       return rc;
+}
+
 /* Open an existent log handle and add it to the open list.
  * This log handle will be closed when all of the records in it are removed.
  *
@@ -427,40 +531,6 @@ out_unlock:
        RETURN(loghandle);
 }
 
-static int llog_cat_update_header(const struct lu_env *env,
-                          struct llog_handle *cathandle)
-{
-       struct llog_handle *loghandle;
-       int rc;
-       ENTRY;
-
-       /* refresh llog */
-       down_write(&cathandle->lgh_lock);
-       if (!cathandle->lgh_stale) {
-               up_write(&cathandle->lgh_lock);
-               RETURN(0);
-       }
-       list_for_each_entry(loghandle, &cathandle->u.chd.chd_head,
-                           u.phd.phd_entry) {
-               if (!llog_exist(loghandle))
-                       continue;
-
-               rc = llog_read_header(env, loghandle, NULL);
-               if (rc != 0) {
-                       up_write(&cathandle->lgh_lock);
-                       GOTO(out, rc);
-               }
-       }
-       rc = llog_read_header(env, cathandle, NULL);
-       if (rc == 0)
-               cathandle->lgh_stale = 0;
-       up_write(&cathandle->lgh_lock);
-       if (rc != 0)
-               GOTO(out, rc);
-out:
-       RETURN(rc);
-}
-
 /* Add a single record to the recovery log(s) using a catalog
  * Returns as llog_write_record
  *
@@ -530,167 +600,39 @@ int llog_cat_declare_add_rec(const struct lu_env *env,
                             struct llog_handle *cathandle,
                             struct llog_rec_hdr *rec, struct thandle *th)
 {
-       struct llog_thread_info *lgi = llog_info(env);
-       struct llog_logid_rec   *lirec = &lgi->lgi_logid;
-       struct llog_handle      *loghandle, *next;
-       int                      rc = 0;
+       int rc;
 
        ENTRY;
 
-       if (cathandle->u.chd.chd_current_log == NULL) {
-               /* declare new plain llog */
-               down_write(&cathandle->lgh_lock);
-               if (cathandle->u.chd.chd_current_log == NULL) {
-                       rc = llog_open(env, cathandle->lgh_ctxt, &loghandle,
-                                      NULL, NULL, LLOG_OPEN_NEW);
-                       if (rc == 0) {
-                               cathandle->u.chd.chd_current_log = loghandle;
-                               list_add_tail(&loghandle->u.phd.phd_entry,
-                                             &cathandle->u.chd.chd_head);
-                       }
-               }
-               up_write(&cathandle->lgh_lock);
-       } else if (cathandle->u.chd.chd_next_log == NULL ||
-                  IS_ERR(cathandle->u.chd.chd_next_log)) {
-               /* declare next plain llog */
-               down_write(&cathandle->lgh_lock);
-               if (cathandle->u.chd.chd_next_log == NULL ||
-                   IS_ERR(cathandle->u.chd.chd_next_log)) {
-                       rc = llog_open(env, cathandle->lgh_ctxt, &loghandle,
-                                      NULL, NULL, LLOG_OPEN_NEW);
-                       if (rc == 0) {
-                               cathandle->u.chd.chd_next_log = loghandle;
-                               list_add_tail(&loghandle->u.phd.phd_entry,
-                                             &cathandle->u.chd.chd_head);
-                       }
-               }
-               up_write(&cathandle->lgh_lock);
-       }
+       rc = llog_cat_prep_log(env, cathandle,
+                              &cathandle->u.chd.chd_current_log, th);
        if (rc)
-               GOTO(out, rc);
-
-       lirec->lid_hdr.lrh_len = sizeof(*lirec);
+               RETURN(rc);
 
-       if (!llog_exist(cathandle->u.chd.chd_current_log)) {
-               if (dt_object_remote(cathandle->lgh_obj)) {
-                       /* For remote operation, if we put the llog object
-                        * creation in the current transaction, then the
-                        * llog object will not be created on the remote
-                        * target until the transaction stop, if other
-                        * operations start before the transaction stop,
-                        * and use the same llog object, will be dependent
-                        * on the success of this transaction. So let's
-                        * create the llog object synchronously here to
-                        * remove the dependency. */
-create_again:
-                       down_read_nested(&cathandle->lgh_lock, LLOGH_CAT);
-                       loghandle = cathandle->u.chd.chd_current_log;
-                       down_write_nested(&loghandle->lgh_lock, LLOGH_LOG);
-                       if (cathandle->lgh_stale) {
-                               up_write(&loghandle->lgh_lock);
-                               up_read(&cathandle->lgh_lock);
-                               GOTO(out, rc = -EIO);
-                       }
-                       if (!llog_exist(loghandle)) {
-                               rc = llog_cat_new_log(env, cathandle, loghandle,
-                                                     NULL);
-                               if (rc == -ESTALE)
-                                       cathandle->lgh_stale = 1;
-                       }
-                       up_write(&loghandle->lgh_lock);
-                       up_read(&cathandle->lgh_lock);
-                       if (rc == -ESTALE) {
-                               rc = llog_cat_update_header(env, cathandle);
-                               if (rc != 0)
-                                       GOTO(out, rc);
-                               goto create_again;
-                       } else if (rc < 0) {
-                               GOTO(out, rc);
-                       }
-               } else {
-                       rc = llog_declare_create(env,
-                                       cathandle->u.chd.chd_current_log, th);
-                       if (rc)
-                               GOTO(out, rc);
-                       llog_declare_write_rec(env, cathandle,
-                                              &lirec->lid_hdr, -1, th);
-               }
-       }
+       rc = llog_cat_prep_log(env, cathandle, &cathandle->u.chd.chd_next_log,
+                              th);
+       if (rc)
+               RETURN(rc);
 
-write_again:
-       /* declare records in the llogs */
+declare_write:
        rc = llog_declare_write_rec(env, cathandle->u.chd.chd_current_log,
                                    rec, -1, th);
-       if (rc == -ESTALE) {
-               down_write(&cathandle->lgh_lock);
-               if (cathandle->lgh_stale) {
-                       up_write(&cathandle->lgh_lock);
-                       GOTO(out, rc = -EIO);
-               }
-
-               cathandle->lgh_stale = 1;
-               up_write(&cathandle->lgh_lock);
-               rc = llog_cat_update_header(env, cathandle);
-               if (rc != 0)
-                       GOTO(out, rc);
-               goto write_again;
-       } else if (rc < 0) {
-               GOTO(out, rc);
+       if (rc == -ESTALE && dt_object_remote(cathandle->lgh_obj)) {
+               rc = llog_cat_refresh(env, cathandle);
+               if (!rc)
+                       goto declare_write;
        }
 
-       next = cathandle->u.chd.chd_next_log;
-       if (!IS_ERR_OR_NULL(next)) {
-               if (!llog_exist(next)) {
-                       if (dt_object_remote(cathandle->lgh_obj)) {
-                               /* For remote operation, if we put the llog
-                                * object creation in the current transaction,
-                                * then the llog object will not be created on
-                                * the remote target until the transaction stop,
-                                * if other operations start before the
-                                * transaction stop, and use the same llog
-                                * object, will be dependent on the success of
-                                * this transaction. So let's create the llog
-                                * object synchronously here to remove the
-                                * dependency. */
-                               down_write_nested(&cathandle->lgh_lock,
-                                                LLOGH_CAT);
-                               next = cathandle->u.chd.chd_next_log;
-                               if (IS_ERR_OR_NULL(next)) {
-                                       /* Sigh, another thread just tried,
-                                        * let's fail as well */
-                                       up_write(&cathandle->lgh_lock);
-                                       if (next == NULL)
-                                               rc = -EIO;
-                                       else
-                                               rc = PTR_ERR(next);
-                                       GOTO(out, rc);
-                               }
-
-                               down_write_nested(&next->lgh_lock, LLOGH_LOG);
-                               if (!llog_exist(next)) {
-                                       rc = llog_cat_new_log(env, cathandle,
-                                                             next, NULL);
-                                       if (rc < 0)
-                                               cathandle->u.chd.chd_next_log =
-                                                               ERR_PTR(rc);
-                               }
-                               up_write(&next->lgh_lock);
-                               up_write(&cathandle->lgh_lock);
-                               if (rc < 0)
-                                       GOTO(out, rc);
-                       } else {
-                               rc = llog_declare_create(env, next, th);
-                               llog_declare_write_rec(env, cathandle,
-                                               &lirec->lid_hdr, -1, th);
-                       }
-               }
-               /* XXX: we hope for declarations made for existing llog
-                *      this might be not correct with some backends
-                *      where declarations are expected against specific
-                *      object like ZFS with full debugging enabled */
-               /*llog_declare_write_rec(env, next, rec, -1, th);*/
-       }
-out:
+#if 0
+       /*
+        * XXX: we hope for declarations made for existing llog this might be
+        * not correct with some backends where declarations are expected
+        * against specific object like ZFS with full debugging enabled.
+        */
+       rc = llog_declare_write_rec(env, cathandle->u.chd.chd_next_log, rec, -1,
+                                   th);
+#endif
+
        RETURN(rc);
 }
 EXPORT_SYMBOL(llog_cat_declare_add_rec);

diff --git a/lustre/tests/sanity.sh b/lustre/tests/sanity.sh
index 42789ad..c989bca 100755 (executable)
--- a/lustre/tests/sanity.sh
+++ b/lustre/tests/sanity.sh
@@ -6350,6 +6350,36 @@ test_60e() {
 }
 run_test 60e "no space while new llog is being created"
 
+test_60g() {
+       local pid
+
+       test_mkdir -c $MDSCOUNT $DIR/$tdir
+       $LFS setdirstripe -D -i -1 -c $MDSCOUNT $DIR/$tdir
+
+       (
+               local index=0
+               while true; do
+                       mkdir $DIR/$tdir/subdir$index 2>/dev/null
+                       rmdir $DIR/$tdir/subdir$index 2>/dev/null
+                       index=$((index + 1))
+               done
+       ) &
+
+       pid=$!
+
+       for i in $(seq 100); do 
+               # define OBD_FAIL_OSD_TXN_START    0x19a
+               do_facet mds1 lctl set_param fail_loc=0x8000019a
+               usleep 100
+       done
+
+       kill -9 $pid
+
+       mkdir $DIR/$tdir/new || error "mkdir failed"
+       rmdir $DIR/$tdir/new || error "rmdir failed"
+}
+run_test 60g "transaction abort won't cause MDT hung"
+
 test_61() {
        [ $PARALLEL == "yes" ] && skip "skip parallel run"