The branch main has been updated by mjg: URL: https://cgit.FreeBSD.org/src/commit/?id=b0695c6abe3f33a4188bcbcbf214823cd86f54d6
commit b0695c6abe3f33a4188bcbcbf214823cd86f54d6 Author: Mateusz Guzik <[email protected]> AuthorDate: 2023-04-25 16:01:22 +0000 Commit: Mateusz Guzik <[email protected]> CommitDate: 2023-04-25 16:01:22 +0000 zfs: Revert "Fix data race between zil_commit() and zil_suspend()" This reverts commit 4c856fb333ac57d9b4a6ddd44407fd022a702f00. To quote a pending upstream PR: This reverts commit 4c856fb to resolve a newly introduced deadlock which in practice is more disruptive that the issue this commit intended to address. Causes deadlocks described in https://github.com/openzfs/zfs/issues/14775 Sponsored by: Rubicon Communications, LLC ("Netgate") --- sys/contrib/openzfs/include/sys/zil_impl.h | 1 - sys/contrib/openzfs/module/zfs/zil.c | 27 --------------------------- 2 files changed, 28 deletions(-) diff --git a/sys/contrib/openzfs/include/sys/zil_impl.h b/sys/contrib/openzfs/include/sys/zil_impl.h index f44a01afee5c..bb85bf6d1eb1 100644 --- a/sys/contrib/openzfs/include/sys/zil_impl.h +++ b/sys/contrib/openzfs/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/sys/contrib/openzfs/module/zfs/zil.c b/sys/contrib/openzfs/module/zfs/zil.c index eb26e4b32998..d1631c2ac9db 100644 --- a/sys/contrib/openzfs/module/zfs/zil.c +++ b/sys/contrib/openzfs/module/zfs/zil.c @@ -3317,21 +3317,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 @@ -3340,14 +3325,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 @@ -3612,8 +3594,6 @@ zil_alloc(objset_t *os, zil_header_t *zh_phys) cv_init(&zilog->zl_cv_suspend, NULL, CV_DEFAULT, NULL); cv_init(&zilog->zl_lwb_io_cv, NULL, CV_DEFAULT, NULL); - rw_init(&zilog->zl_suspend_lock, NULL, RW_DEFAULT, NULL); - return (zilog); } @@ -3653,8 +3633,6 @@ zil_free(zilog_t *zilog) cv_destroy(&zilog->zl_cv_suspend); cv_destroy(&zilog->zl_lwb_io_cv); - rw_destroy(&zilog->zl_suspend_lock); - kmem_free(zilog, sizeof (zilog_t)); } @@ -3782,14 +3760,11 @@ zil_suspend(const char *osname, void **cookiep) return (error); 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)); } @@ -3803,7 +3778,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); return (0); } @@ -3812,7 +3786,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) { /*
