Author: avg
Date: Sat Dec 24 14:25:20 2016
New Revision: 310515
URL: https://svnweb.freebsd.org/changeset/base/310515

Log:
  MFC r309250: MFV r309249: 3821 Race in rollback, zil close, and zil flush

Modified:
  stable/11/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_pool.c
  stable/11/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/spa.c
  stable/11/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zil.c
Directory Properties:
  stable/11/   (props changed)

Modified: stable/11/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_pool.c
==============================================================================
--- stable/11/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_pool.c Sat Dec 
24 14:23:14 2016        (r310514)
+++ stable/11/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_pool.c Sat Dec 
24 14:25:20 2016        (r310515)
@@ -20,7 +20,7 @@
  */
 /*
  * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved.
- * Copyright (c) 2011, 2015 by Delphix. All rights reserved.
+ * Copyright (c) 2011, 2016 by Delphix. All rights reserved.
  * Copyright (c) 2013 Steven Hartland. All rights reserved.
  * Copyright (c) 2014 Spectra Logic Corporation, All rights reserved.
  * Copyright (c) 2014 Integros [integros.com]
@@ -681,9 +681,16 @@ dsl_pool_sync_done(dsl_pool_t *dp, uint6
 {
        zilog_t *zilog;
 
-       while (zilog = txg_list_remove(&dp->dp_dirty_zilogs, txg)) {
+       while (zilog = txg_list_head(&dp->dp_dirty_zilogs, txg)) {
                dsl_dataset_t *ds = dmu_objset_ds(zilog->zl_os);
+               /*
+                * We don't remove the zilog from the dp_dirty_zilogs
+                * list until after we've cleaned it. This ensures that
+                * callers of zilog_is_dirty() receive an accurate
+                * answer when they are racing with the spa sync thread.
+                */
                zil_clean(zilog, txg);
+               (void) txg_list_remove_this(&dp->dp_dirty_zilogs, zilog, txg);
                ASSERT(!dmu_objset_is_dirty(zilog->zl_os, txg));
                dmu_buf_rele(ds->ds_dbuf, zilog);
        }

Modified: stable/11/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/spa.c
==============================================================================
--- stable/11/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/spa.c      Sat Dec 
24 14:23:14 2016        (r310514)
+++ stable/11/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/spa.c      Sat Dec 
24 14:25:20 2016        (r310515)
@@ -7091,8 +7091,6 @@ spa_sync(spa_t *spa, uint64_t txg)
                spa->spa_config_syncing = NULL;
        }
 
-       spa->spa_ubsync = spa->spa_uberblock;
-
        dsl_pool_sync_done(dp, txg);
 
        mutex_enter(&spa->spa_alloc_lock);
@@ -7117,6 +7115,13 @@ spa_sync(spa_t *spa, uint64_t txg)
 
        spa->spa_sync_pass = 0;
 
+       /*
+        * Update the last synced uberblock here. We want to do this at
+        * the end of spa_sync() so that consumers of spa_last_synced_txg()
+        * will be guaranteed that all the processing associated with
+        * that txg has been completed.
+        */
+       spa->spa_ubsync = spa->spa_uberblock;
        spa_config_exit(spa, SCL_CONFIG, FTAG);
 
        spa_handle_ignored_writes(spa);

Modified: stable/11/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zil.c
==============================================================================
--- stable/11/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zil.c      Sat Dec 
24 14:23:14 2016        (r310514)
+++ stable/11/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zil.c      Sat Dec 
24 14:25:20 2016        (r310515)
@@ -20,8 +20,7 @@
  */
 /*
  * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved.
- * Copyright (c) 2011, 2015 by Delphix. All rights reserved.
- * Copyright (c) 2011, 2014 by Delphix. All rights reserved.
+ * Copyright (c) 2011, 2016 by Delphix. All rights reserved.
  * Copyright (c) 2014 Integros [integros.com]
  */
 
@@ -492,6 +491,27 @@ zilog_dirty(zilog_t *zilog, uint64_t txg
        }
 }
 
