From d04a41cbf772645420c874638046c5a6ed1c359c Mon Sep 17 00:00:00 2001 From: Arun KV Date: Tue, 22 Sep 2026 10:29:07 +0530 Subject: [PATCH 01/15] Revert "Fix data race between zil_commit() and zil_suspend()" This reverts commit a8055cced9aac7db39d0c7fbe92f4f112e6f157b. --- include/sys/zil_impl.h | 1 - module/zfs/zil.c | 27 --------------------------- 2 files changed, 28 deletions(-) diff --git a/include/sys/zil_impl.h b/include/sys/zil_impl.h index cb01be69d7e6..d2f4018653a6 100644 --- a/include/sys/zil_impl.h +++ b/include/sys/zil_impl.h @@ -183,7 +183,6 @@ struct zilog { uint64_t zl_destroy_txg; /* txg of last zil_destroy() */ uint64_t zl_replayed_seq[TXG_SIZE]; /* last replayed rec seq */ uint64_t zl_replaying_seq; /* current replay seq number */ - krwlock_t zl_suspend_lock; /* protects suspend count */ uint32_t zl_suspend; /* log suspend count */ kcondvar_t zl_cv_suspend; /* log suspend completion */ uint8_t zl_suspending; /* log is currently suspending */ diff --git a/module/zfs/zil.c b/module/zfs/zil.c index 873129b8a3b3..788408771e04 100644 --- a/module/zfs/zil.c +++ b/module/zfs/zil.c @@ -2942,21 +2942,6 @@ zil_commit(zilog_t *zilog, uint64_t foid) return; } - /* - * The ->zl_suspend_lock rwlock ensures that all in-flight - * zil_commit() operations finish before suspension begins and that - * no more begin. Without it, it is possible for the scheduler to - * preempt us right after the zilog->zl_suspend suspend check, run - * another thread that runs zil_suspend() and after the other thread - * has finished its call to zil_commit_impl(), resume this thread while - * zil is suspended. This can trigger an assertion failure in - * VERIFY(list_is_empty(&lwb->lwb_itxs)). If it is held, it means that - * `zil_suspend()` is executing in another thread, so we go to - * txg_wait_synced(). - */ - if (!rw_tryenter(&zilog->zl_suspend_lock, RW_READER)) - goto wait; - /* * If the ZIL is suspended, we don't want to dirty it by calling * zil_commit_itx_assign() below, nor can we write out @@ -2965,14 +2950,11 @@ zil_commit(zilog_t *zilog, uint64_t foid) * semantics, and avoid calling those functions altogether. */ if (zilog->zl_suspend > 0) { - rw_exit(&zilog->zl_suspend_lock); -wait: txg_wait_synced(zilog->zl_dmu_pool, 0); return; } zil_commit_impl(zilog, foid); - rw_exit(&zilog->zl_suspend_lock); } void @@ -3216,8 +3198,6 @@ zil_alloc(objset_t *os, zil_header_t *zh_phys) cv_init(&zilog->zl_cv_suspend, NULL, CV_DEFAULT, NULL); - rw_init(&zilog->zl_suspend_lock, NULL, RW_DEFAULT, NULL); - return (zilog); } @@ -3255,8 +3235,6 @@ zil_free(zilog_t *zilog) cv_destroy(&zilog->zl_cv_suspend); - rw_destroy(&zilog->zl_suspend_lock); - kmem_free(zilog, sizeof (zilog_t)); } @@ -3377,14 +3355,11 @@ zil_suspend(const char *osname, void **cookiep) } zilog = dmu_objset_zil(os); - rw_enter(&zilog->zl_suspend_lock, RW_WRITER); - mutex_enter(&zilog->zl_lock); zh = zilog->zl_header; if (zh->zh_flags & ZIL_REPLAY_NEEDED) { /* unplayed log */ mutex_exit(&zilog->zl_lock); - rw_exit(&zilog->zl_suspend_lock); dmu_objset_rele(os, suspend_tag); return (SET_ERROR(EBUSY)); } @@ -3398,7 +3373,6 @@ zil_suspend(const char *osname, void **cookiep) if (cookiep == NULL && !zilog->zl_suspending && (zilog->zl_suspend > 0 || BP_IS_HOLE(&zh->zh_log))) { mutex_exit(&zilog->zl_lock); - rw_exit(&zilog->zl_suspend_lock); dmu_objset_rele(os, suspend_tag); TraceEvent(8, "%s:%d: Returning 0\n", __func__, __LINE__); return (0); @@ -3408,7 +3382,6 @@ zil_suspend(const char *osname, void **cookiep) dsl_pool_rele(dmu_objset_pool(os), suspend_tag); zilog->zl_suspend++; - rw_exit(&zilog->zl_suspend_lock); if (zilog->zl_suspend > 1) { /* From 84d70abe395448881e68caeb33ed4758b6ed112c Mon Sep 17 00:00:00 2001 From: Kevin Jin <33590050+jxdking@users.noreply.github.com> Date: Thu, 26 May 2022 12:36:14 -0400 Subject: [PATCH 02/15] Fix inflated quiesce time caused by lwb_tx during zil_commit() In current zil_commit() process, transaction lwb_tx is assigned in zil_lwb_write_issue(), and is committed in zil_lwb_flush_vdevs_done(). Thus, during lwb write out process, the txg is held in open or quiesing state, until zil_lwb_flush_vdevs_done() is called. If the zil's zio latency is high, it will cause txg_sync_thread() to starve. The goal here is to defer waiting for zil_lwb_flush_vdevs_done to the 'syncing' txg state. That is, in zil_sync(). In this patch, it achieves the goal without holding transaction. A new function zil_lwb_flush_wait_all() is introduced. It waits for the completion of all the zil_lwb_flush_vdevs_done() by given txg. Reviewed-by: Alexander Motin Reviewed-by: Brian Behlendorf Reviewed-by: Prakash Surya Signed-off-by: jxdking Closes #12321 (cherry picked from commit 152d6fda54e61042a70059c95c44b364aea0bbd8) --- include/sys/zil_impl.h | 8 +++- module/zfs/zil.c | 89 ++++++++++++++++++++++++++++++++---------- 2 files changed, 76 insertions(+), 21 deletions(-) diff --git a/include/sys/zil_impl.h b/include/sys/zil_impl.h index d2f4018653a6..8409ce864e90 100644 --- a/include/sys/zil_impl.h +++ b/include/sys/zil_impl.h @@ -99,7 +99,7 @@ typedef struct lwb { char *lwb_buf; /* log write buffer */ zio_t *lwb_write_zio; /* zio for the lwb buffer */ zio_t *lwb_root_zio; /* root zio for lwb write and flushes */ - dmu_tx_t *lwb_tx; /* tx for log block allocation */ + uint64_t lwb_issued_txg; /* the txg when the write is issued */ uint64_t lwb_max_txg; /* highest txg in this lwb */ list_node_t lwb_node; /* zilog->zl_lwb_list linkage */ list_t lwb_itxs; /* list of itx's */ @@ -209,6 +209,12 @@ struct zilog { uint_t zl_prev_rotor; /* rotor for zl_prev[] */ txg_node_t zl_dirty_link; /* protected by dp_dirty_zilogs list */ uint64_t zl_dirty_max_txg; /* highest txg used to dirty zilog */ + + kmutex_t zl_lwb_io_lock; /* protect following members */ + uint64_t zl_lwb_inflight[TXG_SIZE]; /* io issued, but not done */ + kcondvar_t zl_lwb_io_cv; /* signal when the flush is done */ + uint64_t zl_lwb_max_issued_txg; /* max txg when lwb io issued */ + /* * Max block size for this ZIL. Note that this can not be changed * while the ZIL is in use because consumers (ZPL/zvol) need to take diff --git a/module/zfs/zil.c b/module/zfs/zil.c index 788408771e04..22a9bcdac50e 100644 --- a/module/zfs/zil.c +++ b/module/zfs/zil.c @@ -561,8 +561,8 @@ zil_alloc_lwb(zilog_t *zilog, blkptr_t *bp, boolean_t slog, uint64_t txg, lwb->lwb_max_txg = txg; lwb->lwb_write_zio = NULL; lwb->lwb_root_zio = NULL; - lwb->lwb_tx = NULL; lwb->lwb_issued_timestamp = 0; + lwb->lwb_issued_txg = 0; if (BP_GET_CHECKSUM(bp) == ZIO_CHECKSUM_ZILOG2) { lwb->lwb_nused = sizeof (zil_chain_t); lwb->lwb_sz = BP_GET_LSIZE(bp); @@ -1127,9 +1127,9 @@ zil_lwb_flush_vdevs_done(zio_t *zio) { lwb_t *lwb = zio->io_private; zilog_t *zilog = lwb->lwb_zilog; - dmu_tx_t *tx = lwb->lwb_tx; zil_commit_waiter_t *zcw; itx_t *itx; + uint64_t txg; spa_config_exit(zilog->zl_spa, SCL_STATE, lwb); @@ -1138,15 +1138,13 @@ zil_lwb_flush_vdevs_done(zio_t *zio) mutex_enter(&zilog->zl_lock); /* - * Ensure the lwb buffer pointer is cleared before releasing the - * txg. If we have had an allocation failure and the txg is + * If we have had an allocation failure and the txg is * waiting to sync then we want zil_sync() to remove the lwb so * that it's not picked up as the next new one in * zil_process_commit_list(). zil_sync() will only remove the * lwb if lwb_buf is null. */ lwb->lwb_buf = NULL; - lwb->lwb_tx = NULL; ASSERT3U(lwb->lwb_issued_timestamp, >, 0); zilog->zl_last_lwb_latency = gethrtime() - lwb->lwb_issued_timestamp; @@ -1191,12 +1189,47 @@ zil_lwb_flush_vdevs_done(zio_t *zio) mutex_exit(&zilog->zl_lock); - /* - * Now that we've written this log block, we have a stable pointer - * to the next block in the chain, so it's OK to let the txg in - * which we allocated the next block sync. - */ - dmu_tx_commit(tx); + mutex_enter(&zilog->zl_lwb_io_lock); + txg = lwb->lwb_issued_txg; + ASSERT3U(zilog->zl_lwb_inflight[txg & TXG_MASK], >, 0); + zilog->zl_lwb_inflight[txg & TXG_MASK]--; + if (zilog->zl_lwb_inflight[txg & TXG_MASK] == 0) + cv_broadcast(&zilog->zl_lwb_io_cv); + mutex_exit(&zilog->zl_lwb_io_lock); +} + +/* + * Wait for the completion of all issued write/flush of that txg provided. + * It guarantees zil_lwb_flush_vdevs_done() is called and returned. + */ +static void +zil_lwb_flush_wait_all(zilog_t *zilog, uint64_t txg) +{ + ASSERT3U(txg, ==, spa_syncing_txg(zilog->zl_spa)); + + mutex_enter(&zilog->zl_lwb_io_lock); + while (zilog->zl_lwb_inflight[txg & TXG_MASK] > 0) + cv_wait(&zilog->zl_lwb_io_cv, &zilog->zl_lwb_io_lock); + mutex_exit(&zilog->zl_lwb_io_lock); + +#ifdef ZFS_DEBUG + mutex_enter(&zilog->zl_lock); + mutex_enter(&zilog->zl_lwb_io_lock); + lwb_t *lwb = list_head(&zilog->zl_lwb_list); + while (lwb != NULL && lwb->lwb_max_txg <= txg) { + if (lwb->lwb_issued_txg <= txg) { + ASSERT(lwb->lwb_state != LWB_STATE_ISSUED); + ASSERT(lwb->lwb_state != LWB_STATE_WRITE_DONE); + IMPLY(lwb->lwb_issued_txg > 0, + lwb->lwb_state == LWB_STATE_FLUSH_DONE); + } + IMPLY(lwb->lwb_state == LWB_STATE_FLUSH_DONE, + lwb->lwb_buf == NULL); + lwb = list_next(&zilog->zl_lwb_list, lwb); + } + mutex_exit(&zilog->zl_lwb_io_lock); + mutex_exit(&zilog->zl_lock); +#endif } /* @@ -1478,11 +1511,6 @@ zil_lwb_write_issue(zilog_t *zilog, lwb_t *lwb) /* * Allocate the next block and save its address in this block * before writing it in order to establish the log chain. - * Note that if the allocation of nlwb synced before we wrote - * the block that points at it (lwb), we'd leak it if we crashed. - * Therefore, we don't do dmu_tx_commit() until zil_lwb_write_done(). - * We dirty the dataset to ensure that zil_sync() will be called - * to clean up in the event of allocation failure or I/O failure. */ tx = dmu_tx_create(zilog->zl_os); @@ -1498,7 +1526,11 @@ zil_lwb_write_issue(zilog_t *zilog, lwb_t *lwb) dsl_dataset_dirty(dmu_objset_ds(zilog->zl_os), tx); txg = dmu_tx_get_txg(tx); - lwb->lwb_tx = tx; + mutex_enter(&zilog->zl_lwb_io_lock); + lwb->lwb_issued_txg = txg; + zilog->zl_lwb_inflight[txg & TXG_MASK]++; + zilog->zl_lwb_max_issued_txg = MAX(txg, zilog->zl_lwb_max_issued_txg); + mutex_exit(&zilog->zl_lwb_io_lock); /* * Log blocks are pre-allocated. Here we select the size of the next @@ -1573,6 +1605,8 @@ zil_lwb_write_issue(zilog_t *zilog, lwb_t *lwb) zio_nowait(lwb->lwb_root_zio); zio_nowait(lwb->lwb_write_zio); + dmu_tx_commit(tx); + /* * If there was an allocation failure then nlwb will be null which * forces a txg_wait_synced(). @@ -3030,6 +3064,8 @@ zil_sync(zilog_t *zilog, dmu_tx_t *tx) if (spa_sync_pass(spa) != 1) return; + zil_lwb_flush_wait_all(zilog, txg); + mutex_enter(&zilog->zl_lock); ASSERT(zilog->zl_stop_sync == 0); @@ -3184,6 +3220,7 @@ zil_alloc(objset_t *os, zil_header_t *zh_phys) mutex_init(&zilog->zl_lock, NULL, MUTEX_DEFAULT, NULL); mutex_init(&zilog->zl_issuer_lock, NULL, MUTEX_DEFAULT, NULL); + mutex_init(&zilog->zl_lwb_io_lock, NULL, MUTEX_DEFAULT, NULL); for (int i = 0; i < TXG_SIZE; i++) { mutex_init(&zilog->zl_itxg[i].itxg_lock, NULL, @@ -3197,6 +3234,7 @@ zil_alloc(objset_t *os, zil_header_t *zh_phys) offsetof(itx_t, itx_node)); cv_init(&zilog->zl_cv_suspend, NULL, CV_DEFAULT, NULL); + cv_init(&zilog->zl_lwb_io_cv, NULL, CV_DEFAULT, NULL); return (zilog); } @@ -3232,8 +3270,10 @@ zil_free(zilog_t *zilog) mutex_destroy(&zilog->zl_issuer_lock); mutex_destroy(&zilog->zl_lock); + mutex_destroy(&zilog->zl_lwb_io_lock); cv_destroy(&zilog->zl_cv_suspend); + cv_destroy(&zilog->zl_lwb_io_cv); kmem_free(zilog, sizeof (zilog_t)); } @@ -3281,9 +3321,18 @@ zil_close(zilog_t *zilog) mutex_exit(&zilog->zl_lock); /* - * We need to use txg_wait_synced() to wait long enough for the - * ZIL to be clean, and to wait for all pending lwbs to be - * written out. + * zl_lwb_max_issued_txg may be larger than lwb_max_txg. It depends + * on the time when the dmu_tx transaction is assigned in + * zil_lwb_write_issue(). + */ + mutex_enter(&zilog->zl_lwb_io_lock); + txg = MAX(zilog->zl_lwb_max_issued_txg, txg); + mutex_exit(&zilog->zl_lwb_io_lock); + + /* + * We need to use txg_wait_synced() to wait until that txg is synced. + * zil_sync() will guarantee all lwbs up to that txg have been + * written out, flushed, and cleaned. */ if (txg != 0) txg_wait_synced(zilog->zl_dmu_pool, txg); From 73ea6fabba261a934e10e66a1f7529a78ea0372c Mon Sep 17 00:00:00 2001 From: Mark Johnston Date: Fri, 16 Jul 2021 09:34:54 -0400 Subject: [PATCH 03/15] Zero pad bytes when allocating a ZIL record When allocating a record, we round up the allocation size to a multiple of 8. In this case, any padding bytes should be zeroed, otherwise the contents of uninitialized memory are written to the ZIL. This was found using KMSAN. Reviewed-by: Brian Behlendorf Reviewed-by: Alexander Motin Signed-off-by: Mark Johnston Closes #12383 (cherry picked from commit 58714c2817891e27f08b964c7212bf331cee71ac) --- module/zfs/zil.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/module/zfs/zil.c b/module/zfs/zil.c index 22a9bcdac50e..9689f3badd35 100644 --- a/module/zfs/zil.c +++ b/module/zfs/zil.c @@ -1820,18 +1820,19 @@ zil_lwb_commit(zilog_t *zilog, itx_t *itx, lwb_t *lwb) } itx_t * -zil_itx_create(uint64_t txtype, size_t lrsize) +zil_itx_create(uint64_t txtype, size_t olrsize) { - size_t itxsize; + size_t itxsize, lrsize; itx_t *itx; - lrsize = P2ROUNDUP_TYPED(lrsize, sizeof (uint64_t), size_t); + lrsize = P2ROUNDUP_TYPED(olrsize, sizeof (uint64_t), size_t); itxsize = offsetof(itx_t, itx_lr) + lrsize; itx = zio_data_buf_alloc(itxsize); itx->itx_lr.lrc_txtype = txtype; itx->itx_lr.lrc_reclen = lrsize; itx->itx_lr.lrc_seq = 0; /* defensive */ + bzero((char *)&itx->itx_lr + olrsize, lrsize - olrsize); itx->itx_sync = B_TRUE; /* default is synchronous */ itx->itx_callback = NULL; itx->itx_callback_data = NULL; From aa3a3b16315664380dd51975549ae31bb44c6025 Mon Sep 17 00:00:00 2001 From: Mark Johnston Date: Fri, 16 Jul 2021 10:05:28 -0400 Subject: [PATCH 04/15] Zero pad bytes following TX_WRITE log data When logging a TX_WRITE record in the case where file data has to be copied from the DMU, we pad the log record size to a multiple of 8 bytes. In this case, any padding bytes should be zeroed, otherwise the contents of uninitialized memory are written to the ZIL. This was found using KMSAN. Reviewed-by: Brian Behlendorf Reviewed-by: Alexander Motin Signed-off-by: Mark Johnston Closes #12383 (cherry picked from commit 3a185275a0172ea8d2ed8b55c7b3524c52147395) --- module/zfs/zil.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/module/zfs/zil.c b/module/zfs/zil.c index 9689f3badd35..1e2310082a98 100644 --- a/module/zfs/zil.c +++ b/module/zfs/zil.c @@ -1654,7 +1654,7 @@ zil_lwb_commit(zilog_t *zilog, itx_t *itx, lwb_t *lwb) lr_t *lrcb, *lrc; lr_write_t *lrwb, *lrw; char *lr_buf; - uint64_t dlen, dnow, lwb_sp, reclen, txg, max_log_data; + uint64_t dlen, dnow, dpad, lwb_sp, reclen, txg, max_log_data; ASSERT(MUTEX_HELD(&zilog->zl_issuer_lock)); ASSERT3P(lwb, !=, NULL); @@ -1688,8 +1688,9 @@ zil_lwb_commit(zilog_t *zilog, itx_t *itx, lwb_t *lwb) if (lrc->lrc_txtype == TX_WRITE && itx->itx_wr_state == WR_NEED_COPY) { dlen = P2ROUNDUP_TYPED( lrw->lr_length, sizeof (uint64_t), uint64_t); + dpad = dlen - lrw->lr_length; } else { - dlen = 0; + dlen = dpad = 0; } reclen = lrc->lrc_reclen; zilog->zl_cur_used += (reclen + dlen); @@ -1783,6 +1784,9 @@ zil_lwb_commit(zilog_t *zilog, itx_t *itx, lwb_t *lwb) error = zilog->zl_get_data(itx->itx_private, itx->itx_gen, lrwb, dbuf, lwb, lwb->lwb_write_zio); + if (dbuf != NULL && error == 0 && dnow == dlen) + /* Zero any padding bytes in the last block. */ + bzero((char *)dbuf + lrwb->lr_length, dpad); if (error == EIO) { txg_wait_synced(zilog->zl_dmu_pool, txg); From add036555cb6273fae874163db3f9e2071fb2eb8 Mon Sep 17 00:00:00 2001 From: Arun KV <65647132+arun-kv@users.noreply.github.com> Date: Tue, 14 Sep 2021 01:32:39 +0530 Subject: [PATCH 05/15] Fixed data integrity issue when underlying disk returns error Errors in zil_lwb_write_done() are not propagated to zil_lwb_flush_vdevs_done() which can result in zil_commit_impl() not returning an error to applications even when zfs was not able to write data to the disk. Remove the ZIO_FLAG_DONT_PROPAGATE flag from zio_rewrite() to allow errors to propagate and consolidate the error handling for flush and write errors to a single location (rather than having error handling split between the "write done" and "flush done" handlers). Reviewed-by: George Wilson Reviewed-by: Prakash Surya Signed-off-by: Arun KV Closes #12391 Closes #12443 (cherry picked from commit f82f0279ed588505c16a9026786ba966dbc5f0ee) --- module/zfs/zil.c | 34 +++++++++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/module/zfs/zil.c b/module/zfs/zil.c index 1e2310082a98..485ff29c6812 100644 --- a/module/zfs/zil.c +++ b/module/zfs/zil.c @@ -1177,6 +1177,20 @@ zil_lwb_flush_vdevs_done(zio_t *zio) ASSERT3P(zcw->zcw_lwb, ==, lwb); zcw->zcw_lwb = NULL; + /* + * We expect any ZIO errors from child ZIOs to have been + * propagated "up" to this specific LWB's root ZIO, in + * order for this error handling to work correctly. This + * includes ZIO errors from either this LWB's write or + * flush, as well as any errors from other dependent LWBs + * (e.g. a root LWB ZIO that might be a child of this LWB). + * + * With that said, it's important to note that LWB flush + * errors are not propagated up to the LWB root ZIO. + * This is incorrect behavior, and results in VDEV flush + * errors not being handled correctly here. See the + * comment above the call to "zio_flush" for details. + */ zcw->zcw_zio_error = zio->io_error; @@ -1285,6 +1299,12 @@ zil_lwb_write_done(zio_t *zio) * nodes. We avoid calling zio_flush() since there isn't any * good reason for doing so, after the lwb block failed to be * written out. + * + * Additionally, we don't perform any further error handling at + * this point (e.g. setting "zcw_zio_error" appropriately), as + * we expect that to occur in "zil_lwb_flush_vdevs_done" (thus, + * we expect any error seen here, to have been propagated to + * that function). */ if (zio->io_error != 0) { while ((zv = avl_destroy_nodes(t, &cookie)) != NULL) @@ -1315,8 +1335,17 @@ zil_lwb_write_done(zio_t *zio) while ((zv = avl_destroy_nodes(t, &cookie)) != NULL) { vdev_t *vd = vdev_lookup_top(spa, zv->zv_vdev); - if (vd != NULL) + if (vd != NULL) { + /* + * The "ZIO_FLAG_DONT_PROPAGATE" is currently + * always used within "zio_flush". This means, + * any errors when flushing the vdev(s), will + * (unfortunately) not be handled correctly, + * since these "zio_flush" errors will not be + * propagated up to "zil_lwb_flush_vdevs_done". + */ zio_flush(lwb->lwb_root_zio, vd); + } kmem_free(zv, sizeof (*zv)); } } @@ -1433,8 +1462,7 @@ zil_lwb_write_open(zilog_t *zilog, lwb_t *lwb) lwb->lwb_write_zio = zio_rewrite(lwb->lwb_root_zio, zilog->zl_spa, 0, &lwb->lwb_blk, lwb_abd, BP_GET_LSIZE(&lwb->lwb_blk), zil_lwb_write_done, lwb, - prio, ZIO_FLAG_CANFAIL | ZIO_FLAG_DONT_PROPAGATE | - ZIO_FLAG_FASTWRITE, &zb); + prio, ZIO_FLAG_CANFAIL | ZIO_FLAG_FASTWRITE, &zb); ASSERT3P(lwb->lwb_write_zio, !=, NULL); lwb->lwb_state = LWB_STATE_OPENED; From e33b56681461aad48642be62c1f28b48bf2f998c Mon Sep 17 00:00:00 2001 From: Alexander Motin Date: Tue, 24 Jan 2023 12:20:32 -0500 Subject: [PATCH 06/15] Introduce minimal ZIL block commit delay Despite all optimizations, tests on actual hardware show that FreeBSD kernel can't sleep for less then ~2us. Similar tests on Linux show ~50us delay at least from nanosleep() (haven't tested inside kernel). It means that on very fast log device ZIL may not be able to satisfy zfs_commit_timeout_pct block commit timeout, increasing log latency more than desired. Handle that by introduction of zil_min_commit_timeout parameter, specifying minimal timeout value where additional delays to aggregate writes may be skipped. Also skip delays if the LWB is more than 7/8 full, that often happens if I/O sizes are constant and match one of LWB sizes. Both things are applied only if there were no already outstanding log blocks, that may indicate single-threaded workload, that by definition can not benefit from the commit delays. While there, add short time moving average to zl_last_lwb_latency to make it more stable. Tests of single-threaded 4KB writes to NVDIMM SLOG on FreeBSD show IOPS increase by 9% instead of expected 5%. For zfs_commit_timeout_pct of 1 there IOPS increase by 5.5% instead of expected 1%. Reviewed-by: Allan Jude Reviewed-by: Brian Behlendorf Reviewed-by: Prakash Surya Signed-off-by: Alexander Motin Sponsored by: iXsystems, Inc. Closes #14418 (cherry picked from commit 0f740a4f1dd444fb961e9070a3eb5d3bc68a03b3) --- man/man4/zfs.4 | 7 +++++++ module/zfs/zil.c | 36 ++++++++++++++++++++++++++++++++++-- 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/man/man4/zfs.4 b/man/man4/zfs.4 index 660af4dc80e0..164612c95ecd 100644 --- a/man/man4/zfs.4 +++ b/man/man4/zfs.4 @@ -2058,6 +2058,13 @@ On very fragmented pools, lowering this .Pq typically to Sy 36kB can improve performance. . +.It Sy zil_min_commit_timeout Ns = Ns Sy 5000 Pq u64 +This sets the minimum delay in nanoseconds ZIL care to delay block commit, +waiting for more records. +If ZIL writes are too fast, kernel may not be able sleep for so short interval, +increasing log latency above allowed by +.Sy zfs_commit_timeout_pct . +. .It Sy zil_nocacheflush Ns = Ns Sy 0 Ns | Ns 1 Pq int Disable the cache flush commands that are normally sent to disk by the ZIL after an LWB write has completed. diff --git a/module/zfs/zil.c b/module/zfs/zil.c index 485ff29c6812..a3fbab92ebe1 100644 --- a/module/zfs/zil.c +++ b/module/zfs/zil.c @@ -91,6 +91,14 @@ */ int zfs_commit_timeout_pct = 5; +/* + * Minimal time we care to delay commit waiting for more ZIL records. + * At least FreeBSD kernel can't sleep for less than 2us at its best. + * So requests to sleep for less then 5us is a waste of CPU time with + * a risk of significant log latency increase due to oversleep. + */ +static uint64_t zil_min_commit_timeout = 5000; + /* * See zil.h for more information about these fields. */ @@ -1147,7 +1155,8 @@ zil_lwb_flush_vdevs_done(zio_t *zio) lwb->lwb_buf = NULL; ASSERT3U(lwb->lwb_issued_timestamp, >, 0); - zilog->zl_last_lwb_latency = gethrtime() - lwb->lwb_issued_timestamp; + zilog->zl_last_lwb_latency = (zilog->zl_last_lwb_latency * 3 + + gethrtime() - lwb->lwb_issued_timestamp) / 4; lwb->lwb_root_zio = NULL; @@ -2311,8 +2320,9 @@ zil_process_commit_list(zilog_t *zilog) spa_t *spa = zilog->zl_spa; list_t nolwb_itxs; list_t nolwb_waiters; - lwb_t *lwb; + lwb_t *lwb, *plwb; itx_t *itx; + boolean_t first = B_TRUE; ASSERT(MUTEX_HELD(&zilog->zl_issuer_lock)); @@ -2334,6 +2344,9 @@ zil_process_commit_list(zilog_t *zilog) ASSERT3S(lwb->lwb_state, !=, LWB_STATE_ISSUED); ASSERT3S(lwb->lwb_state, !=, LWB_STATE_WRITE_DONE); ASSERT3S(lwb->lwb_state, !=, LWB_STATE_FLUSH_DONE); + first = (lwb->lwb_state != LWB_STATE_OPENED) && + ((plwb = list_prev(&zilog->zl_lwb_list, lwb)) == NULL || + plwb->lwb_state == LWB_STATE_FLUSH_DONE); } while ((itx = list_head(&zilog->zl_itx_commit_list)) != NULL) { @@ -2504,7 +2517,23 @@ zil_process_commit_list(zilog_t *zilog) * try and pack as many itxs into as few lwbs as * possible, without significantly impacting the latency * of each individual itx. + * + * If we had no already running or open LWBs, it can be + * the workload is single-threaded. And if the ZIL write + * latency is very small or if the LWB is almost full, it + * may be cheaper to bypass the delay. */ + if (lwb->lwb_state == LWB_STATE_OPENED && first) { + hrtime_t sleep = zilog->zl_last_lwb_latency * + zfs_commit_timeout_pct / 100; + if (sleep < zil_min_commit_timeout || + lwb->lwb_sz - lwb->lwb_nused < lwb->lwb_sz / 8) { + lwb = zil_lwb_write_issue(zilog, lwb); + zilog->zl_cur_used = 0; + if (lwb == NULL) + zil_commit_writer_stall(zilog); + } + } } } @@ -3778,6 +3807,9 @@ EXPORT_SYMBOL(zil_set_logbias); ZFS_MODULE_PARAM(zfs, zfs_, commit_timeout_pct, INT, ZMOD_RW, "ZIL block open timeout percentage"); +ZFS_MODULE_PARAM(zfs_zil, zil_, min_commit_timeout, U64, ZMOD_RW, + "Minimum delay we care for ZIL block commit"); + ZFS_MODULE_PARAM(zfs_zil, zil_, replay_disable, INT, ZMOD_RW, "Disable intent logging replay"); From 79db392bd2deb5e4caa907d97a2cb6bc198a15a7 Mon Sep 17 00:00:00 2001 From: Richard Yao Date: Wed, 1 Mar 2023 12:39:41 -0500 Subject: [PATCH 07/15] Handle unexpected errors in zil_lwb_commit() without ASSERT() We tripped `ASSERT(error == ENOENT || error == EEXIST || error == EALREADY)` in `zil_lwb_commit()` at Klara when doing robustness testing of ZIL against drive power cycles. That assertion presumably exists because when this code was written, the only errors expected from here were EIO, ENOENT, EEXIST and EALREADY, with EIO having its own handling before the assertion. However, upon doing a manual depth first search traversal of the source tree, it turns out that a large number of unexpected errors are possible here. In theory, EINVAL and ENOSPC can come from dnode_hold_impl(). However, most unexpected errors originate in the block layer and come to us from zio_wait() in various ways. One way is ->zl_get_data() -> dmu_buf_hold() -> dbuf_read() -> zio_wait(). From vdev_disk.c on Linux alone, zio_wait() can return the unexpected errors ENXIO, ENOTSUP, EOPNOTSUPP, ETIMEDOUT, ENOSPC, ENOLINK, EREMOTEIO, EBADE, ENODATA, EILSEQ and ENOMEM This was only observed after what have been likely over 1000 test iterations, so we do not expect to reproduce this again to find out what the error code was. However, circumstantial evidence suggests that the error was ENXIO. When ENXIO or any other unexpected error occurs, the `fsync()` or equivalent operation that called zil_commit() will return success, when in fact, dirty data has not been committed to stable storage. This is a violation of the Single UNIX Specification. The code should be able to handle this and any other unknown error by calling `txg_wait_synced()`. In addition to changing the code to call txg_wait_synced() on unexpected errors instead of returning, we modify it to print information about unexpected errors to dmesg. Reviewed-by: Brian Behlendorf Signed-off-by: Richard Yao Sponsored-By: Wasabi Technology, Inc. Closes #14532 (cherry picked from commit 3a7c35119e5456b5d2eb54db18c331e201ef776c) --- module/zfs/zil.c | 38 ++++++++++++++++++++++++++++++++------ 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/module/zfs/zil.c b/module/zfs/zil.c index a3fbab92ebe1..78bc0cbb5907 100644 --- a/module/zfs/zil.c +++ b/module/zfs/zil.c @@ -1825,13 +1825,39 @@ zil_lwb_commit(zilog_t *zilog, itx_t *itx, lwb_t *lwb) /* Zero any padding bytes in the last block. */ bzero((char *)dbuf + lrwb->lr_length, dpad); - if (error == EIO) { + /* + * Typically, the only return values we should see from + * ->zl_get_data() are 0, EIO, ENOENT, EEXIST or + * EALREADY. However, it is also possible to see other + * error values such as ENOSPC or EINVAL from + * dmu_read() -> dnode_hold() -> dnode_hold_impl() or + * ENXIO as well as a multitude of others from the + * block layer through dmu_buf_hold() -> dbuf_read() + * -> zio_wait(), as well as through dmu_read() -> + * dnode_hold() -> dnode_hold_impl() -> dbuf_read() -> + * zio_wait(). When these errors happen, we can assume + * that neither an immediate write nor an indirect + * write occurred, so we need to fall back to + * txg_wait_synced(). This is unusual, so we print to + * dmesg whenever one of these errors occurs. + */ + switch (error) { + case 0: + break; + default: + cmn_err(CE_WARN, "zil_lwb_commit() received " + "unexpected error %d from ->zl_get_data()" + ". Falling back to txg_wait_synced().", + error); + zfs_fallthrough; + case EIO: txg_wait_synced(zilog->zl_dmu_pool, txg); - return (lwb); - } - if (error != 0) { - ASSERT(error == ENOENT || error == EEXIST || - error == EALREADY); + zfs_fallthrough; + case ENOENT: + zfs_fallthrough; + case EEXIST: + zfs_fallthrough; + case EALREADY: return (lwb); } } From 1c84eeb0ee74a142d007f401538f218c93db34b2 Mon Sep 17 00:00:00 2001 From: Alexander Motin Date: Fri, 12 May 2023 12:14:29 -0400 Subject: [PATCH 08/15] zil: Some micro-optimizations. Should not cause functional changes. Reviewed-by: Brian Behlendorf Signed-off-by: Alexander Motin Sponsored by: iXsystems, Inc. Closes #14854 (cherry picked from commit 895e03135e4251be0872d96ce38f387bdc13faa2) --- module/zfs/zil.c | 75 +++++++++++++++--------------------------------- 1 file changed, 23 insertions(+), 52 deletions(-) diff --git a/module/zfs/zil.c b/module/zfs/zil.c index 78bc0cbb5907..58f7e7767ee0 100644 --- a/module/zfs/zil.c +++ b/module/zfs/zil.c @@ -143,9 +143,6 @@ unsigned long zil_slog_bulk = 768 * 1024; static kmem_cache_t *zil_lwb_cache; static kmem_cache_t *zil_zcw_cache; -#define LWB_EMPTY(lwb) ((BP_GET_LSIZE(&lwb->lwb_blk) - \ - sizeof (zil_chain_t)) == (lwb->lwb_sz - lwb->lwb_nused)) - static int zil_bp_compare(const void *x1, const void *x2) { @@ -583,11 +580,6 @@ zil_alloc_lwb(zilog_t *zilog, blkptr_t *bp, boolean_t slog, uint64_t txg, list_insert_tail(&zilog->zl_lwb_list, lwb); mutex_exit(&zilog->zl_lock); - ASSERT(!MUTEX_HELD(&lwb->lwb_vdev_lock)); - ASSERT(avl_is_empty(&lwb->lwb_vdev_tree)); - VERIFY(list_is_empty(&lwb->lwb_waiters)); - VERIFY(list_is_empty(&lwb->lwb_itxs)); - return (lwb); } @@ -596,8 +588,8 @@ zil_free_lwb(zilog_t *zilog, lwb_t *lwb) { ASSERT(MUTEX_HELD(&zilog->zl_lock)); ASSERT(!MUTEX_HELD(&lwb->lwb_vdev_lock)); - VERIFY(list_is_empty(&lwb->lwb_waiters)); - VERIFY(list_is_empty(&lwb->lwb_itxs)); + ASSERT(list_is_empty(&lwb->lwb_waiters)); + ASSERT(list_is_empty(&lwb->lwb_itxs)); ASSERT(avl_is_empty(&lwb->lwb_vdev_tree)); ASSERT3P(lwb->lwb_write_zio, ==, NULL); ASSERT3P(lwb->lwb_root_zio, ==, NULL); @@ -783,12 +775,10 @@ zil_destroy(zilog_t *zilog, boolean_t keep_first) if (!list_is_empty(&zilog->zl_lwb_list)) { ASSERT(zh->zh_claim_txg == 0); VERIFY(!keep_first); - while ((lwb = list_head(&zilog->zl_lwb_list)) != NULL) { + while ((lwb = list_remove_head(&zilog->zl_lwb_list)) != NULL) { if (lwb->lwb_fastwrite) metaslab_fastwrite_unmark(zilog->zl_spa, &lwb->lwb_blk); - - list_remove(&zilog->zl_lwb_list, lwb); if (lwb->lwb_buf != NULL) zio_buf_free(lwb->lwb_buf, lwb->lwb_sz); zio_free(zilog->zl_spa, txg, &lwb->lwb_blk); @@ -1142,6 +1132,7 @@ zil_lwb_flush_vdevs_done(zio_t *zio) spa_config_exit(zilog->zl_spa, SCL_STATE, lwb); zio_buf_free(lwb->lwb_buf, lwb->lwb_sz); + hrtime_t t = gethrtime() - lwb->lwb_issued_timestamp; mutex_enter(&zilog->zl_lock); @@ -1154,9 +1145,7 @@ zil_lwb_flush_vdevs_done(zio_t *zio) */ lwb->lwb_buf = NULL; - ASSERT3U(lwb->lwb_issued_timestamp, >, 0); - zilog->zl_last_lwb_latency = (zilog->zl_last_lwb_latency * 3 + - gethrtime() - lwb->lwb_issued_timestamp) / 4; + zilog->zl_last_lwb_latency = (zilog->zl_last_lwb_latency * 7 + t) / 8; lwb->lwb_root_zio = NULL; @@ -1173,17 +1162,12 @@ zil_lwb_flush_vdevs_done(zio_t *zio) zilog->zl_commit_lr_seq = zilog->zl_lr_seq; } - while ((itx = list_head(&lwb->lwb_itxs)) != NULL) { - list_remove(&lwb->lwb_itxs, itx); + while ((itx = list_remove_head(&lwb->lwb_itxs)) != NULL) zil_itx_destroy(itx); - } - while ((zcw = list_head(&lwb->lwb_waiters)) != NULL) { + while ((zcw = list_remove_head(&lwb->lwb_waiters)) != NULL) { mutex_enter(&zcw->zcw_lock); - ASSERT(list_link_active(&zcw->zcw_node)); - list_remove(&lwb->lwb_waiters, zcw); - ASSERT3P(zcw->zcw_lwb, ==, lwb); zcw->zcw_lwb = NULL; /* @@ -1336,7 +1320,7 @@ zil_lwb_write_done(zio_t *zio) * write and/or fsync activity, as it has the potential to * coalesce multiple flush commands to a vdev into one. */ - if (list_head(&lwb->lwb_waiters) == NULL && nlwb != NULL) { + if (list_is_empty(&lwb->lwb_waiters) && nlwb != NULL) { zil_lwb_flush_defer(lwb, nlwb); ASSERT(avl_is_empty(&lwb->lwb_vdev_tree)); return; @@ -1344,7 +1328,7 @@ zil_lwb_write_done(zio_t *zio) while ((zv = avl_destroy_nodes(t, &cookie)) != NULL) { vdev_t *vd = vdev_lookup_top(spa, zv->zv_vdev); - if (vd != NULL) { + if (vd != NULL && !vd->vdev_nowritecache) { /* * The "ZIO_FLAG_DONT_PROPAGATE" is currently * always used within "zio_flush". This means, @@ -1733,8 +1717,6 @@ zil_lwb_commit(zilog_t *zilog, itx_t *itx, lwb_t *lwb) zilog->zl_cur_used += (reclen + dlen); txg = lrc->lrc_txg; - ASSERT3U(zilog->zl_cur_used, <, UINT64_MAX - (reclen + dlen)); - cont: /* * If this record won't fit in the current log block, start a new one. @@ -1750,7 +1732,6 @@ zil_lwb_commit(zilog_t *zilog, itx_t *itx, lwb_t *lwb) if (lwb == NULL) return (NULL); zil_lwb_write_open(zilog, lwb); - ASSERT(LWB_EMPTY(lwb)); lwb_sp = lwb->lwb_sz - lwb->lwb_nused; /* @@ -1935,7 +1916,7 @@ zil_itxg_clean(void *arg) itx_async_node_t *ian; list = &itxs->i_sync_list; - while ((itx = list_head(list)) != NULL) { + while ((itx = list_remove_head(list)) != NULL) { /* * In the general case, commit itxs will not be found * here, as they'll be committed to an lwb via @@ -1958,7 +1939,6 @@ zil_itxg_clean(void *arg) if (itx->itx_lr.lrc_txtype == TX_COMMIT) zil_commit_waiter_skip(itx->itx_private); - list_remove(list, itx); zil_itx_destroy(itx); } @@ -1966,8 +1946,7 @@ zil_itxg_clean(void *arg) t = &itxs->i_async_tree; while ((ian = avl_destroy_nodes(t, &cookie)) != NULL) { list = &ian->ia_list; - while ((itx = list_head(list)) != NULL) { - list_remove(list, itx); + while ((itx = list_remove_head(list)) != NULL) { /* commit itxs should never be on the async lists. */ ASSERT3U(itx->itx_lr.lrc_txtype, !=, TX_COMMIT); zil_itx_destroy(itx); @@ -2028,8 +2007,7 @@ zil_remove_async(zilog_t *zilog, uint64_t oid) list_move_tail(&clean_list, &ian->ia_list); mutex_exit(&itxg->itxg_lock); } - while ((itx = list_head(&clean_list)) != NULL) { - list_remove(&clean_list, itx); + while ((itx = list_remove_head(&clean_list)) != NULL) { /* commit itxs should never be on the async lists. */ ASSERT3U(itx->itx_lr.lrc_txtype, !=, TX_COMMIT); zil_itx_destroy(itx); @@ -2331,7 +2309,7 @@ zil_commit_writer_stall(zilog_t *zilog) */ ASSERT(MUTEX_HELD(&zilog->zl_issuer_lock)); txg_wait_synced(zilog->zl_dmu_pool, 0); - ASSERT3P(list_tail(&zilog->zl_lwb_list), ==, NULL); + ASSERT(list_is_empty(&zilog->zl_lwb_list)); } /* @@ -2356,7 +2334,7 @@ zil_process_commit_list(zilog_t *zilog) * Return if there's nothing to commit before we dirty the fs by * calling zil_create(). */ - if (list_head(&zilog->zl_itx_commit_list) == NULL) + if (list_is_empty(&zilog->zl_itx_commit_list)) return; list_create(&nolwb_itxs, sizeof (itx_t), offsetof(itx_t, itx_node)); @@ -2375,7 +2353,7 @@ zil_process_commit_list(zilog_t *zilog) plwb->lwb_state == LWB_STATE_FLUSH_DONE); } - while ((itx = list_head(&zilog->zl_itx_commit_list)) != NULL) { + while ((itx = list_remove_head(&zilog->zl_itx_commit_list)) != NULL) { lr_t *lrc = &itx->itx_lr; uint64_t txg = lrc->lrc_txg; @@ -2389,8 +2367,6 @@ zil_process_commit_list(zilog_t *zilog) zilog_t *, zilog, itx_t *, itx); } - list_remove(&zilog->zl_itx_commit_list, itx); - boolean_t synced = txg <= spa_last_synced_txg(spa); boolean_t frozen = txg > spa_freeze_txg(spa); @@ -2476,20 +2452,16 @@ zil_process_commit_list(zilog_t *zilog) * normal. */ zil_commit_waiter_t *zcw; - while ((zcw = list_head(&nolwb_waiters)) != NULL) { + while ((zcw = list_remove_head(&nolwb_waiters)) != NULL) zil_commit_waiter_skip(zcw); - list_remove(&nolwb_waiters, zcw); - } /* * And finally, we have to destroy the itx's that * couldn't be committed to an lwb; this will also call * the itx's callback if one exists for the itx. */ - while ((itx = list_head(&nolwb_itxs)) != NULL) { - list_remove(&nolwb_itxs, itx); + while ((itx = list_remove_head(&nolwb_itxs)) != NULL) zil_itx_destroy(itx); - } } else { ASSERT(list_is_empty(&nolwb_waiters)); ASSERT3P(lwb, !=, NULL); @@ -2697,7 +2669,7 @@ zil_commit_waiter_timeout(zilog_t *zilog, zil_commit_waiter_t *zcw) */ lwb_t *nlwb = zil_lwb_write_issue(zilog, lwb); - IMPLY(nlwb != NULL, lwb->lwb_state != LWB_STATE_OPENED); + ASSERT3S(lwb->lwb_state, !=, LWB_STATE_OPENED); /* * Since the lwb's zio hadn't been issued by the time this thread @@ -3167,7 +3139,7 @@ zil_sync(zilog_t *zilog, dmu_tx_t *tx) if (zilog->zl_destroy_txg == txg) { blkptr_t blk = zh->zh_log; - ASSERT(list_head(&zilog->zl_lwb_list) == NULL); + ASSERT(list_is_empty(&zilog->zl_lwb_list)); bzero(zh, sizeof (zil_header_t)); bzero(zilog->zl_replayed_seq, sizeof (zilog->zl_replayed_seq)); @@ -3200,7 +3172,7 @@ zil_sync(zilog_t *zilog, dmu_tx_t *tx) * out the zil_header blkptr so that we don't end * up freeing the same block twice. */ - if (list_head(&zilog->zl_lwb_list) == NULL) + if (list_is_empty(&zilog->zl_lwb_list)) BP_ZERO(&zh->zh_log); } @@ -3395,7 +3367,7 @@ zil_close(zilog_t *zilog) if (!dmu_objset_is_snapshot(zilog->zl_os)) { zil_commit(zilog, 0); } else { - ASSERT3P(list_tail(&zilog->zl_lwb_list), ==, NULL); + ASSERT(list_is_empty(&zilog->zl_lwb_list)); ASSERT0(zilog->zl_dirty_max_txg); ASSERT3B(zilog_is_dirty(zilog), ==, B_FALSE); } @@ -3437,15 +3409,14 @@ zil_close(zilog_t *zilog) * 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); + lwb = list_remove_head(&zilog->zl_lwb_list); if (lwb != NULL) { - ASSERT3P(lwb, ==, list_tail(&zilog->zl_lwb_list)); + ASSERT(list_is_empty(&zilog->zl_lwb_list)); ASSERT3S(lwb->lwb_state, !=, LWB_STATE_ISSUED); if (lwb->lwb_fastwrite) metaslab_fastwrite_unmark(zilog->zl_spa, &lwb->lwb_blk); - list_remove(&zilog->zl_lwb_list, lwb); zio_buf_free(lwb->lwb_buf, lwb->lwb_sz); zil_free_lwb(zilog, lwb); } From a531c6079573eb882d481bc6d8f0a9430d8c5bdf Mon Sep 17 00:00:00 2001 From: Alexander Motin Date: Thu, 25 May 2023 12:48:43 -0400 Subject: [PATCH 09/15] ZIL: Reduce scope of per-dataset zl_issuer_lock. Before this change ZIL copied all log data while holding the lock. It caused huge lock contention on workloads with many big parallel writes. This change splits the process into two parts: first, zil_lwb_assign() estimates the log space needed for all transactions, and zil_lwb_write_close() allocates blocks and zios while holding the lock, then, after the lock in dropped, zil_lwb_commit() copies the data, and zil_lwb_write_issue() issues the I/Os. Also while there slightly reduce scope of zl_lock. Reviewed-by: Paul Dagnelie Reviewed-by: Prakash Surya Reviewed-by: Richard Yao Signed-off-by: Alexander Motin Sponsored by: iXsystems, Inc. Closes #14841 (cherry picked from commit f63811f07213361d49878648bd597af88d06859c) --- include/sys/zil_impl.h | 7 +- module/zfs/zil.c | 425 +++++++++++++++++++++++++++-------------- 2 files changed, 284 insertions(+), 148 deletions(-) diff --git a/include/sys/zil_impl.h b/include/sys/zil_impl.h index 8409ce864e90..4412261659f6 100644 --- a/include/sys/zil_impl.h +++ b/include/sys/zil_impl.h @@ -44,7 +44,7 @@ extern "C" { * must be held. * * After the lwb is "opened", it can transition into the "issued" state - * via zil_lwb_write_issue(). Again, the zilog's "zl_issuer_lock" must + * via zil_lwb_write_close(). Again, the zilog's "zl_issuer_lock" must * be held when making this transition. * * After the lwb's write zio completes, it transitions into the "write @@ -93,20 +93,23 @@ typedef struct lwb { blkptr_t lwb_blk; /* on disk address of this log blk */ boolean_t lwb_fastwrite; /* is blk marked for fastwrite? */ boolean_t lwb_slog; /* lwb_blk is on SLOG device */ + boolean_t lwb_indirect; /* do not postpone zil_lwb_commit() */ int lwb_nused; /* # used bytes in buffer */ + int lwb_nfilled; /* # filled bytes in buffer */ int lwb_sz; /* size of block and buffer */ lwb_state_t lwb_state; /* the state of this lwb */ char *lwb_buf; /* log write buffer */ zio_t *lwb_write_zio; /* zio for the lwb buffer */ zio_t *lwb_root_zio; /* root zio for lwb write and flushes */ + hrtime_t lwb_issued_timestamp; /* when was the lwb issued? */ uint64_t lwb_issued_txg; /* the txg when the write is issued */ uint64_t lwb_max_txg; /* highest txg in this lwb */ list_node_t lwb_node; /* zilog->zl_lwb_list linkage */ + list_node_t lwb_issue_node; /* linkage of lwbs ready for issue */ list_t lwb_itxs; /* list of itx's */ list_t lwb_waiters; /* list of zil_commit_waiter's */ avl_tree_t lwb_vdev_tree; /* vdevs to flush after lwb write */ kmutex_t lwb_vdev_lock; /* protects lwb_vdev_tree */ - hrtime_t lwb_issued_timestamp; /* when was the lwb issued? */ } lwb_t; /* diff --git a/module/zfs/zil.c b/module/zfs/zil.c index 58f7e7767ee0..443732efc39f 100644 --- a/module/zfs/zil.c +++ b/module/zfs/zil.c @@ -143,6 +143,9 @@ unsigned long zil_slog_bulk = 768 * 1024; static kmem_cache_t *zil_lwb_cache; static kmem_cache_t *zil_zcw_cache; +static void zil_lwb_commit(zilog_t *zilog, lwb_t *lwb, itx_t *itx); +static itx_t *zil_itx_clone(itx_t *oitx); + static int zil_bp_compare(const void *x1, const void *x2) { @@ -561,20 +564,21 @@ zil_alloc_lwb(zilog_t *zilog, blkptr_t *bp, boolean_t slog, uint64_t txg, lwb->lwb_blk = *bp; lwb->lwb_fastwrite = fastwrite; lwb->lwb_slog = slog; + lwb->lwb_indirect = B_FALSE; + if (BP_GET_CHECKSUM(bp) == ZIO_CHECKSUM_ZILOG2) { + lwb->lwb_nused = lwb->lwb_nfilled = sizeof (zil_chain_t); + lwb->lwb_sz = BP_GET_LSIZE(bp); + } else { + lwb->lwb_nused = lwb->lwb_nfilled = 0; + lwb->lwb_sz = BP_GET_LSIZE(bp) - sizeof (zil_chain_t); + } lwb->lwb_state = LWB_STATE_CLOSED; lwb->lwb_buf = zio_buf_alloc(BP_GET_LSIZE(bp)); - lwb->lwb_max_txg = txg; lwb->lwb_write_zio = NULL; lwb->lwb_root_zio = NULL; lwb->lwb_issued_timestamp = 0; lwb->lwb_issued_txg = 0; - if (BP_GET_CHECKSUM(bp) == ZIO_CHECKSUM_ZILOG2) { - lwb->lwb_nused = sizeof (zil_chain_t); - lwb->lwb_sz = BP_GET_LSIZE(bp); - } else { - lwb->lwb_nused = 0; - lwb->lwb_sz = BP_GET_LSIZE(bp) - sizeof (zil_chain_t); - } + lwb->lwb_max_txg = txg; mutex_enter(&zilog->zl_lock); list_insert_tail(&zilog->zl_lwb_list, lwb); @@ -1162,6 +1166,8 @@ zil_lwb_flush_vdevs_done(zio_t *zio) zilog->zl_commit_lr_seq = zilog->zl_lr_seq; } + mutex_exit(&zilog->zl_lock); + while ((itx = list_remove_head(&lwb->lwb_itxs)) != NULL) zil_itx_destroy(itx); @@ -1194,8 +1200,6 @@ zil_lwb_flush_vdevs_done(zio_t *zio) mutex_exit(&zcw->zcw_lock); } - mutex_exit(&zilog->zl_lock); - mutex_enter(&zilog->zl_lwb_io_lock); txg = lwb->lwb_issued_txg; ASSERT3U(zilog->zl_lwb_inflight[txg & TXG_MASK], >, 0); @@ -1428,46 +1432,42 @@ zil_lwb_write_open(zilog_t *zilog, lwb_t *lwb) EQUIV(lwb->lwb_root_zio == NULL, lwb->lwb_state == LWB_STATE_CLOSED); EQUIV(lwb->lwb_root_zio != NULL, lwb->lwb_state == LWB_STATE_OPENED); + if (lwb->lwb_root_zio != NULL) + return; + + lwb->lwb_root_zio = zio_root(zilog->zl_spa, + zil_lwb_flush_vdevs_done, lwb, ZIO_FLAG_CANFAIL); + + abd_t *lwb_abd = abd_get_from_buf(lwb->lwb_buf, + BP_GET_LSIZE(&lwb->lwb_blk)); + + if (!lwb->lwb_slog || zilog->zl_cur_used <= zil_slog_bulk) + prio = ZIO_PRIORITY_SYNC_WRITE; + else + prio = ZIO_PRIORITY_ASYNC_WRITE; + SET_BOOKMARK(&zb, lwb->lwb_blk.blk_cksum.zc_word[ZIL_ZC_OBJSET], ZB_ZIL_OBJECT, ZB_ZIL_LEVEL, lwb->lwb_blk.blk_cksum.zc_word[ZIL_ZC_SEQ]); /* Lock so zil_sync() doesn't fastwrite_unmark after zio is created */ mutex_enter(&zilog->zl_lock); - if (lwb->lwb_root_zio == NULL) { - abd_t *lwb_abd = abd_get_from_buf(lwb->lwb_buf, - BP_GET_LSIZE(&lwb->lwb_blk)); - - if (!lwb->lwb_fastwrite) { - metaslab_fastwrite_mark(zilog->zl_spa, &lwb->lwb_blk); - lwb->lwb_fastwrite = 1; - } - - if (!lwb->lwb_slog || zilog->zl_cur_used <= zil_slog_bulk) - prio = ZIO_PRIORITY_SYNC_WRITE; - else - prio = ZIO_PRIORITY_ASYNC_WRITE; - - lwb->lwb_root_zio = zio_root(zilog->zl_spa, - zil_lwb_flush_vdevs_done, lwb, ZIO_FLAG_CANFAIL); - ASSERT3P(lwb->lwb_root_zio, !=, NULL); + if (!lwb->lwb_fastwrite) { + metaslab_fastwrite_mark(zilog->zl_spa, &lwb->lwb_blk); + lwb->lwb_fastwrite = 1; + } - lwb->lwb_write_zio = zio_rewrite(lwb->lwb_root_zio, - zilog->zl_spa, 0, &lwb->lwb_blk, lwb_abd, - BP_GET_LSIZE(&lwb->lwb_blk), zil_lwb_write_done, lwb, - prio, ZIO_FLAG_CANFAIL | ZIO_FLAG_FASTWRITE, &zb); - ASSERT3P(lwb->lwb_write_zio, !=, NULL); + lwb->lwb_write_zio = zio_rewrite(lwb->lwb_root_zio, zilog->zl_spa, 0, + &lwb->lwb_blk, lwb_abd, BP_GET_LSIZE(&lwb->lwb_blk), + zil_lwb_write_done, lwb, prio, + ZIO_FLAG_CANFAIL | ZIO_FLAG_DONT_PROPAGATE | ZIO_FLAG_FASTWRITE, + &zb); - lwb->lwb_state = LWB_STATE_OPENED; + lwb->lwb_state = LWB_STATE_OPENED; - zil_lwb_set_zio_dependency(zilog, lwb); - zilog->zl_last_lwb_opened = lwb; - } + zil_lwb_set_zio_dependency(zilog, lwb); + zilog->zl_last_lwb_opened = lwb; mutex_exit(&zilog->zl_lock); - - ASSERT3P(lwb->lwb_root_zio, !=, NULL); - ASSERT3P(lwb->lwb_write_zio, !=, NULL); - ASSERT3S(lwb->lwb_state, ==, LWB_STATE_OPENED); } /* @@ -1498,11 +1498,11 @@ struct { int zil_maxblocksize = SPA_OLD_MAXBLOCKSIZE; /* - * Start a log block write and advance to the next log block. - * Calls are serialized. + * Close the log block for being issued and allocate the next one. + * Has to be called under zl_issuer_lock to chain more lwbs. */ static lwb_t * -zil_lwb_write_issue(zilog_t *zilog, lwb_t *lwb) +zil_lwb_write_close(zilog_t *zilog, lwb_t *lwb) { lwb_t *nlwb = NULL; zil_chain_t *zilc; @@ -1510,7 +1510,7 @@ zil_lwb_write_issue(zilog_t *zilog, lwb_t *lwb) blkptr_t *bp; dmu_tx_t *tx; uint64_t txg; - uint64_t zil_blksz, wsz; + uint64_t zil_blksz; int i, error; boolean_t slog; @@ -1519,16 +1519,17 @@ zil_lwb_write_issue(zilog_t *zilog, lwb_t *lwb) ASSERT3P(lwb->lwb_write_zio, !=, NULL); ASSERT3S(lwb->lwb_state, ==, LWB_STATE_OPENED); - if (BP_GET_CHECKSUM(&lwb->lwb_blk) == ZIO_CHECKSUM_ZILOG2) { - zilc = (zil_chain_t *)lwb->lwb_buf; - bp = &zilc->zc_next_blk; - } else { - zilc = (zil_chain_t *)(lwb->lwb_buf + lwb->lwb_sz); - bp = &zilc->zc_next_blk; + /* + * If this lwb includes indirect writes, we have to commit before + * creating the transaction, otherwise we may end up in dead lock. + */ + if (lwb->lwb_indirect) { + for (itx_t *itx = list_head(&lwb->lwb_itxs); itx; + itx = list_next(&lwb->lwb_itxs, itx)) + zil_lwb_commit(zilog, lwb, itx); + lwb->lwb_nused = lwb->lwb_nfilled; } - ASSERT(lwb->lwb_nused <= lwb->lwb_sz); - /* * Allocate the next block and save its address in this block * before writing it in order to establish the log chain. @@ -1578,15 +1579,13 @@ zil_lwb_write_issue(zilog_t *zilog, lwb_t *lwb) zil_blksz = MAX(zil_blksz, zilog->zl_prev_blks[i]); zilog->zl_prev_rotor = (zilog->zl_prev_rotor + 1) & (ZIL_PREV_BLKS - 1); + if (BP_GET_CHECKSUM(&lwb->lwb_blk) == ZIO_CHECKSUM_ZILOG2) + zilc = (zil_chain_t *)lwb->lwb_buf; + else + zilc = (zil_chain_t *)(lwb->lwb_buf + lwb->lwb_sz); + bp = &zilc->zc_next_blk; BP_ZERO(bp); error = zio_alloc_zil(spa, zilog->zl_os, txg, bp, zil_blksz, &slog); - if (slog) { - ZIL_STAT_BUMP(zil_itx_metaslab_slog_count); - ZIL_STAT_INCR(zil_itx_metaslab_slog_bytes, lwb->lwb_nused); - } else { - ZIL_STAT_BUMP(zil_itx_metaslab_normal_count); - ZIL_STAT_INCR(zil_itx_metaslab_normal_bytes, lwb->lwb_nused); - } if (error == 0) { ASSERT3U(bp->blk_birth, ==, txg); bp->blk_cksum = lwb->lwb_blk.blk_cksum; @@ -1598,16 +1597,46 @@ zil_lwb_write_issue(zilog_t *zilog, lwb_t *lwb) nlwb = zil_alloc_lwb(zilog, bp, slog, txg, TRUE); } + lwb->lwb_state = LWB_STATE_ISSUED; + + dmu_tx_commit(tx); + + /* + * If there was an allocation failure then nlwb will be null which + * forces a txg_wait_synced(). + */ + return (nlwb); +} + +/* + * Finalize previously closed block and issue the write zio. + * Does not require locking. + */ +static void +zil_lwb_write_issue(zilog_t *zilog, lwb_t *lwb) +{ + zil_chain_t *zilc; + int wsz; + + /* Actually fill the lwb with the data if not yet. */ + if (!lwb->lwb_indirect) { + for (itx_t *itx = list_head(&lwb->lwb_itxs); itx; + itx = list_next(&lwb->lwb_itxs, itx)) + zil_lwb_commit(zilog, lwb, itx); + lwb->lwb_nused = lwb->lwb_nfilled; + } + if (BP_GET_CHECKSUM(&lwb->lwb_blk) == ZIO_CHECKSUM_ZILOG2) { /* For Slim ZIL only write what is used. */ - wsz = P2ROUNDUP_TYPED(lwb->lwb_nused, ZIL_MIN_BLKSZ, uint64_t); - ASSERT3U(wsz, <=, lwb->lwb_sz); + wsz = P2ROUNDUP_TYPED(lwb->lwb_nused, ZIL_MIN_BLKSZ, int); + ASSERT3S(wsz, <=, lwb->lwb_sz); zio_shrink(lwb->lwb_write_zio, wsz); + zilc = (zil_chain_t *)lwb->lwb_buf; } else { wsz = lwb->lwb_sz; + zilc = (zil_chain_t *)(lwb->lwb_buf + lwb->lwb_sz); } - zilc->zc_pad = 0; zilc->zc_nused = lwb->lwb_nused; zilc->zc_eck.zec_cksum = lwb->lwb_blk.blk_cksum; @@ -1617,22 +1646,18 @@ zil_lwb_write_issue(zilog_t *zilog, lwb_t *lwb) */ bzero(lwb->lwb_buf + lwb->lwb_nused, wsz - lwb->lwb_nused); + if (lwb->lwb_slog) { + ZIL_STAT_BUMP(zil_itx_metaslab_slog_count); + ZIL_STAT_INCR(zil_itx_metaslab_slog_bytes, lwb->lwb_nused); + } else { + ZIL_STAT_BUMP(zil_itx_metaslab_normal_count); + ZIL_STAT_INCR(zil_itx_metaslab_normal_bytes, lwb->lwb_nused); + } spa_config_enter(zilog->zl_spa, SCL_STATE, lwb, RW_READER); - zil_lwb_add_block(lwb, &lwb->lwb_blk); lwb->lwb_issued_timestamp = gethrtime(); - lwb->lwb_state = LWB_STATE_ISSUED; - zio_nowait(lwb->lwb_root_zio); zio_nowait(lwb->lwb_write_zio); - - dmu_tx_commit(tx); - - /* - * If there was an allocation failure then nlwb will be null which - * forces a txg_wait_synced(). - */ - return (nlwb); } /* @@ -1669,13 +1694,19 @@ zil_max_copied_data(zilog_t *zilog) sizeof (lr_write_t)); } +/* + * Estimate space needed in the lwb for the itx. Allocate more lwbs or + * split the itx as needed, but don't touch the actual transaction data. + * Has to be called under zl_issuer_lock to call zil_lwb_write_close() + * to chain more lwbs. + */ static lwb_t * -zil_lwb_commit(zilog_t *zilog, itx_t *itx, lwb_t *lwb) +zil_lwb_assign(zilog_t *zilog, lwb_t *lwb, itx_t *itx, list_t *ilwbs) { - lr_t *lrcb, *lrc; - lr_write_t *lrwb, *lrw; - char *lr_buf; - uint64_t dlen, dnow, dpad, lwb_sp, reclen, txg, max_log_data; + itx_t *citx; + lr_t *lr, *clr; + lr_write_t *lrw; + uint64_t dlen, dnow, lwb_sp, reclen, max_log_data; ASSERT(MUTEX_HELD(&zilog->zl_issuer_lock)); ASSERT3P(lwb, !=, NULL); @@ -1683,8 +1714,8 @@ zil_lwb_commit(zilog_t *zilog, itx_t *itx, lwb_t *lwb) zil_lwb_write_open(zilog, lwb); - lrc = &itx->itx_lr; - lrw = (lr_write_t *)lrc; + lr = &itx->itx_lr; + lrw = (lr_write_t *)lr; /* * A commit itx doesn't represent any on-disk state; instead @@ -1698,24 +1729,23 @@ zil_lwb_commit(zilog_t *zilog, itx_t *itx, lwb_t *lwb) * * For more details, see the comment above zil_commit(). */ - if (lrc->lrc_txtype == TX_COMMIT) { + if (lr->lrc_txtype == TX_COMMIT) { mutex_enter(&zilog->zl_lock); zil_commit_waiter_link_lwb(itx->itx_private, lwb); itx->itx_private = NULL; mutex_exit(&zilog->zl_lock); + list_insert_tail(&lwb->lwb_itxs, itx); return (lwb); } - if (lrc->lrc_txtype == TX_WRITE && itx->itx_wr_state == WR_NEED_COPY) { + if (lr->lrc_txtype == TX_WRITE && itx->itx_wr_state == WR_NEED_COPY) { dlen = P2ROUNDUP_TYPED( lrw->lr_length, sizeof (uint64_t), uint64_t); - dpad = dlen - lrw->lr_length; } else { - dlen = dpad = 0; + dlen = 0; } - reclen = lrc->lrc_reclen; + reclen = lr->lrc_reclen; zilog->zl_cur_used += (reclen + dlen); - txg = lrc->lrc_txg; cont: /* @@ -1728,7 +1758,8 @@ zil_lwb_commit(zilog_t *zilog, itx_t *itx, lwb_t *lwb) lwb_sp < zil_max_waste_space(zilog) && (dlen % max_log_data == 0 || lwb_sp < reclen + dlen % max_log_data))) { - lwb = zil_lwb_write_issue(zilog, lwb); + list_insert_tail(ilwbs, lwb); + lwb = zil_lwb_write_close(zilog, lwb); if (lwb == NULL) return (NULL); zil_lwb_write_open(zilog, lwb); @@ -1747,19 +1778,99 @@ zil_lwb_commit(zilog_t *zilog, itx_t *itx, lwb_t *lwb) } dnow = MIN(dlen, lwb_sp - reclen); - lr_buf = lwb->lwb_buf + lwb->lwb_nused; - bcopy(lrc, lr_buf, reclen); - lrcb = (lr_t *)lr_buf; /* Like lrc, but inside lwb. */ - lrwb = (lr_write_t *)lrcb; /* Like lrw, but inside lwb. */ + if (dlen > dnow) { + ASSERT3U(lr->lrc_txtype, ==, TX_WRITE); + ASSERT3U(itx->itx_wr_state, ==, WR_NEED_COPY); + citx = zil_itx_clone(itx); + clr = &citx->itx_lr; + lr_write_t *clrw = (lr_write_t *)clr; + clrw->lr_length = dnow; + lrw->lr_offset += dnow; + lrw->lr_length -= dnow; + } else { + citx = itx; + clr = lr; + } + + /* + * We're actually making an entry, so update lrc_seq to be the + * log record sequence number. Note that this is generally not + * equal to the itx sequence number because not all transactions + * are synchronous, and sometimes spa_sync() gets there first. + */ + clr->lrc_seq = ++zilog->zl_lr_seq; + + lwb->lwb_nused += reclen + dnow; + ASSERT3U(lwb->lwb_nused, <=, lwb->lwb_sz); + ASSERT0(P2PHASE(lwb->lwb_nused, sizeof (uint64_t))); + + zil_lwb_add_txg(lwb, lr->lrc_txg); + list_insert_tail(&lwb->lwb_itxs, citx); + + dlen -= dnow; + if (dlen > 0) { + zilog->zl_cur_used += reclen; + goto cont; + } + + /* + * We have to really issue all queued LWBs before we may have to + * wait for a txg sync. Otherwise we may end up in a dead lock. + */ + if (lr->lrc_txtype == TX_WRITE) { + boolean_t frozen = lr->lrc_txg > spa_freeze_txg(zilog->zl_spa); + if (frozen || itx->itx_wr_state == WR_INDIRECT) { + lwb_t *tlwb; + while ((tlwb = list_remove_head(ilwbs)) != NULL) + zil_lwb_write_issue(zilog, tlwb); + } + if (itx->itx_wr_state == WR_INDIRECT) + lwb->lwb_indirect = B_TRUE; + if (frozen) + txg_wait_synced(zilog->zl_dmu_pool, lr->lrc_txg); + } + + return (lwb); +} + +/* + * Fill the actual transaction data into the lwb, following zil_lwb_assign(). + * Does not require locking. + */ +static void +zil_lwb_commit(zilog_t *zilog, lwb_t *lwb, itx_t *itx) +{ + lr_t *lr, *lrb; + lr_write_t *lrw, *lrwb; + char *lr_buf; + uint64_t dlen, reclen; + + lr = &itx->itx_lr; + lrw = (lr_write_t *)lr; + + if (lr->lrc_txtype == TX_COMMIT) + return; + + if (lr->lrc_txtype == TX_WRITE && itx->itx_wr_state == WR_NEED_COPY) { + dlen = P2ROUNDUP_TYPED( + lrw->lr_length, sizeof (uint64_t), uint64_t); + } else { + dlen = 0; + } + reclen = lr->lrc_reclen; + ASSERT3U(reclen + dlen, <=, lwb->lwb_nused - lwb->lwb_nfilled); + + lr_buf = lwb->lwb_buf + lwb->lwb_nfilled; + memcpy(lr_buf, lr, reclen); + lrb = (lr_t *)lr_buf; /* Like lr, but inside lwb. */ + lrwb = (lr_write_t *)lrb; /* Like lrw, but inside lwb. */ ZIL_STAT_BUMP(zil_itx_count); /* * If it's a write, fetch the data or get its blkptr as appropriate. */ - if (lrc->lrc_txtype == TX_WRITE) { - if (txg > spa_freeze_txg(zilog->zl_spa)) - txg_wait_synced(zilog->zl_dmu_pool, txg); + if (lr->lrc_txtype == TX_WRITE) { if (itx->itx_wr_state == WR_COPIED) { ZIL_STAT_BUMP(zil_itx_copied_count); ZIL_STAT_INCR(zil_itx_copied_bytes, lrw->lr_length); @@ -1769,13 +1880,9 @@ zil_lwb_commit(zilog_t *zilog, itx_t *itx, lwb_t *lwb) if (itx->itx_wr_state == WR_NEED_COPY) { dbuf = lr_buf + reclen; - lrcb->lrc_reclen += dnow; - if (lrwb->lr_length > dnow) - lrwb->lr_length = dnow; - lrw->lr_offset += dnow; - lrw->lr_length -= dnow; + lrb->lrc_reclen += dlen; ZIL_STAT_BUMP(zil_itx_needcopy_count); - ZIL_STAT_INCR(zil_itx_needcopy_bytes, dnow); + ZIL_STAT_INCR(zil_itx_needcopy_bytes, dlen); } else { ASSERT3S(itx->itx_wr_state, ==, WR_INDIRECT); dbuf = NULL; @@ -1802,9 +1909,11 @@ zil_lwb_commit(zilog_t *zilog, itx_t *itx, lwb_t *lwb) error = zilog->zl_get_data(itx->itx_private, itx->itx_gen, lrwb, dbuf, lwb, lwb->lwb_write_zio); - if (dbuf != NULL && error == 0 && dnow == dlen) + if (dbuf != NULL && error == 0) { /* Zero any padding bytes in the last block. */ - bzero((char *)dbuf + lrwb->lr_length, dpad); + memset((char *)dbuf + lrwb->lr_length, 0, + dlen - lrwb->lr_length); + } /* * Typically, the only return values we should see from @@ -1832,39 +1941,26 @@ zil_lwb_commit(zilog_t *zilog, itx_t *itx, lwb_t *lwb) error); zfs_fallthrough; case EIO: - txg_wait_synced(zilog->zl_dmu_pool, txg); + if (lwb->lwb_indirect) { + txg_wait_synced(zilog->zl_dmu_pool, + lr->lrc_txg); + } else { + lwb->lwb_write_zio->io_error = error; + } zfs_fallthrough; case ENOENT: zfs_fallthrough; case EEXIST: zfs_fallthrough; case EALREADY: - return (lwb); + return; } } } - /* - * We're actually making an entry, so update lrc_seq to be the - * log record sequence number. Note that this is generally not - * equal to the itx sequence number because not all transactions - * are synchronous, and sometimes spa_sync() gets there first. - */ - lrcb->lrc_seq = ++zilog->zl_lr_seq; - lwb->lwb_nused += reclen + dnow; - - zil_lwb_add_txg(lwb, txg); - - ASSERT3U(lwb->lwb_nused, <=, lwb->lwb_sz); - ASSERT0(P2PHASE(lwb->lwb_nused, sizeof (uint64_t))); - - dlen -= dnow; - if (dlen > 0) { - zilog->zl_cur_used += reclen; - goto cont; - } - - return (lwb); + lwb->lwb_nfilled += reclen + dlen; + ASSERT3S(lwb->lwb_nfilled, <=, lwb->lwb_nused); + ASSERT0(P2PHASE(lwb->lwb_nfilled, sizeof (uint64_t))); } itx_t * @@ -1889,6 +1985,16 @@ zil_itx_create(uint64_t txtype, size_t olrsize) return (itx); } +static itx_t * +zil_itx_clone(itx_t *oitx) +{ + itx_t *itx = zio_data_buf_alloc(oitx->itx_size); + memcpy(itx, oitx, oitx->itx_size); + itx->itx_callback = NULL; + itx->itx_callback_data = NULL; + return (itx); +} + void zil_itx_destroy(itx_t *itx) { @@ -1920,7 +2026,7 @@ zil_itxg_clean(void *arg) /* * In the general case, commit itxs will not be found * here, as they'll be committed to an lwb via - * zil_lwb_commit(), and free'd in that function. Having + * zil_lwb_assign(), and free'd in that function. Having * said that, it is still possible for commit itxs to be * found here, due to the following race: * @@ -2319,7 +2425,7 @@ zil_commit_writer_stall(zilog_t *zilog) * lwb will be issued to the zio layer to be written to disk. */ static void -zil_process_commit_list(zilog_t *zilog) +zil_process_commit_list(zilog_t *zilog, zil_commit_waiter_t *zcw, list_t *ilwbs) { spa_t *spa = zilog->zl_spa; list_t nolwb_itxs; @@ -2416,18 +2522,23 @@ zil_process_commit_list(zilog_t *zilog) */ if (frozen || !synced || lrc->lrc_txtype == TX_COMMIT) { if (lwb != NULL) { - lwb = zil_lwb_commit(zilog, itx, lwb); - - if (lwb == NULL) + lwb = zil_lwb_assign(zilog, lwb, itx, ilwbs); + if (lwb == NULL) { list_insert_tail(&nolwb_itxs, itx); - else - list_insert_tail(&lwb->lwb_itxs, itx); + } else if ((zcw->zcw_lwb != NULL && + zcw->zcw_lwb != lwb) || zcw->zcw_done) { + /* + * Our lwb is done, leave the rest of + * itx list to somebody else who care. + */ + first = B_FALSE; + break; + } } else { if (lrc->lrc_txtype == TX_COMMIT) { zil_commit_waiter_link_nolwb( itx->itx_private, &nolwb_waiters); } - list_insert_tail(&nolwb_itxs, itx); } } else { @@ -2443,6 +2554,8 @@ zil_process_commit_list(zilog_t *zilog) * the ZIL write pipeline; see the comment within * zil_commit_writer_stall() for more details. */ + while ((lwb = list_remove_head(ilwbs)) != NULL) + zil_lwb_write_issue(zilog, lwb); zil_commit_writer_stall(zilog); /* @@ -2488,13 +2601,13 @@ zil_process_commit_list(zilog_t *zilog) * on the system, such that this function will be * immediately called again (not necessarily by the same * thread) and this lwb's zio will be issued via - * zil_lwb_commit(). This way, the lwb is guaranteed to + * zil_lwb_assign(). This way, the lwb is guaranteed to * be "full" when it is issued to disk, and we'll make * use of the lwb's size the best we can. * * 2. If there isn't sufficient ZIL activity occurring on * the system, such that this lwb's zio isn't issued via - * zil_lwb_commit(), zil_commit_waiter() will issue the + * zil_lwb_assign(), zil_commit_waiter() will issue the * lwb's zio. If this occurs, the lwb is not guaranteed * to be "full" by the time its zio is issued, and means * the size of the lwb was "too large" given the amount @@ -2526,10 +2639,15 @@ zil_process_commit_list(zilog_t *zilog) zfs_commit_timeout_pct / 100; if (sleep < zil_min_commit_timeout || lwb->lwb_sz - lwb->lwb_nused < lwb->lwb_sz / 8) { - lwb = zil_lwb_write_issue(zilog, lwb); + list_insert_tail(ilwbs, lwb); + lwb = zil_lwb_write_close(zilog, lwb); zilog->zl_cur_used = 0; - if (lwb == NULL) + if (lwb == NULL) { + while ((lwb = list_remove_head(ilwbs)) + != NULL) + zil_lwb_write_issue(zilog, lwb); zil_commit_writer_stall(zilog); + } } } } @@ -2552,9 +2670,13 @@ zil_process_commit_list(zilog_t *zilog) static void zil_commit_writer(zilog_t *zilog, zil_commit_waiter_t *zcw) { + list_t ilwbs; + lwb_t *lwb; + ASSERT(!MUTEX_HELD(&zilog->zl_lock)); ASSERT(spa_writeable(zilog->zl_spa)); + list_create(&ilwbs, sizeof (lwb_t), offsetof(lwb_t, lwb_issue_node)); mutex_enter(&zilog->zl_issuer_lock); if (zcw->zcw_lwb != NULL || zcw->zcw_done) { @@ -2581,10 +2703,13 @@ zil_commit_writer(zilog_t *zilog, zil_commit_waiter_t *zcw) zil_get_commit_list(zilog); zil_prune_commit_list(zilog); - zil_process_commit_list(zilog); + zil_process_commit_list(zilog, zcw, &ilwbs); out: mutex_exit(&zilog->zl_issuer_lock); + while ((lwb = list_remove_head(&ilwbs)) != NULL) + zil_lwb_write_issue(zilog, lwb); + list_destroy(&ilwbs); } static void @@ -2611,7 +2736,7 @@ zil_commit_waiter_timeout(zilog_t *zilog, zil_commit_waiter_t *zcw) return; /* - * In order to call zil_lwb_write_issue() we must hold the + * In order to call zil_lwb_write_close() we must hold the * zilog's "zl_issuer_lock". We can't simply acquire that lock, * since we're already holding the commit waiter's "zcw_lock", * and those two locks are acquired in the opposite order @@ -2629,8 +2754,10 @@ zil_commit_waiter_timeout(zilog_t *zilog, zil_commit_waiter_t *zcw) * the waiter is marked "done"), so without this check we could * wind up with a use-after-free error below. */ - if (zcw->zcw_done) + if (zcw->zcw_done) { + lwb = NULL; goto out; + } ASSERT3P(lwb, ==, zcw->zcw_lwb); @@ -2649,15 +2776,17 @@ zil_commit_waiter_timeout(zilog_t *zilog, zil_commit_waiter_t *zcw) * if it's ISSUED or OPENED, and block any other threads that might * attempt to issue this lwb. For that reason we hold the * zl_issuer_lock when checking the lwb_state; we must not call - * zil_lwb_write_issue() if the lwb had already been issued. + * zil_lwb_write_close() if the lwb had already been issued. * * See the comment above the lwb_state_t structure definition for * more details on the lwb states, and locking requirements. */ if (lwb->lwb_state == LWB_STATE_ISSUED || lwb->lwb_state == LWB_STATE_WRITE_DONE || - lwb->lwb_state == LWB_STATE_FLUSH_DONE) + lwb->lwb_state == LWB_STATE_FLUSH_DONE) { + lwb = NULL; goto out; + } ASSERT3S(lwb->lwb_state, ==, LWB_STATE_OPENED); @@ -2667,7 +2796,7 @@ zil_commit_waiter_timeout(zilog_t *zilog, zil_commit_waiter_t *zcw) * since we've reached the commit waiter's timeout and it still * hasn't been issued. */ - lwb_t *nlwb = zil_lwb_write_issue(zilog, lwb); + lwb_t *nlwb = zil_lwb_write_close(zilog, lwb); ASSERT3S(lwb->lwb_state, !=, LWB_STATE_OPENED); @@ -2687,7 +2816,7 @@ zil_commit_waiter_timeout(zilog_t *zilog, zil_commit_waiter_t *zcw) if (nlwb == NULL) { /* - * When zil_lwb_write_issue() returns NULL, this + * When zil_lwb_write_close() returns NULL, this * indicates zio_alloc_zil() failed to allocate the * "next" lwb on-disk. When this occurs, the ZIL write * pipeline must be stalled; see the comment within the @@ -2709,12 +2838,16 @@ zil_commit_waiter_timeout(zilog_t *zilog, zil_commit_waiter_t *zcw) * lock, which occurs prior to calling dmu_tx_commit() */ mutex_exit(&zcw->zcw_lock); + zil_lwb_write_issue(zilog, lwb); + lwb = NULL; zil_commit_writer_stall(zilog); mutex_enter(&zcw->zcw_lock); } out: mutex_exit(&zilog->zl_issuer_lock); + if (lwb) + zil_lwb_write_issue(zilog, lwb); ASSERT(MUTEX_HELD(&zcw->zcw_lock)); } @@ -2729,7 +2862,7 @@ zil_commit_waiter_timeout(zilog_t *zilog, zil_commit_waiter_t *zcw) * waited "long enough" and the lwb is still in the "open" state. * * Given a sufficient amount of itxs being generated and written using - * the ZIL, the lwb's zio will be issued via the zil_lwb_commit() + * the ZIL, the lwb's zio will be issued via the zil_lwb_assign() * function. If this does not occur, this secondary responsibility will * ensure the lwb is issued even if there is not other synchronous * activity on the system. @@ -3383,7 +3516,7 @@ zil_close(zilog_t *zilog) /* * zl_lwb_max_issued_txg may be larger than lwb_max_txg. It depends * on the time when the dmu_tx transaction is assigned in - * zil_lwb_write_issue(). + * zil_lwb_write_close(). */ mutex_enter(&zilog->zl_lwb_io_lock); txg = MAX(zilog->zl_lwb_max_issued_txg, txg); From 0eb8365388b65bd9defbe222bb59854d622220c1 Mon Sep 17 00:00:00 2001 From: Alexander Motin Date: Fri, 9 Jun 2023 13:08:05 -0400 Subject: [PATCH 10/15] ZIL: Fix race introduced by f63811f0721. We are not allowed to access lwb after setting LWB_STATE_FLUSH_DONE state and dropping zl_lock, since it may be freed by zil_sync(). To free itxs and waiters after dropping the lock we need to move lwb_itxs and lwb_waiters lists elements to local storage. Reviewed-by: Brian Behlendorf Signed-off-by: Alexander Motin Sponsored by: iXsystems, Inc. Closes #14957 Closes #14959 (cherry picked from commit 55b1842f92a24fe7192d129bca7b60882080d31a) --- module/zfs/zil.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/module/zfs/zil.c b/module/zfs/zil.c index 443732efc39f..504dcaab1219 100644 --- a/module/zfs/zil.c +++ b/module/zfs/zil.c @@ -1132,10 +1132,16 @@ zil_lwb_flush_vdevs_done(zio_t *zio) zil_commit_waiter_t *zcw; itx_t *itx; uint64_t txg; + list_t itxs, waiters; spa_config_exit(zilog->zl_spa, SCL_STATE, lwb); zio_buf_free(lwb->lwb_buf, lwb->lwb_sz); + + list_create(&itxs, sizeof (itx_t), offsetof(itx_t, itx_node)); + list_create(&waiters, sizeof (zil_commit_waiter_t), + offsetof(zil_commit_waiter_t, zcw_node)); + hrtime_t t = gethrtime() - lwb->lwb_issued_timestamp; mutex_enter(&zilog->zl_lock); @@ -1153,9 +1159,6 @@ zil_lwb_flush_vdevs_done(zio_t *zio) lwb->lwb_root_zio = NULL; - ASSERT3S(lwb->lwb_state, ==, LWB_STATE_WRITE_DONE); - lwb->lwb_state = LWB_STATE_FLUSH_DONE; - if (zilog->zl_last_lwb_opened == lwb) { /* * Remember the highest committed log sequence number @@ -1166,15 +1169,21 @@ zil_lwb_flush_vdevs_done(zio_t *zio) zilog->zl_commit_lr_seq = zilog->zl_lr_seq; } + list_move_tail(&itxs, &lwb->lwb_itxs); + list_move_tail(&waiters, &lwb->lwb_waiters); + + ASSERT3S(lwb->lwb_state, ==, LWB_STATE_WRITE_DONE); + lwb->lwb_state = LWB_STATE_FLUSH_DONE; + mutex_exit(&zilog->zl_lock); - while ((itx = list_remove_head(&lwb->lwb_itxs)) != NULL) + while ((itx = list_remove_head(&itxs)) != NULL) zil_itx_destroy(itx); + list_destroy(&itxs); - while ((zcw = list_remove_head(&lwb->lwb_waiters)) != NULL) { + while ((zcw = list_remove_head(&waiters)) != NULL) { mutex_enter(&zcw->zcw_lock); - ASSERT3P(zcw->zcw_lwb, ==, lwb); zcw->zcw_lwb = NULL; /* * We expect any ZIO errors from child ZIOs to have been @@ -1199,6 +1208,7 @@ zil_lwb_flush_vdevs_done(zio_t *zio) mutex_exit(&zcw->zcw_lock); } + list_destroy(&waiters); mutex_enter(&zilog->zl_lwb_io_lock); txg = lwb->lwb_issued_txg; From 8cc159bf56519e1d2aeb11cd0bf67871eb162cbb Mon Sep 17 00:00:00 2001 From: Alexander Motin Date: Tue, 27 Jun 2023 20:03:37 -0400 Subject: [PATCH 11/15] ZIL: Fix another use-after-free. lwb->lwb_issued_txg can not be accessed after lwb_state is set to LWB_STATE_FLUSH_DONE and zl_lock is dropped, since the lwb may be freed by zil_sync(). We must save the txg number before that. This is similar to the 55b1842f92, but as I see the bug is not new. It existed for quite a while, just was not triggered due to smaller race window. Reviewed-by: Allan Jude Reviewed-by: Brian Atkinson Signed-off-by: Alexander Motin Sponsored by: iXsystems, Inc. Closes #14988 Closes #14999 (cherry picked from commit a9d6b0690b1863f39a7efce08b1227f2e9e26abb) --- module/zfs/zil.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/module/zfs/zil.c b/module/zfs/zil.c index 504dcaab1219..14b51e972eee 100644 --- a/module/zfs/zil.c +++ b/module/zfs/zil.c @@ -1171,6 +1171,7 @@ zil_lwb_flush_vdevs_done(zio_t *zio) list_move_tail(&itxs, &lwb->lwb_itxs); list_move_tail(&waiters, &lwb->lwb_waiters); + txg = lwb->lwb_issued_txg; ASSERT3S(lwb->lwb_state, ==, LWB_STATE_WRITE_DONE); lwb->lwb_state = LWB_STATE_FLUSH_DONE; @@ -1211,7 +1212,6 @@ zil_lwb_flush_vdevs_done(zio_t *zio) list_destroy(&waiters); mutex_enter(&zilog->zl_lwb_io_lock); - txg = lwb->lwb_issued_txg; ASSERT3U(zilog->zl_lwb_inflight[txg & TXG_MASK], >, 0); zilog->zl_lwb_inflight[txg & TXG_MASK]--; if (zilog->zl_lwb_inflight[txg & TXG_MASK] == 0) From 50834f7480cd781358b85876d8d3348903a68330 Mon Sep 17 00:00:00 2001 From: Alexander Motin Date: Fri, 30 Jun 2023 11:59:39 -0400 Subject: [PATCH 12/15] Again fix race between zil_commit() and zil_suspend(). With zl_suspend read in zil_commit() not protected by any locks it is possible for new ZIL writes to be in progress while zil_destroy() called by zil_suspend() freeing them. This patch closes the race by taking zl_issuer_lock in zil_suspend() and adding the second zl_suspend check to zil_get_commit_list(), protected by the lock. It allows all already queued transactions to be logged normally, while blocks any new ones, calling txg_wait_synced() for the TXGs. Reviewed-by: Brian Behlendorf Signed-off-by: Alexander Motin Sponsored by: iXsystems, Inc. Closes #14979 (cherry picked from commit 233425a153af74b7d5ef9730684f3a1d61ff8f11) --- module/zfs/zil.c | 36 ++++++++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/module/zfs/zil.c b/module/zfs/zil.c index 14b51e972eee..8fac4659c8ac 100644 --- a/module/zfs/zil.c +++ b/module/zfs/zil.c @@ -592,8 +592,8 @@ zil_free_lwb(zilog_t *zilog, lwb_t *lwb) { ASSERT(MUTEX_HELD(&zilog->zl_lock)); ASSERT(!MUTEX_HELD(&lwb->lwb_vdev_lock)); - ASSERT(list_is_empty(&lwb->lwb_waiters)); - ASSERT(list_is_empty(&lwb->lwb_itxs)); + VERIFY(list_is_empty(&lwb->lwb_waiters)); + VERIFY(list_is_empty(&lwb->lwb_itxs)); ASSERT(avl_is_empty(&lwb->lwb_vdev_tree)); ASSERT3P(lwb->lwb_write_zio, ==, NULL); ASSERT3P(lwb->lwb_root_zio, ==, NULL); @@ -2254,10 +2254,10 @@ zil_clean(zilog_t *zilog, uint64_t synced_txg) * This function will traverse the queue of itxs that need to be * committed, and move them onto the ZIL's zl_itx_commit_list. */ -static void +static uint64_t zil_get_commit_list(zilog_t *zilog) { - uint64_t otxg, txg; + uint64_t otxg, txg, wtxg = 0; list_t *commit_list = &zilog->zl_itx_commit_list; ASSERT(MUTEX_HELD(&zilog->zl_issuer_lock)); @@ -2291,10 +2291,22 @@ zil_get_commit_list(zilog_t *zilog) */ 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); + list_t *sync_list = &itxg->itxg_itxs->i_sync_list; + if (unlikely(zilog->zl_suspend > 0)) { + /* + * ZIL was just suspended, but we lost the race. + * Allow all earlier itxs to be committed, but ask + * caller to do txg_wait_synced(txg) for any new. + */ + if (!list_is_empty(sync_list)) + wtxg = MAX(wtxg, txg); + } else { + list_move_tail(commit_list, sync_list); + } mutex_exit(&itxg->itxg_lock); } + return (wtxg); } /* @@ -2677,11 +2689,12 @@ zil_process_commit_list(zilog_t *zilog, zil_commit_waiter_t *zcw, list_t *ilwbs) * not issued, we rely on future calls to zil_commit_writer() to issue * the lwb, or the timeout mechanism found in zil_commit_waiter(). */ -static void +static uint64_t zil_commit_writer(zilog_t *zilog, zil_commit_waiter_t *zcw) { list_t ilwbs; lwb_t *lwb; + uint64_t wtxg = 0; ASSERT(!MUTEX_HELD(&zilog->zl_lock)); ASSERT(spa_writeable(zilog->zl_spa)); @@ -2711,7 +2724,7 @@ zil_commit_writer(zilog_t *zilog, zil_commit_waiter_t *zcw) ZIL_STAT_BUMP(zil_commit_writer_count); - zil_get_commit_list(zilog); + wtxg = zil_get_commit_list(zilog); zil_prune_commit_list(zilog); zil_process_commit_list(zilog, zcw, &ilwbs); @@ -2720,6 +2733,7 @@ zil_commit_writer(zilog_t *zilog, zil_commit_waiter_t *zcw) while ((lwb = list_remove_head(&ilwbs)) != NULL) zil_lwb_write_issue(zilog, lwb); list_destroy(&ilwbs); + return (wtxg); } static void @@ -3228,7 +3242,7 @@ zil_commit_impl(zilog_t *zilog, uint64_t foid) zil_commit_waiter_t *zcw = zil_alloc_commit_waiter(); zil_commit_itx_assign(zilog, zcw); - zil_commit_writer(zilog, zcw); + uint64_t wtxg = zil_commit_writer(zilog, zcw); zil_commit_waiter(zilog, zcw); if (zcw->zcw_zio_error != 0) { @@ -3243,6 +3257,8 @@ zil_commit_impl(zilog_t *zilog, uint64_t foid) DTRACE_PROBE2(zil__commit__io__error, zilog_t *, zilog, zil_commit_waiter_t *, zcw); txg_wait_synced(zilog->zl_dmu_pool, 0); + } else if (wtxg != 0) { + txg_wait_synced(zilog->zl_dmu_pool, wtxg); } zil_free_commit_waiter(zcw); @@ -3606,11 +3622,13 @@ zil_suspend(const char *osname, void **cookiep) } zilog = dmu_objset_zil(os); + mutex_enter(&zilog->zl_issuer_lock); mutex_enter(&zilog->zl_lock); zh = zilog->zl_header; if (zh->zh_flags & ZIL_REPLAY_NEEDED) { /* unplayed log */ mutex_exit(&zilog->zl_lock); + mutex_exit(&zilog->zl_issuer_lock); dmu_objset_rele(os, suspend_tag); return (SET_ERROR(EBUSY)); } @@ -3624,6 +3642,7 @@ zil_suspend(const char *osname, void **cookiep) if (cookiep == NULL && !zilog->zl_suspending && (zilog->zl_suspend > 0 || BP_IS_HOLE(&zh->zh_log))) { mutex_exit(&zilog->zl_lock); + mutex_exit(&zilog->zl_issuer_lock); dmu_objset_rele(os, suspend_tag); TraceEvent(8, "%s:%d: Returning 0\n", __func__, __LINE__); return (0); @@ -3633,6 +3652,7 @@ zil_suspend(const char *osname, void **cookiep) dsl_pool_rele(dmu_objset_pool(os), suspend_tag); zilog->zl_suspend++; + mutex_exit(&zilog->zl_issuer_lock); if (zilog->zl_suspend > 1) { /* From d66ed134e25faab372e12d8e3054f868189aa94b Mon Sep 17 00:00:00 2001 From: Arun KV Date: Tue, 22 Sep 2026 11:17:37 +0530 Subject: [PATCH 13/15] Define zfs_fallthrough for Windows build 3a7c35119 (cherry-picked earlier in this branch) introduced zfs_fallthrough usage in zil_lwb_commit()'s error-handling switch, carried over from upstream's os/linux and os/freebsd compiler-compat headers. Neither this fork nor upstream's Windows port has an equivalent definition for MSVC/clang-cl, so it was left undefined. Upstream's own definition depends on HAVE_IMPLICIT_FALLTHROUGH, which is set by the Linux/FreeBSD autoconf build and has no equivalent in this CMake/MSBuild-based Windows build, so it would never be defined here either way. Add a minimal, MSVC-aware definition directly to the Windows feature_tests.h following the existing __NORETURN pattern in the same file: a plain no-op under _MSC_VER, matching upstream's own fallback path for compilers without the fallthrough attribute. --- lib/libspl/include/sys/feature_tests.h | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lib/libspl/include/sys/feature_tests.h b/lib/libspl/include/sys/feature_tests.h index a126de0fb863..f9843fd33cd3 100644 --- a/lib/libspl/include/sys/feature_tests.h +++ b/lib/libspl/include/sys/feature_tests.h @@ -33,4 +33,14 @@ #define __NORETURN __attribute__((__noreturn__)) #endif +#if !defined(zfs_fallthrough) +#if defined(_MSC_VER) +#define zfs_fallthrough ((void)0) +#elif defined(HAVE_IMPLICIT_FALLTHROUGH) +#define zfs_fallthrough __attribute__((__fallthrough__)) +#else +#define zfs_fallthrough ((void)0) +#endif +#endif + #endif From e202da507ca1e7ea9626c0d4713d8abae6c79704 Mon Sep 17 00:00:00 2001 From: Arun KV Date: Tue, 22 Sep 2026 11:30:53 +0530 Subject: [PATCH 14/15] Define zfs_fallthrough for the kernel driver build The earlier fix (feature_tests.h) only covers userland targets (libzpool, zpool, zfs) which include lib/libspl/include. The actual kernel driver (ZFSin/zfskern) resolves to include/os/windows/spl/sys/types.h instead -- a completely separate header tree that lib/libspl is never even in the include path for. zfs_fallthrough was still undefined there, which is what zfskern.lib was failing to compile against. --- include/os/windows/spl/sys/types.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/include/os/windows/spl/sys/types.h b/include/os/windows/spl/sys/types.h index db7b1fb545e8..8ce1bf26b816 100644 --- a/include/os/windows/spl/sys/types.h +++ b/include/os/windows/spl/sys/types.h @@ -27,6 +27,10 @@ #ifndef _SPL_TYPES_H #define _SPL_TYPES_H +#if !defined(zfs_fallthrough) +#define zfs_fallthrough ((void)0) +#endif + // use ntintsafe.h ? typedef enum { B_FALSE = 0, B_TRUE = 1 } boolean_t; typedef short pri_t; From 943e7f27912364dafe58d3244c27c41554d6b879 Mon Sep 17 00:00:00 2001 From: Alexander Motin Date: Tue, 9 May 2023 11:54:01 -0400 Subject: [PATCH 15/15] Remove single parent assertion from zio_nowait(). We only need to know if ZIO has any parent there. We do not care if it has more than one, but use of zio_unique_parent() == NULL asserts that. Reviewed-by: Brian Behlendorf Signed-off-by: Alexander Motin Sponsored by: iXsystems, Inc. Closes #14823 (cherry picked from commit b035f2b2cb9b88b1330c4b48641b8793d6460c9b) --- module/zfs/zio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/module/zfs/zio.c b/module/zfs/zio.c index 4cc3a2ef3584..7667acebbc15 100644 --- a/module/zfs/zio.c +++ b/module/zfs/zio.c @@ -2273,7 +2273,7 @@ zio_nowait(zio_t *zio) ASSERT3P(zio->io_executor, ==, NULL); if (zio->io_child_type == ZIO_CHILD_LOGICAL && - zio_unique_parent(zio) == NULL) { + list_is_empty(&zio->io_parent_list)) { zio_t *pio; /*