From nobody Tue Apr 25 16:03:27 2023 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4Q5Rcb6Fydz47MRR; Tue, 25 Apr 2023 16:03:27 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Q5Rcb5CBYz3p6b; Tue, 25 Apr 2023 16:03:27 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1682438607; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=SeuGWp4VJeUwKvT94cqz/yUijsVIcm6gWJGSzuhTVC0=; b=xJapVgTZC6eZ7gJF0BZE+QfawhrRqYfNHlwCK8qsnOEVWjBYNzsKfXvwVTwVcUorM6eXHW Tr8Wn1cEMSnZkny2lhQgKW97lOVwZk1REm8HgZAv2c80YrfMR6o7T2Gjq9B4UhA6K138Ni gVOTKU+9aeQfZz0Rufhj8rmX9npcTM5/mKhOsTqivvs6unqRRqlQHFH8uGr9LSl+Ck+JLr ZsqkC2nQF6yb4QP//9QfCT6UjGeIrEaprL86PU5PJ7ezkgBcJ+cDJ7Y3C11FMDBsOVOG42 1EyqfEImjNE5Ra9yFJPItR9575xXHFhGEqFZLbL9xXEXTnlxEVdkFxyFj1oveQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1682438607; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=SeuGWp4VJeUwKvT94cqz/yUijsVIcm6gWJGSzuhTVC0=; b=a3a95TBhKhWPsV/XRWuDnvOdtU91wkVIMTuDstnkjFX/3b7JpC2h1QkyZnpXjLe+OQVo6f WfQ1yTuqLX+MLizZIMNMkjQ7K/WI/Im13yNKDQLlQ4hGXjp9C91o51CO7RnLobZfPnXIAi j8LccGUrV9xJvUKpJUVnMoudEEPFWK10TRd7TLFw/21JJM7FHYZyQ0G9fhkGj3T6W69JNX r6gnB+5eT0HL2Qb69/MYkOFzLlTOkVd8iyRScP3GUY3ThAPi9PuXAAtsaofkqZ+EXRWLEo 3dO/82oiJlCqg6bbaS1/xJhTY9jOELw4Cnh04DdbBglhyvftlAjkry0KO92rdg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1682438607; a=rsa-sha256; cv=none; b=hjovq28D14Q49Tk+wEy0puKr/D6KxE4G5mGeeGj/Y4aSLJyxhXHtdIYgF8Z1AfIuf88rVL PLGUmQB5egnCYCO/diW6c39DZAuBaSLlkLYgDe+kGnCJlh0cc2ukOKK5J4drcn70XAtjnf rIme0ztmQTo+RUIwhT48gjKP38k9qKal6DlrUv0lyb6cChcGqkCd+8j4sO1j4JEV9TkWn+ MBTrcAXDeUVPGr4nCKTGIELQxlP0g6veqBMSYWpmpY1vMTRI54zLDOeupyVQt+tWHfqAzo o3EEuIgKpWjTp4TqtfnUgvNONNl7bnlNVia+LnNOA9DucegPRcZkdfZ212ewnw== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4Q5Rcb4FzJz1CvN; Tue, 25 Apr 2023 16:03:27 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 33PG3RXX095586; Tue, 25 Apr 2023 16:03:27 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 33PG3R7J095585; Tue, 25 Apr 2023 16:03:27 GMT (envelope-from git) Date: Tue, 25 Apr 2023 16:03:27 GMT Message-Id: <202304251603.33PG3R7J095585@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Mateusz Guzik Subject: git: b0695c6abe3f - main - zfs: Revert "Fix data race between zil_commit() and zil_suspend()" List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: mjg X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: b0695c6abe3f33a4188bcbcbf214823cd86f54d6 Auto-Submitted: auto-generated X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by mjg: URL: https://cgit.FreeBSD.org/src/commit/?id=b0695c6abe3f33a4188bcbcbf214823cd86f54d6 commit b0695c6abe3f33a4188bcbcbf214823cd86f54d6 Author: Mateusz Guzik AuthorDate: 2023-04-25 16:01:22 +0000 Commit: Mateusz Guzik 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) { /*