+/*
+ * Determine if the zil is dirty in the specified txg. Callers wanting to
+ * ensure that the dirty state does not change must hold the itxg_lock for
+ * the specified txg. Holding the lock will ensure that the zil cannot be
+ * dirtied (zil_itx_assign) or cleaned (zil_clean) while we check its current
+ * state.
+ */
+boolean_t
+zilog_is_dirty_in_txg(zilog_t *zilog, uint64_t txg)
+{
+       dsl_pool_t *dp = zilog->zl_dmu_pool;
+
+       if (txg_list_member(&dp->dp_dirty_zilogs, zilog, txg & TXG_MASK))
+               return (B_TRUE);
+       return (B_FALSE);
+}
+
+/*
+ * Determine if the zil is dirty. The zil is considered dirty if it has
+ * any pending itx records that have not been cleaned by zil_clean().
+ */
 boolean_t
 zilog_is_dirty(zilog_t *zilog)
 {
@@ -1055,8 +1075,6 @@ zil_lwb_commit(zilog_t *zilog, itx_t *it
                return (NULL);
 
        ASSERT(lwb->lwb_buf != NULL);
-       ASSERT(zilog_is_dirty(zilog) ||
-           spa_freeze_txg(zilog->zl_spa) != UINT64_MAX);
 
        if (lrc->lrc_txtype == TX_WRITE && itx->itx_wr_state == WR_NEED_COPY)
                dlen = P2ROUNDUP_TYPED(
@@ -1389,6 +1407,11 @@ zil_get_commit_list(zilog_t *zilog)
        else
                otxg = spa_last_synced_txg(zilog->zl_spa) + 1;
 
+       /*
+        * This is inherently racy, since there is nothing to prevent
+        * the last synced txg from changing. That's okay since we'll
+        * only commit things in the future.
+        */
        for (txg = otxg; txg < (otxg + TXG_CONCURRENT_STATES); txg++) {
                itxg_t *itxg = &zilog->zl_itxg[txg & TXG_MASK];
 
@@ -1398,6 +1421,16 @@ zil_get_commit_list(zilog_t *zilog)
                        continue;
                }
 
+               /*
+                * If we're adding itx records to the zl_itx_commit_list,
+                * then the zil better be dirty in this "txg". We can assert
+                * that here since we're holding the itxg_lock which will
+                * prevent spa_sync from cleaning it. Once we add the itxs
+                * to the zl_itx_commit_list we must commit it to disk even
+                * if it's unnecessary (i.e. the txg was synced).
+                */
+               ASSERT(zilog_is_dirty_in_txg(zilog, txg) ||
+                   spa_freeze_txg(zilog->zl_spa) != UINT64_MAX);
                list_move_tail(commit_list, &itxg->itxg_itxs->i_sync_list);
                push_sod += itxg->itxg_sod;
                itxg->itxg_sod = 0;
@@ -1423,6 +1456,10 @@ zil_async_to_sync(zilog_t *zilog, uint64
        else
                otxg = spa_last_synced_txg(zilog->zl_spa) + 1;
 
+       /*
+        * This is inherently racy, since there is nothing to prevent
+        * the last synced txg from changing.
+        */
        for (txg = otxg; txg < (otxg + TXG_CONCURRENT_STATES); txg++) {
                itxg_t *itxg = &zilog->zl_itxg[txg & TXG_MASK];
 
@@ -1494,8 +1531,14 @@ zil_commit_writer(zilog_t *zilog)
        DTRACE_PROBE1(zil__cw1, zilog_t *, zilog);
        while (itx = list_head(&zilog->zl_itx_commit_list)) {
                txg = itx->itx_lr.lrc_txg;
-               ASSERT(txg);
+               ASSERT3U(txg, !=, 0);
 
+               /*
+                * This is inherently racy and may result in us writing
+                * out a log block for a txg that was just synced. This is
+                * ok since we'll end cleaning up that log block the next
+                * time we call zil_sync().
+                */
                if (txg > spa_last_synced_txg(spa) || txg > spa_freeze_txg(spa))
                        lwb = zil_lwb_commit(zilog, itx, lwb);
                list_remove(&zilog->zl_itx_commit_list, itx);
@@ -1812,7 +1855,10 @@ zil_close(zilog_t *zilog)
        mutex_exit(&zilog->zl_lock);
        if (txg)
                txg_wait_synced(zilog->zl_dmu_pool, txg);
-       ASSERT(!zilog_is_dirty(zilog));
+
+       if (zilog_is_dirty(zilog))
+               zfs_dbgmsg("zil (%p) is dirty, txg %llu", zilog, txg);
+       VERIFY(!zilog_is_dirty(zilog));
 
        taskq_destroy(zilog->zl_clean_taskq);
        zilog->zl_clean_taskq = NULL;
_______________________________________________
[email protected] mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "[email protected]"

Reply via email to