Author: mm
Date: Sat Jul 30 19:00:31 2011
New Revision: 224526
URL: http://svn.freebsd.org/changeset/base/224526

Log:
  Fix serious bug in ZIL that can lead to pool corruption
  in the case of a held dataset during remount.
  
  Detailed description is available at:
  https://www.illumos.org/issues/883
  
  illumos-gate revision:        13380:161b964a0e10
  
  Reviewed by:  pjd
  Approved by:  re (kib)
  Obtained from:        Illumos (Bug #883)
  MFC after:    3 days

Modified:
  head/cddl/contrib/opensolaris/cmd/ztest/ztest.c
  head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zil.c

Modified: head/cddl/contrib/opensolaris/cmd/ztest/ztest.c
==============================================================================
--- head/cddl/contrib/opensolaris/cmd/ztest/ztest.c     Sat Jul 30 17:44:06 
2011        (r224525)
+++ head/cddl/contrib/opensolaris/cmd/ztest/ztest.c     Sat Jul 30 19:00:31 
2011        (r224526)
@@ -205,6 +205,7 @@ typedef struct ztest_od {
  */
 typedef struct ztest_ds {
        objset_t        *zd_os;
+       rwlock_t        zd_zilog_lock;
        zilog_t         *zd_zilog;
        uint64_t        zd_seq;
        ztest_od_t      *zd_od;         /* debugging aid */
@@ -238,6 +239,7 @@ ztest_func_t ztest_dmu_commit_callbacks;
 ztest_func_t ztest_zap;
 ztest_func_t ztest_zap_parallel;
 ztest_func_t ztest_zil_commit;
+ztest_func_t ztest_zil_remount;
 ztest_func_t ztest_dmu_read_write_zcopy;
 ztest_func_t ztest_dmu_objset_create_destroy;
 ztest_func_t ztest_dmu_prealloc;
@@ -273,6 +275,7 @@ ztest_info_t ztest_info[] = {
        { ztest_zap_parallel,                   100,    &zopt_always    },
        { ztest_split_pool,                     1,      &zopt_always    },
        { ztest_zil_commit,                     1,      &zopt_incessant },
+       { ztest_zil_remount,                    1,      &zopt_sometimes },
        { ztest_dmu_read_write_zcopy,           1,      &zopt_often     },
        { ztest_dmu_objset_create_destroy,      1,      &zopt_often     },
        { ztest_dsl_prop_get_set,               1,      &zopt_often     },
@@ -986,6 +989,7 @@ ztest_zd_init(ztest_ds_t *zd, objset_t *
        zd->zd_seq = 0;
        dmu_objset_name(os, zd->zd_name);
 
+       VERIFY(rwlock_init(&zd->zd_zilog_lock, USYNC_THREAD, NULL) == 0);
        VERIFY(_mutex_init(&zd->zd_dirobj_lock, USYNC_THREAD, NULL) == 0);
 
        for (int l = 0; l < ZTEST_OBJECT_LOCKS; l++)
@@ -1965,6 +1969,8 @@ ztest_io(ztest_ds_t *zd, uint64_t object
        if (ztest_random(2) == 0)
                io_type = ZTEST_IO_WRITE_TAG;
 
+       (void) rw_rdlock(&zd->zd_zilog_lock);
+
        switch (io_type) {
 
        case ZTEST_IO_WRITE_TAG:
@@ -2000,6 +2006,8 @@ ztest_io(ztest_ds_t *zd, uint64_t object
                break;
        }
 
+       (void) rw_unlock(&zd->zd_zilog_lock);
+
        umem_free(data, blocksize);
 }
 
@@ -2054,6 +2062,8 @@ ztest_zil_commit(ztest_ds_t *zd, uint64_
 {
        zilog_t *zilog = zd->zd_zilog;
 
+       (void) rw_rdlock(&zd->zd_zilog_lock);
+
        zil_commit(zilog, ztest_random(ZTEST_OBJECTS));
 
        /*
@@ -2065,6 +2075,31 @@ ztest_zil_commit(ztest_ds_t *zd, uint64_
        ASSERT(zd->zd_seq <= zilog->zl_commit_lr_seq);
        zd->zd_seq = zilog->zl_commit_lr_seq;
        mutex_exit(&zilog->zl_lock);
+
+       (void) rw_unlock(&zd->zd_zilog_lock);
+}
+
+/*
+ * This function is designed to simulate the operations that occur during a
+ * mount/unmount operation.  We hold the dataset across these operations in an
+ * attempt to expose any implicit assumptions about ZIL management.
+ */
+/* ARGSUSED */
+void
+ztest_zil_remount(ztest_ds_t *zd, uint64_t id)
+{
+       objset_t *os = zd->zd_os;
+
+       (void) rw_wrlock(&zd->zd_zilog_lock);
+
+       /* zfsvfs_teardown() */
+       zil_close(zd->zd_zilog);
+
+       /* zfsvfs_setup() */
+       VERIFY(zil_open(os, ztest_get_data) == zd->zd_zilog);
+       zil_replay(os, zd, ztest_replay_vector);
+
+       (void) rw_unlock(&zd->zd_zilog_lock);
 }
 
 /*

Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zil.c
==============================================================================
--- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zil.c   Sat Jul 30 
17:44:06 2011        (r224525)
+++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zil.c   Sat Jul 30 
19:00:31 2011        (r224526)
@@ -20,6 +20,7 @@
  */
 /*
  * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011 by Delphix. All rights reserved.
  */
 
 /* Portions Copyright 2010 Robert Milkowski */
@@ -567,7 +568,7 @@ zil_destroy(zilog_t *zilog, boolean_t ke
 
        if (!list_is_empty(&zilog->zl_lwb_list)) {
                ASSERT(zh->zh_claim_txg == 0);
-               ASSERT(!keep_first);
+               VERIFY(!keep_first);
                while ((lwb = list_head(&zilog->zl_lwb_list)) != NULL) {
                        list_remove(&zilog->zl_lwb_list, lwb);
                        if (lwb->lwb_buf != NULL)
@@ -1668,20 +1669,9 @@ zil_alloc(objset_t *os, zil_header_t *zh
 void
 zil_free(zilog_t *zilog)
 {
-       lwb_t *head_lwb;
-
        zilog->zl_stop_sync = 1;
 
-       /*
-        * After zil_close() there should only be one lwb with a buffer.
-        */
-       head_lwb = list_head(&zilog->zl_lwb_list);
-       if (head_lwb) {
-               ASSERT(head_lwb == list_tail(&zilog->zl_lwb_list));
-               list_remove(&zilog->zl_lwb_list, head_lwb);
-               zio_buf_free(head_lwb->lwb_buf, head_lwb->lwb_sz);
-               kmem_cache_free(zil_lwb_cache, head_lwb);
-       }
+       ASSERT(list_is_empty(&zilog->zl_lwb_list));
        list_destroy(&zilog->zl_lwb_list);
 
        avl_destroy(&zilog->zl_vdev_tree);
@@ -1721,6 +1711,10 @@ zil_open(objset_t *os, zil_get_data_t *g
 {
        zilog_t *zilog = dmu_objset_zil(os);
 
+       ASSERT(zilog->zl_clean_taskq == NULL);
+       ASSERT(zilog->zl_get_data == NULL);
+       ASSERT(list_is_empty(&zilog->zl_lwb_list));
+
        zilog->zl_get_data = get_data;
        zilog->zl_clean_taskq = taskq_create("zil_clean", 1, minclsyspri,
            2, 2, TASKQ_PREPOPULATE);
@@ -1734,7 +1728,7 @@ zil_open(objset_t *os, zil_get_data_t *g
 void
 zil_close(zilog_t *zilog)
 {
-       lwb_t *tail_lwb;
+       lwb_t *lwb;
        uint64_t txg = 0;
 
        zil_commit(zilog, 0); /* commit all itx */
@@ -1746,9 +1740,9 @@ zil_close(zilog_t *zilog)
         * destroy the zl_clean_taskq.
         */
        mutex_enter(&zilog->zl_lock);
-       tail_lwb = list_tail(&zilog->zl_lwb_list);
-       if (tail_lwb != NULL)
-               txg = tail_lwb->lwb_max_txg;
+       lwb = list_tail(&zilog->zl_lwb_list);
+       if (lwb != NULL)
+               txg = lwb->lwb_max_txg;
        mutex_exit(&zilog->zl_lock);
        if (txg)
                txg_wait_synced(zilog->zl_dmu_pool, txg);
@@ -1756,6 +1750,19 @@ zil_close(zilog_t *zilog)
        taskq_destroy(zilog->zl_clean_taskq);
        zilog->zl_clean_taskq = NULL;
        zilog->zl_get_data = NULL;
+
+       /*
+        * We should have only one LWB left on the list; remove it now.
+        */
+       mutex_enter(&zilog->zl_lock);
+       lwb = list_head(&zilog->zl_lwb_list);
+       if (lwb != NULL) {
+               ASSERT(lwb == list_tail(&zilog->zl_lwb_list));
+               list_remove(&zilog->zl_lwb_list, lwb);
+               zio_buf_free(lwb->lwb_buf, lwb->lwb_sz);
+               kmem_cache_free(zil_lwb_cache, lwb);
+       }
+       mutex_exit(&zilog->zl_lock);
 }
 
 /*
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to