[PATCH] file-posix: fix cache.direct=on check for zoned block devices

Niklas Cassel posted 1 patch 1 month, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260616124535.1818951-1-cassel@kernel.org
Maintainers: Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>
There is a newer version of this series
block/file-posix.c | 22 ++++++++++------------
1 file changed, 10 insertions(+), 12 deletions(-)
[PATCH] file-posix: fix cache.direct=on check for zoned block devices
Posted by Niklas Cassel 1 month, 1 week ago
For zoned block devices with SWR zones, the order of the writes must be
maintained to avoid write not at write pointer errors.
Thus, cache.direct=on must be used to bypass the page cache, since the
page cache uses write-back caching, where the physical order of the writes
is not guaranteed.

There is already a check for cache.direct=on in raw_open_common(),
however, this check is done before raw_refresh_zoned_limits() has been
called (which initializes bs->bl.zoned), so it is currently dead code.

Fix this by moving the check to raw_refresh_zoned_limits(), such that the
check is done after bs->bl.zoned has been initialized.

Fixes: 774c726ceb2a ("block: add zoned BlockDriver check to block layer")
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 block/file-posix.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index 5019ddade9..2cdae2d40c 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -800,18 +800,6 @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
             goto fail;
         }
     }
-#ifdef CONFIG_BLKZONED
-    /*
-     * The kernel page cache does not reliably work for writes to SWR zones
-     * of zoned block device because it can not guarantee the order of writes.
-     */
-    if ((bs->bl.zoned != BLK_Z_NONE) &&
-        (!(s->open_flags & O_DIRECT))) {
-        error_setg(errp, "The driver supports zoned devices, and it requires "
-                         "cache.direct=on, which was not specified.");
-        return -EINVAL; /* No host kernel page cache */
-    }
-#endif
 
 #ifdef __FreeBSD__
     if (S_ISCHR(st.st_mode)) {
@@ -1462,6 +1450,16 @@ static void raw_refresh_zoned_limits(BlockDriverState *bs, struct stat *st,
     }
     bs->bl.zoned = zoned;
 
+    /*
+     * The kernel page cache does not reliably work for writes to SWR zones
+     * of zoned block device because it can not guarantee the order of writes.
+     */
+    if (!(s->open_flags & O_DIRECT)) {
+        error_setg(errp, "The driver supports zoned devices, and it requires "
+                         "cache.direct=on, which was not specified.");
+        goto no_zoned;
+    }
+
     ret = get_sysfs_long_val(st, "max_open_zones");
     if (ret >= 0) {
         bs->bl.max_open_zones = ret;
-- 
2.54.0
Re: [PATCH] file-posix: fix cache.direct=on check for zoned block devices
Posted by Dmitry Fomichev 1 month, 1 week ago
Looks good.

Reviewed-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>

________________________________
From: Niklas Cassel <cassel@kernel.org>
Sent: Tuesday, June 16, 2026 8:45 AM
To: Kevin Wolf <kwolf@redhat.com>; Hanna Reitz <hreitz@redhat.com>
Cc: qemu-block@nongnu.org <qemu-block@nongnu.org>; qemu-devel@nongnu.org <qemu-devel@nongnu.org>; Sam Li <faithilikerun@gmail.com>; Dmitry Fomichev <Dmitry.Fomichev@wdc.com>; Stefan Hajnoczi <stefanha@redhat.com>; Damien Le Moal <dlemoal@kernel.org>; Johannes Thumshirn <Johannes.Thumshirn@wdc.com>; Niklas Cassel <cassel@kernel.org>
Subject: [PATCH] file-posix: fix cache.direct=on check for zoned block devices

For zoned block devices with SWR zones, the order of the writes must be
maintained to avoid write not at write pointer errors.
Thus, cache.direct=on must be used to bypass the page cache, since the
page cache uses write-back caching, where the physical order of the writes
is not guaranteed.

There is already a check for cache.direct=on in raw_open_common(),
however, this check is done before raw_refresh_zoned_limits() has been
called (which initializes bs->bl.zoned), so it is currently dead code.

Fix this by moving the check to raw_refresh_zoned_limits(), such that the
check is done after bs->bl.zoned has been initialized.

Fixes: 774c726ceb2a ("block: add zoned BlockDriver check to block layer")
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 block/file-posix.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index 5019ddade9..2cdae2d40c 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -800,18 +800,6 @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
             goto fail;
         }
     }
