[pve-devel] [PATCH zfsonlinux/stable-6] cherry-pick fix for assertion from 2.0.6-staging

[pve-devel] [PATCH zfsonlinux/stable-6] cherry-pick fix for assertion from 2.0.6-staging

[Stoiko Ivanov]

The panic described in the issues closed by this commit were observed
by some users.

Since the revert is already included in upstreams master and 2.1
branches, and the proposed port to the 2.0 branches is already
accepted the limit for regression should be small.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
Tested by building pve-kernel-5.4 with zfs including this patch and
booting it on 2 VMs of mine with replication enabled.

 ...onsolidate-arc_buf-allocation-checks.patch | 181 ++++++++++++++++++
 debian/patches/series                         |   1 +
 2 files changed, 182 insertions(+)
 create mode 100644 debian/patches/0010-Revert-Consolidate-arc_buf-allocation-checks.patch

diff --git a/debian/patches/0010-Revert-Consolidate-arc_buf-allocation-checks.patch b/debian/patches/0010-Revert-Consolidate-arc_buf-allocation-checks.patch
new file mode 100644
index 00000000..b64da7e5
--- /dev/null
+++ b/debian/patches/0010-Revert-Consolidate-arc_buf-allocation-checks.patch
@@ -0,0 +1,181 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Antonio Russo <aerusso@aerusso.net>
+Date: Tue, 22 Jun 2021 22:39:15 -0600
+Subject: [PATCH] Revert Consolidate arc_buf allocation checks
+
+This reverts commit 13fac09868b4e4e08cc3ef7b937ac277c1c407b1.
+
+Per the discussion in #11531, the reverted commit---which intended only
+to be a cleanup commit---introduced a subtle, unintended change in
+behavior.
+
+Care was taken to partially revert and then reapply 10b3c7f5e4
+which would otherwise have caused a conflict.  These changes were
+squashed in to this commit.
+
+Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
+Suggested-by: @chrisrd
+Suggested-by: robn@despairlabs.com
+Signed-off-by: Antonio Russo <aerusso@aerusso.net>
+Closes #11531
+Closes #12227
+(cherry picked from commit c01032417f655555b29f4e67c187e0622d587c8d)
+Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
+---
+ module/zfs/dbuf.c | 121 +++++++++++++++++++++++++++++-----------------
+ 1 file changed, 77 insertions(+), 44 deletions(-)
+
+diff --git a/module/zfs/dbuf.c b/module/zfs/dbuf.c
+index 7d817320a..844f69367 100644
+--- a/module/zfs/dbuf.c
++++ b/module/zfs/dbuf.c
+@@ -1090,42 +1090,6 @@ dbuf_set_data(dmu_buf_impl_t *db, arc_buf_t *buf)
+ 	db->db.db_data = buf->b_data;
+ }
+ 
+-static arc_buf_t *
+-dbuf_alloc_arcbuf_from_arcbuf(dmu_buf_impl_t *db, arc_buf_t *data)
+-{
+-	objset_t *os = db->db_objset;
+-	spa_t *spa = os->os_spa;
+-	arc_buf_contents_t type = DBUF_GET_BUFC_TYPE(db);
+-	enum zio_compress compress_type;
+-	uint8_t complevel;
+-	int psize, lsize;
+-
+-	psize = arc_buf_size(data);
+-	lsize = arc_buf_lsize(data);
+-	compress_type = arc_get_compression(data);
+-	complevel = arc_get_complevel(data);
+-
+-	if (arc_is_encrypted(data)) {
+-		boolean_t byteorder;
+-		uint8_t salt[ZIO_DATA_SALT_LEN];
+-		uint8_t iv[ZIO_DATA_IV_LEN];
+-		uint8_t mac[ZIO_DATA_MAC_LEN];
+-		dnode_t *dn = DB_DNODE(db);
+-
+-		arc_get_raw_params(data, &byteorder, salt, iv, mac);
+-		data = arc_alloc_raw_buf(spa, db, dmu_objset_id(os),
+-		    byteorder, salt, iv, mac, dn->dn_type, psize, lsize,
+-		    compress_type, complevel);
+-	} else if (compress_type != ZIO_COMPRESS_OFF) {
+-		ASSERT3U(type, ==, ARC_BUFC_DATA);
+-		data = arc_alloc_compressed_buf(spa, db,
+-		    psize, lsize, compress_type, complevel);
+-	} else {
+-		data = arc_alloc_buf(spa, db, type, psize);
+-	}
+-	return (data);
+-}
+-
+ static arc_buf_t *
+ dbuf_alloc_arcbuf(dmu_buf_impl_t *db)
+ {
+@@ -1575,9 +1539,35 @@ dbuf_fix_old_data(dmu_buf_impl_t *db, uint64_t txg)
+ 		arc_space_consume(bonuslen, ARC_SPACE_BONUS);
+ 		bcopy(db->db.db_data, dr->dt.dl.dr_data, bonuslen);
+ 	} else if (zfs_refcount_count(&db->db_holds) > db->db_dirtycnt) {
+-		arc_buf_t *buf = dbuf_alloc_arcbuf_from_arcbuf(db, db->db_buf);
+-		dr->dt.dl.dr_data = buf;
+-		bcopy(db->db.db_data, buf->b_data, arc_buf_size(buf));
++		dnode_t *dn = DB_DNODE(db);
++		int size = arc_buf_size(db->db_buf);
++		arc_buf_contents_t type = DBUF_GET_BUFC_TYPE(db);
++		spa_t *spa = db->db_objset->os_spa;
++		enum zio_compress compress_type =
++		    arc_get_compression(db->db_buf);
++		uint8_t complevel = arc_get_complevel(db->db_buf);
++
++		if (arc_is_encrypted(db->db_buf)) {
++			boolean_t byteorder;
++			uint8_t salt[ZIO_DATA_SALT_LEN];
++			uint8_t iv[ZIO_DATA_IV_LEN];
++			uint8_t mac[ZIO_DATA_MAC_LEN];
++
++			arc_get_raw_params(db->db_buf, &byteorder, salt,
++			    iv, mac);
++			dr->dt.dl.dr_data = arc_alloc_raw_buf(spa, db,
++			    dmu_objset_id(dn->dn_objset), byteorder, salt, iv,
++			    mac, dn->dn_type, size, arc_buf_lsize(db->db_buf),
++			    compress_type, complevel);
++		} else if (compress_type != ZIO_COMPRESS_OFF) {
++			ASSERT3U(type, ==, ARC_BUFC_DATA);
++			dr->dt.dl.dr_data = arc_alloc_compressed_buf(spa, db,
++			    size, arc_buf_lsize(db->db_buf), compress_type,
++			    complevel);
++		} else {
++			dr->dt.dl.dr_data = arc_alloc_buf(spa, db, type, size);
++		}
++		bcopy(db->db.db_data, dr->dt.dl.dr_data->b_data, size);
+ 	} else {
+ 		db->db_buf = NULL;
+ 		dbuf_clear_data(db);
+@@ -3280,10 +3270,30 @@ noinline static void
+ dbuf_hold_copy(dnode_t *dn, dmu_buf_impl_t *db)
+ {
+ 	dbuf_dirty_record_t *dr = db->db_data_pending;
+-	arc_buf_t *newdata, *data = dr->dt.dl.dr_data;
++	arc_buf_t *data = dr->dt.dl.dr_data;
++	enum zio_compress compress_type = arc_get_compression(data);
++	uint8_t complevel = arc_get_complevel(data);
++
++	if (arc_is_encrypted(data)) {
++		boolean_t byteorder;
++		uint8_t salt[ZIO_DATA_SALT_LEN];
++		uint8_t iv[ZIO_DATA_IV_LEN];
++		uint8_t mac[ZIO_DATA_MAC_LEN];
++
++		arc_get_raw_params(data, &byteorder, salt, iv, mac);
++		dbuf_set_data(db, arc_alloc_raw_buf(dn->dn_objset->os_spa, db,
++		    dmu_objset_id(dn->dn_objset), byteorder, salt, iv, mac,
++		    dn->dn_type, arc_buf_size(data), arc_buf_lsize(data),
++		    compress_type, complevel));
++	} else if (compress_type != ZIO_COMPRESS_OFF) {
++		dbuf_set_data(db, arc_alloc_compressed_buf(
++		    dn->dn_objset->os_spa, db, arc_buf_size(data),
++		    arc_buf_lsize(data), compress_type, complevel));
++	} else {
++		dbuf_set_data(db, arc_alloc_buf(dn->dn_objset->os_spa, db,
++		    DBUF_GET_BUFC_TYPE(db), db->db.db_size));
++	}
+ 
+-	newdata = dbuf_alloc_arcbuf_from_arcbuf(db, data);
+-	dbuf_set_data(db, newdata);
+ 	rw_enter(&db->db_rwlock, RW_WRITER);
+ 	bcopy(data->b_data, db->db.db_data, arc_buf_size(data));
+ 	rw_exit(&db->db_rwlock);
+@@ -4067,8 +4077,31 @@ dbuf_sync_leaf(dbuf_dirty_record_t *dr, dmu_tx_t *tx)
+ 		 * objects only modified in the syncing context (e.g.
+ 		 * DNONE_DNODE blocks).
+ 		 */
+-		*datap = dbuf_alloc_arcbuf_from_arcbuf(db, db->db_buf);
+-		bcopy(db->db.db_data, (*datap)->b_data, arc_buf_size(*datap));
++		int psize = arc_buf_size(*datap);
++		int lsize = arc_buf_lsize(*datap);
++		arc_buf_contents_t type = DBUF_GET_BUFC_TYPE(db);
++		enum zio_compress compress_type = arc_get_compression(*datap);
++		uint8_t complevel = arc_get_complevel(*datap);
++
++		if (arc_is_encrypted(*datap)) {
++			boolean_t byteorder;
++			uint8_t salt[ZIO_DATA_SALT_LEN];
++			uint8_t iv[ZIO_DATA_IV_LEN];
++			uint8_t mac[ZIO_DATA_MAC_LEN];
++
++			arc_get_raw_params(*datap, &byteorder, salt, iv, mac);
++			*datap = arc_alloc_raw_buf(os->os_spa, db,
++			    dmu_objset_id(os), byteorder, salt, iv, mac,
++			    dn->dn_type, psize, lsize, compress_type,
++			    complevel);
++		} else if (compress_type != ZIO_COMPRESS_OFF) {
++			ASSERT3U(type, ==, ARC_BUFC_DATA);
++			*datap = arc_alloc_compressed_buf(os->os_spa, db,
++			    psize, lsize, compress_type, complevel);
++		} else {
++			*datap = arc_alloc_buf(os->os_spa, db, type, psize);
++		}
++		bcopy(db->db.db_data, (*datap)->b_data, psize);
+ 	}
+ 	db->db_data_pending = dr;
+ 
diff --git a/debian/patches/series b/debian/patches/series
index 91b8a3b1..76636a04 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -7,3 +7,4 @@
 0007-Use-installed-python3.patch
 0008-Add-systemd-unit-for-importing-specific-pools.patch
 0009-Patch-move-manpage-arcstat-1-to-arcstat-8.patch
+0010-Revert-Consolidate-arc_buf-allocation-checks.patch
-- 
2.30.2

Re: [pve-devel] [PATCH zfsonlinux/stable-6] cherry-pick fix for assertion from 2.0.6-staging

[Thomas Lamprecht]

On 08.09.21 16:22, Stoiko Ivanov wrote:
> The panic described in the issues closed by this commit were observed
> by some users.
> 
> Since the revert is already included in upstreams master and 2.1
> branches, and the proposed port to the 2.0 branches is already
> accepted the limit for regression should be small.
> 
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
> Tested by building pve-kernel-5.4 with zfs including this patch and
> booting it on 2 VMs of mine with replication enabled.
> 
>  ...onsolidate-arc_buf-allocation-checks.patch | 181 ++++++++++++++++++
>  debian/patches/series                         |   1 +
>  2 files changed, 182 insertions(+)
>  create mode 100644 debian/patches/0010-Revert-Consolidate-arc_buf-allocation-checks.patch
> 

as told off-list, I did the thankfully trivial cherry-pick at that time yesterday already
and also triggered a kernel build including an update to latest stable tag, so I rather
pushed that out.