From 1e05119d5bac6ba44bdf80dd3251339923aae225 Mon Sep 17 00:00:00 2001 From: seekfirstleapsecond <38333432+seekfirstleapsecond@users.noreply.github.com> Date: Fri, 4 Jan 2019 18:17:42 -0800 Subject: [PATCH 1/6] making sure the last quiesced txg is synced Fixed a potential bug as described in #8233: Consider this scenario (see [txg.c](https://github.com/zfsonlinux/zfs/blob/06f3fc2a4b097545259935d54634c5c6f49ed20f/module/zfs/txg.c) ): There is heavy write load when the pool exports. After `txg_sync_stop`'s call of `txg_wait_synced` returns, many more txgs get processed, but right before` txg_sync_stop` gets `tx_sync_lock`, the following happens: - `txg_sync_thread` begins waiting on `tx_sync_more_cv`. - `txg_quiesce_thread` gets done with `txg_quiesce(dp, txg)`. - `txg_sync_stop` gets `tx_sync_lock` first, calls `cv_broadcast`s with `tx_exiting` == 1, and waits for exits. - `txg_sync_thread` wakes up first and exits. - Finally, `txg_quiesce_thread` gets `tx_sync_lock`, and calls `cv_broadcast(&tx->tx_sync_more_cv)`, but `txg_sync_thread` is already gone, and the txg in `txg_quiesce(dp, txg)` above never gets synced. Signed-off-by: Leap Second --- module/zfs/txg.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/module/zfs/txg.c b/module/zfs/txg.c index db0f60cd15a3..7cf6a5176cee 100644 --- a/module/zfs/txg.c +++ b/module/zfs/txg.c @@ -508,6 +508,7 @@ txg_sync_thread(void *arg) tx_state_t *tx = &dp->dp_tx; callb_cpr_t cpr; clock_t start, delta; + boolean_t checked_quiescing = B_FALSE; (void) spl_fstrans_mark(); txg_thread_enter(tx, &cpr); @@ -549,8 +550,18 @@ txg_sync_thread(void *arg) txg_thread_wait(tx, &cpr, &tx->tx_quiesce_done_cv, 0); } - if (tx->tx_exiting) - txg_thread_exit(tx, &cpr, &tx->tx_sync_thread); + if (tx->tx_exiting) { + if (checked_quiescing) + txg_thread_exit(tx, &cpr, &tx->tx_sync_thread); + else { + while (tx->tx_threads != 1) + txg_thread_wait(tx, &cpr, &tx->tx_exit_cv, 0); + if (tx->tx_quiesced_txg) + checked_quiescing = B_TRUE; + else + txg_thread_exit(tx, &cpr, &tx->tx_sync_thread); + } + } /* * Consume the quiesced txg which has been handed off to From 80f4270c61487df1d5df95755d0063f64e22fb4c Mon Sep 17 00:00:00 2001 From: seekfirstleapsecond <38333432+seekfirstleapsecond@users.noreply.github.com> Date: Sat, 5 Jan 2019 04:10:40 -0800 Subject: [PATCH 2/6] style fixes Fixed checkstyle complaints: ./module/zfs/txg.c: 558: line > 80 characters ./module/zfs/txg.c: 562: line > 80 characters Signed-off-by: Leap Second --- module/zfs/txg.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/module/zfs/txg.c b/module/zfs/txg.c index 7cf6a5176cee..353877d944e7 100644 --- a/module/zfs/txg.c +++ b/module/zfs/txg.c @@ -551,15 +551,17 @@ txg_sync_thread(void *arg) } if (tx->tx_exiting) { - if (checked_quiescing) + if (checked_quiescing) { txg_thread_exit(tx, &cpr, &tx->tx_sync_thread); - else { + } else { while (tx->tx_threads != 1) - txg_thread_wait(tx, &cpr, &tx->tx_exit_cv, 0); + txg_thread_wait(tx, &cpr, + &tx->tx_exit_cv, 0); if (tx->tx_quiesced_txg) checked_quiescing = B_TRUE; else - txg_thread_exit(tx, &cpr, &tx->tx_sync_thread); + txg_thread_exit(tx, &cpr, + &tx->tx_sync_thread); } } From e3070bb5fdc0ae4b46f9c1c798b8a42b5190c467 Mon Sep 17 00:00:00 2001 From: seekfirstleapsecond <38333432+seekfirstleapsecond@users.noreply.github.com> Date: Sat, 5 Jan 2019 04:33:58 -0800 Subject: [PATCH 3/6] style fixes 2 Addressed checkstype complaints: ./module/zfs/txg.c: 559: continuation should be indented 4 spaces ./module/zfs/txg.c: 564: continuation should be indented 4 spaces Signed-off-by: Leap Second --- module/zfs/txg.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/module/zfs/txg.c b/module/zfs/txg.c index 353877d944e7..13c9b3af7118 100644 --- a/module/zfs/txg.c +++ b/module/zfs/txg.c @@ -556,12 +556,12 @@ txg_sync_thread(void *arg) } else { while (tx->tx_threads != 1) txg_thread_wait(tx, &cpr, - &tx->tx_exit_cv, 0); + &tx->tx_exit_cv, 0); if (tx->tx_quiesced_txg) checked_quiescing = B_TRUE; else txg_thread_exit(tx, &cpr, - &tx->tx_sync_thread); + &tx->tx_sync_thread); } } From 4c113673e910b4e292d779ad3bde266cb2623723 Mon Sep 17 00:00:00 2001 From: seekfirstleapsecond <38333432+seekfirstleapsecond@users.noreply.github.com> Date: Sat, 5 Jan 2019 04:54:18 -0800 Subject: [PATCH 4/6] style fixes 3 Addressed checkstyle complaints: ./module/zfs/txg.c: 559: spaces instead of tabs ./module/zfs/txg.c: 559: continuation should be indented 4 spaces ./module/zfs/txg.c: 564: spaces instead of tabs ./module/zfs/txg.c: 564: continuation should be indented 4 spaces Signed-off-by: Leap Second --- module/zfs/txg.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/module/zfs/txg.c b/module/zfs/txg.c index 13c9b3af7118..fc0cd85e0dbe 100644 --- a/module/zfs/txg.c +++ b/module/zfs/txg.c @@ -556,12 +556,12 @@ txg_sync_thread(void *arg) } else { while (tx->tx_threads != 1) txg_thread_wait(tx, &cpr, - &tx->tx_exit_cv, 0); + &tx->tx_exit_cv, 0); if (tx->tx_quiesced_txg) checked_quiescing = B_TRUE; else txg_thread_exit(tx, &cpr, - &tx->tx_sync_thread); + &tx->tx_sync_thread); } } From c82de60a783dc150308f45b9d876566cce57cc2b Mon Sep 17 00:00:00 2001 From: seekfirstleapsecond <38333432+seekfirstleapsecond@users.noreply.github.com> Date: Mon, 7 Jan 2019 17:37:52 -0800 Subject: [PATCH 5/6] simplified logic Signed-off-by: Leap Second --- module/zfs/txg.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/module/zfs/txg.c b/module/zfs/txg.c index fc0cd85e0dbe..f7b196444240 100644 --- a/module/zfs/txg.c +++ b/module/zfs/txg.c @@ -550,19 +550,15 @@ txg_sync_thread(void *arg) txg_thread_wait(tx, &cpr, &tx->tx_quiesce_done_cv, 0); } + /* + * Wait until the quiesce thread has exited to ensure every + * quiesced txg has been synced before exiting. + */ if (tx->tx_exiting) { - if (checked_quiescing) { + while (tx->tx_threads != 1) + txg_thread_wait(tx, &cpr, &tx->tx_exit_cv, 0); + if (tx->tx_quiesced_txg == 0) txg_thread_exit(tx, &cpr, &tx->tx_sync_thread); - } else { - while (tx->tx_threads != 1) - txg_thread_wait(tx, &cpr, - &tx->tx_exit_cv, 0); - if (tx->tx_quiesced_txg) - checked_quiescing = B_TRUE; - else - txg_thread_exit(tx, &cpr, - &tx->tx_sync_thread); - } } /* From 025861a9812c0f9b0dab2a6bb8c5d8ee5c0d631d Mon Sep 17 00:00:00 2001 From: seekfirstleapsecond <38333432+seekfirstleapsecond@users.noreply.github.com> Date: Mon, 7 Jan 2019 18:01:29 -0800 Subject: [PATCH 6/6] removed garbage Signed-off-by: Leap Second --- module/zfs/txg.c | 1 - 1 file changed, 1 deletion(-) diff --git a/module/zfs/txg.c b/module/zfs/txg.c index f7b196444240..a7094e67dbc9 100644 --- a/module/zfs/txg.c +++ b/module/zfs/txg.c @@ -508,7 +508,6 @@ txg_sync_thread(void *arg) tx_state_t *tx = &dp->dp_tx; callb_cpr_t cpr; clock_t start, delta; - boolean_t checked_quiescing = B_FALSE; (void) spl_fstrans_mark(); txg_thread_enter(tx, &cpr);