-#ifdef CONFIG_BLKZONED
-    /*
-     * The kernel page cache does not reliably work for writes to SWR zones
-     * of zoned block device because it can not guarantee the order of writes.
-     */
-    if ((bs->bl.zoned != BLK_Z_NONE) &&
-        (!(s->open_flags & O_DIRECT))) {
-        error_setg(errp, "The driver supports zoned devices, and it requires "
-                         "cache.direct=on, which was not specified.");
-        return -EINVAL; /* No host kernel page cache */
-    }
-#endif

 #ifdef __FreeBSD__
     if (S_ISCHR(st.st_mode)) {
@@ -1462,6 +1450,16 @@ static void raw_refresh_zoned_limits(BlockDriverState *bs, struct stat *st,
     }
     bs->bl.zoned = zoned;

+    /*
+     * The kernel page cache does not reliably work for writes to SWR zones
+     * of zoned block device because it can not guarantee the order of writes.
+     */
+    if (!(s->open_flags & O_DIRECT)) {
+        error_setg(errp, "The driver supports zoned devices, and it requires "
+                         "cache.direct=on, which was not specified.");
+        goto no_zoned;
+    }
+
     ret = get_sysfs_long_val(st, "max_open_zones");
     if (ret >= 0) {
         bs->bl.max_open_zones = ret;
--
2.54.0

Re: [PATCH] file-posix: fix cache.direct=on check for zoned block devices
Posted by Damien Le Moal 1 month, 1 week ago
On 6/16/26 9:45 PM, Niklas Cassel wrote:
> For zoned block devices with SWR zones, the order of the writes must be
> maintained to avoid write not at write pointer errors.
> Thus, cache.direct=on must be used to bypass the page cache, since the
> page cache uses write-back caching, where the physical order of the writes
> is not guaranteed.
> 
> There is already a check for cache.direct=on in raw_open_common(),
> however, this check is done before raw_refresh_zoned_limits() has been
> called (which initializes bs->bl.zoned), so it is currently dead code.
> 
> Fix this by moving the check to raw_refresh_zoned_limits(), such that the
> check is done after bs->bl.zoned has been initialized.
> 
> Fixes: 774c726ceb2a ("block: add zoned BlockDriver check to block layer")
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
>  block/file-posix.c | 22 ++++++++++------------
>  1 file changed, 10 insertions(+), 12 deletions(-)
> 
> diff --git a/block/file-posix.c b/block/file-posix.c
> index 5019ddade9..2cdae2d40c 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -800,18 +800,6 @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
>              goto fail;
>          }
>      }
> -#ifdef CONFIG_BLKZONED
> -    /*
> -     * The kernel page cache does not reliably work for writes to SWR zones
> -     * of zoned block device because it can not guarantee the order of writes.
> -     */
> -    if ((bs->bl.zoned != BLK_Z_NONE) &&
> -        (!(s->open_flags & O_DIRECT))) {
> -        error_setg(errp, "The driver supports zoned devices, and it requires "
> -                         "cache.direct=on, which was not specified.");
> -        return -EINVAL; /* No host kernel page cache */
> -    }
> -#endif
>  
>  #ifdef __FreeBSD__
>      if (S_ISCHR(st.st_mode)) {
> @@ -1462,6 +1450,16 @@ static void raw_refresh_zoned_limits(BlockDriverState *bs, struct stat *st,
>      }
>      bs->bl.zoned = zoned;
>  
> +    /*
> +     * The kernel page cache does not reliably work for writes to SWR zones
> +     * of zoned block device because it can not guarantee the order of writes.

s/device/devices

Apart from that, looks OK to me.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

> +     */
> +    if (!(s->open_flags & O_DIRECT)) {
> +        error_setg(errp, "The driver supports zoned devices, and it requires "
> +                         "cache.direct=on, which was not specified.");
> +        goto no_zoned;
> +    }
> +
>      ret = get_sysfs_long_val(st, "max_open_zones");
>      if (ret >= 0) {
>          bs->bl.max_open_zones = ret;


-- 
Damien Le Moal
Western Digital Research