git: b0695c6abe3f - main - zfs: Revert "Fix data race between zil_commit() and zil_suspend()"
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Tue, 25 Apr 2023 16:03:27 UTC
The branch main has been updated by mjg:
URL: https://cgit.FreeBSD.org/src/commit/?id=b0695c6abe3f33a4188bcbcbf214823cd86f54d6
commit b0695c6abe3f33a4188bcbcbf214823cd86f54d6
Author: Mateusz Guzik <mjg@FreeBSD.org>
AuthorDate: 2023-04-25 16:01:22 +0000
Commit: Mateusz Guzik <mjg@FreeBSD.org>
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) {
/*