From: Lai Siyao Date: Wed, 17 Oct 2018 05:29:53 +0000 (+0800) Subject: LU-11418 llog: refresh remote llog upon -ESTALE X-Git-Tag: 2.12.0-RC1~51 X-Git-Url: https://git.whamcloud.com/?p=fs%2Flustre-release.git;a=commitdiff_plain;h=71f409c9b31b90fa432f1f46ad4e612fb65c7fcc 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 Change-Id: Ie2d37686e910676587baefa1687ebb607be3c3a1 Reviewed-on: https://review.whamcloud.com/33401 Reviewed-by: Andreas Dilger Tested-by: Jenkins Tested-by: Maloo Reviewed-by: Mike Pershin Reviewed-by: Oleg Drokin --- diff --git a/lustre/include/lustre_log.h b/lustre/include/lustre_log.h index ec4d1f9..aa0a0b5 100644 --- 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 --- 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 --- 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"