From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] superseded: [PATCH zfsonlinux] patches: fix for zvol sync/flush regression
Date: Thu, 13 Mar 2025 12:33:05 +0100 [thread overview]
Message-ID: <1741865510.d2dddmr4xn.astroid@yuna.none> (raw)
In-Reply-To: <20250311084403.130464-1-f.gruenbichler@proxmox.com>
v2 with slightly updated patch from upstream master:
https://lore.proxmox.com/pve-devel/20250313113214.23456-1-f.gruenbichler@proxmox.com/T/#u
On March 11, 2025 9:44 am, Fabian Grünbichler wrote:
> this broke with 2.2.7, and can potentially cause data loss or
> inconsistency. the patch basically reverts to pre-2.2.7 behaviour,
> verified via a fio benchmark.
>
> reported on our forum: https://forum.proxmox.com/threads/163066
>
> upstream PR: https://github.com/openzfs/zfs/pull/17131
>
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
> module only change, needs kernel bump to be active
>
> ...vols-correctly-detect-flush-requests.patch | 58 +++++++++++++++++++
> debian/patches/series | 1 +
> 2 files changed, 59 insertions(+)
> create mode 100644 debian/patches/0012-linux-zvols-correctly-detect-flush-requests.patch
>
> diff --git a/debian/patches/0012-linux-zvols-correctly-detect-flush-requests.patch b/debian/patches/0012-linux-zvols-correctly-detect-flush-requests.patch
> new file mode 100644
> index 0000000..609934a
> --- /dev/null
> +++ b/debian/patches/0012-linux-zvols-correctly-detect-flush-requests.patch
> @@ -0,0 +1,58 @@
> +From c9ae14e064f961709557fdcbdb40e692a97225ec Mon Sep 17 00:00:00 2001
> +From: =?UTF-8?q?Fabian=20Gr=C3=BCnbichler?= <f.gruenbichler@proxmox.com>
> +Date: Mon, 10 Mar 2025 15:51:09 +0100
> +Subject: [PATCH] linux: zvols: correctly detect flush requests
> +MIME-Version: 1.0
> +Content-Type: text/plain; charset=UTF-8
> +Content-Transfer-Encoding: 8bit
> +
> +since 4.10, bio->bi_opf needs to be checked to determine all kinds of
> +flush requests. this was the case prior to the commit referenced below,
> +but the order of ifdefs was not the usual one (newest up top), which
> +might have caused this to slip through.
> +
> +this fixes a regression when using zvols as Qemu block devices, but
> +might have broken other use cases as well. the symptoms are that all
> +sync writes from within a VM configured to use such a virtual block
> +devices are ignored and treated as async writes by the host ZFS layer.
> +
> +this can be verified using fio in sync mode inside the VM, for example
> +with
> +
> + fio \
> + --filename=/dev/sda --ioengine=libaio --loops=1 --size=10G \
> + --time_based --runtime=60 --group_reporting --stonewall --name=cc1 \
> + --description="CC1" --rw=write --bs=4k --direct=1 --iodepth=1 \
> + --numjobs=1 --sync=1
> +
> +which shows an IOPS number way above what the physical device underneath
> +supports, with "zpool iostat -r 1" on the hypervisor side showing no
> +sync IO occuring during the benchmark.
> +
> +with the regression fixed, both fio inside the VM and the IO stats on
> +the host show the expected numbers.
> +
> +Fixes: 846b5985192467acabf5484ae610b4b37b7f8162
> +"config: remove HAVE_REQ_OP_* and HAVE_REQ_*"
> +
> +Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> +---
> + include/os/linux/kernel/linux/blkdev_compat.h | 2 +-
> + 1 file changed, 1 insertion(+), 1 deletion(-)
> +
> +diff --git a/include/os/linux/kernel/linux/blkdev_compat.h b/include/os/linux/kernel/linux/blkdev_compat.h
> +index d96708c60..084869c34 100644
> +--- a/include/os/linux/kernel/linux/blkdev_compat.h
> ++++ b/include/os/linux/kernel/linux/blkdev_compat.h
> +@@ -383,7 +383,7 @@ bio_set_flush(struct bio *bio)
> + static inline boolean_t
> + bio_is_flush(struct bio *bio)
> + {
> +- return (bio_op(bio) == REQ_OP_FLUSH);
> ++ return (bio_op(bio) == REQ_OP_FLUSH) || (op_is_flush(bio->bi_opf));
> + }
> +
> + /*
> +--
> +2.39.5
> +
> diff --git a/debian/patches/series b/debian/patches/series
> index 229027f..938488c 100644
> --- a/debian/patches/series
> +++ b/debian/patches/series
> @@ -9,3 +9,4 @@
> 0009-arc-stat-summary-guard-access-to-freshly-introduced-.patch
> 0010-Fix-nfs_truncate_shares-without-etc-exports.d.patch
> 0011-zpool-status-tighten-bounds-for-noalloc-stat-availab.patch
> +0012-linux-zvols-correctly-detect-flush-requests.patch
> --
> 2.39.5
>
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
prev parent reply other threads:[~2025-03-13 11:33 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-03-11 8:44 [pve-devel] " Fabian Grünbichler
2025-03-11 13:44 ` Stoiko Ivanov
2025-03-13 11:33 ` Fabian Grünbichler [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1741865510.d2dddmr4xn.astroid@yuna.none \
--to=f.gruenbichler@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.