Hi
When running the lvm testsuite, we get a lot of warnings
"blk_update_request: operation not supported error, dev loop0, sector 0 op
0x9:(WRITE_ZEROES) flags 0x800800 phys_seg 0 prio class 0". The lvm
testsuite puts the loop device on tmpfs and the reason for the warning is
that tmpfs supports fallocate, but doesn't support FALLOC_FL_ZERO_RANGE.
I've created this patch to silence the warnings.
Mikulas
From: Mikulas Patocka <mpatocka@redhat.com>
The loop driver checks for the fallocate method and if it is present, it
assumes that the filesystem can do FALLOC_FL_ZERO_RANGE and
FALLOC_FL_PUNCH_HOLE requests. However, some filesystems (such as fat, or
tmpfs) have the fallocate method, but lack the capability to do
FALLOC_FL_ZERO_RANGE and/or FALLOC_FL_PUNCH_HOLE.
This results in syslog warnings "blk_update_request: operation not
supported error, dev loop0, sector 0 op 0x9:(WRITE_ZEROES) flags 0x800800
phys_seg 0 prio class 0"
This patch sets RQF_QUIET to silence the warnings.
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Cc: stable@vger.kernel.org
---
drivers/block/loop.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
Index: linux-2.6/drivers/block/loop.c
===================================================================
On Thu, Sep 23, 2021 at 03:48:27PM -0400, Mikulas Patocka wrote:
quoted hunk
Hi
When running the lvm testsuite, we get a lot of warnings
"blk_update_request: operation not supported error, dev loop0, sector 0 op
0x9:(WRITE_ZEROES) flags 0x800800 phys_seg 0 prio class 0". The lvm
testsuite puts the loop device on tmpfs and the reason for the warning is
that tmpfs supports fallocate, but doesn't support FALLOC_FL_ZERO_RANGE.
I've created this patch to silence the warnings.
Mikulas
From: Mikulas Patocka <mpatocka@redhat.com>
The loop driver checks for the fallocate method and if it is present, it
assumes that the filesystem can do FALLOC_FL_ZERO_RANGE and
FALLOC_FL_PUNCH_HOLE requests. However, some filesystems (such as fat, or
tmpfs) have the fallocate method, but lack the capability to do
FALLOC_FL_ZERO_RANGE and/or FALLOC_FL_PUNCH_HOLE.
This results in syslog warnings "blk_update_request: operation not
supported error, dev loop0, sector 0 op 0x9:(WRITE_ZEROES) flags 0x800800
phys_seg 0 prio class 0"
This patch sets RQF_QUIET to silence the warnings.
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Cc: stable@vger.kernel.org
---
drivers/block/loop.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
Index: linux-2.6/drivers/block/loop.c
===================================================================
From: Christoph Hellwig <hch@lst.de> Date: 2021-09-24 15:58:26
On Thu, Sep 23, 2021 at 03:48:27PM -0400, Mikulas Patocka wrote:
Hi
When running the lvm testsuite, we get a lot of warnings
"blk_update_request: operation not supported error, dev loop0, sector 0 op
0x9:(WRITE_ZEROES) flags 0x800800 phys_seg 0 prio class 0". The lvm
testsuite puts the loop device on tmpfs and the reason for the warning is
that tmpfs supports fallocate, but doesn't support FALLOC_FL_ZERO_RANGE.
I've created this patch to silence the warnings.
Mikulas
From: Mikulas Patocka <mpatocka@redhat.com>
The loop driver checks for the fallocate method and if it is present, it
assumes that the filesystem can do FALLOC_FL_ZERO_RANGE and
FALLOC_FL_PUNCH_HOLE requests. However, some filesystems (such as fat, or
tmpfs) have the fallocate method, but lack the capability to do
FALLOC_FL_ZERO_RANGE and/or FALLOC_FL_PUNCH_HOLE.
This results in syslog warnings "blk_update_request: operation not
supported error, dev loop0, sector 0 op 0x9:(WRITE_ZEROES) flags 0x800800
phys_seg 0 prio class 0"
This patch sets RQF_QUIET to silence the warnings.
Doesn't this just paper over the problem? I think we need an
unsigned int with flag in the file_operations for the supported
operations for this kind of use.
On Thu, Sep 23, 2021 at 03:48:27PM -0400, Mikulas Patocka wrote:
quoted
Hi
When running the lvm testsuite, we get a lot of warnings
"blk_update_request: operation not supported error, dev loop0, sector 0 op
0x9:(WRITE_ZEROES) flags 0x800800 phys_seg 0 prio class 0". The lvm
testsuite puts the loop device on tmpfs and the reason for the warning is
that tmpfs supports fallocate, but doesn't support FALLOC_FL_ZERO_RANGE.
I've created this patch to silence the warnings.
Mikulas
From: Mikulas Patocka <mpatocka@redhat.com>
The loop driver checks for the fallocate method and if it is present, it
assumes that the filesystem can do FALLOC_FL_ZERO_RANGE and
FALLOC_FL_PUNCH_HOLE requests. However, some filesystems (such as fat, or
tmpfs) have the fallocate method, but lack the capability to do
FALLOC_FL_ZERO_RANGE and/or FALLOC_FL_PUNCH_HOLE.
This results in syslog warnings "blk_update_request: operation not
supported error, dev loop0, sector 0 op 0x9:(WRITE_ZEROES) flags 0x800800
phys_seg 0 prio class 0"
This patch sets RQF_QUIET to silence the warnings.
Doesn't this just paper over the problem? I think we need an
unsigned int with flag in the file_operations for the supported
operations for this kind of use.
Do you want this patch?
Mikulas
From: Mikulas Patocka <mpatocka@redhat.com>
The loop driver checks for the fallocate method and if it is present, it
assumes that the filesystem can do FALLOC_FL_ZERO_RANGE and
FALLOC_FL_PUNCH_HOLE requests. However, some filesystems (such as fat, or
tmpfs) have the fallocate method, but lack the capability to do
FALLOC_FL_ZERO_RANGE and/or FALLOC_FL_PUNCH_HOLE.
This results in syslog warnings "blk_update_request: operation not
supported error, dev loop0, sector 0 op 0x9:(WRITE_ZEROES) flags 0x800800
phys_seg 0 prio class 0". The error can be reproduced with this command:
"truncate -s 1GiB /tmp/file; losetup /dev/loop0 /tmp/file; blkdiscard -z /dev/loop0"
This patch introduces a field "fallocate_flags" in struct file_operations
that specifies the flags that are supported by the fallocate methods. The
loopback driver will check this field to determine if FALLOC_FL_PUNCH_HOLE
or FALLOC_FL_ZERO_RANGE is supported
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Index: linux-2.6/block/fops.c
===================================================================
Please avoid over 80 lines for a plain list of flags.
OK. Here I'm sending a new version of the patch.
BTW. for some filesystems (cifs, ext4, fuse, ...), the supported falloc
flags vary dynamically - for example, ext4 can do COLLAPSE_RANGE and
INSERT_RANGE operations only if the filesystem is not ext2 or ext3 and if
the file is not encrypted.
Should we add a new flag FALLOC_FL_RETURN_SUPORTED_FLAGS that will return
the supported flags instead of using a static field in the file_operations
structure?
Mikulas
From: Mikulas Patocka <mpatocka@redhat.com>
The loop driver checks for the fallocate method and if it is present, it
assumes that the filesystem can do FALLOC_FL_ZERO_RANGE and
FALLOC_FL_PUNCH_HOLE requests. However, some filesystems (such as fat, or
tmpfs) have the fallocate method, but lack the capability to do
FALLOC_FL_ZERO_RANGE and/or FALLOC_FL_PUNCH_HOLE.
This results in syslog warnings "blk_update_request: operation not
supported error, dev loop0, sector 0 op 0x9:(WRITE_ZEROES) flags 0x800800
phys_seg 0 prio class 0". The error can be reproduced with this command:
"truncate -s 1GiB /tmp/file; losetup /dev/loop0 /tmp/file; blkdiscard -z
/dev/loop0"
This patch introduces a field "fallocate_supported_flags" in struct
file_operations that specifies the flags that are supported by the
fallocate methods. The loopback driver will check this field to determine
if FALLOC_FL_PUNCH_HOLE or FALLOC_FL_ZERO_RANGE is supported
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Index: linux-2.6/block/fops.c
===================================================================
Hi
Here I'm sending version 4 of the patch. It adds #include <linux/falloc.h>
to cifs and overlayfs to fix the bugs found out by the kernel test robot.
Mikulas
From: Mikulas Patocka <mpatocka@redhat.com>
The loop driver checks for the fallocate method and if it is present, it
assumes that the filesystem can do FALLOC_FL_ZERO_RANGE and
FALLOC_FL_PUNCH_HOLE requests. However, some filesystems (such as fat, or
tmpfs) have the fallocate method, but lack the capability to do
FALLOC_FL_ZERO_RANGE and/or FALLOC_FL_PUNCH_HOLE.
This results in syslog warnings "blk_update_request: operation not
supported error, dev loop0, sector 0 op 0x9:(WRITE_ZEROES) flags 0x800800
phys_seg 0 prio class 0". The error can be reproduced with this command:
"truncate -s 1GiB /tmp/file; losetup /dev/loop0 /tmp/file; blkdiscard -z
/dev/loop0"
This patch introduces a field "fallocate_supported_flags" in struct
file_operations that specifies the flags that are supported by the
fallocate methods. The loopback driver will check this field to determine
if FALLOC_FL_PUNCH_HOLE or FALLOC_FL_ZERO_RANGE is supported
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Index: linux-2.6/block/fops.c
===================================================================
From: Dave Chinner <david@fromorbit.com> Date: 2021-10-27 05:02:56
On Wed, Oct 13, 2021 at 05:28:36AM -0400, Mikulas Patocka wrote:
Hi
Here I'm sending version 4 of the patch. It adds #include <linux/falloc.h>
to cifs and overlayfs to fix the bugs found out by the kernel test robot.
Mikulas
From: Mikulas Patocka <mpatocka@redhat.com>
The loop driver checks for the fallocate method and if it is present, it
assumes that the filesystem can do FALLOC_FL_ZERO_RANGE and
FALLOC_FL_PUNCH_HOLE requests. However, some filesystems (such as fat, or
tmpfs) have the fallocate method, but lack the capability to do
FALLOC_FL_ZERO_RANGE and/or FALLOC_FL_PUNCH_HOLE.
This seems like a loopback driver level problem, not something
filesystems need to solve. fallocate() is defined to return
-EOPNOTSUPP if a flag is passed that it does not support and that's
the mechanism used to inform callers that a fallocate function is
not supported by the underlying filesystem/storage.
Indeed, filesystems can support hole punching at the ->fallocate(),
but then return EOPNOTSUPP because certain dynamic conditions are
not met e.g. CIFS needs sparse file support on the server to support
hole punching, but we don't know this until we actually try to
sparsify the file. IOWs, this patch doesn't address all the cases
where EOPNOTSUPP might actually get returned from filesystems and/or
storage.
This results in syslog warnings "blk_update_request: operation not
supported error, dev loop0, sector 0 op 0x9:(WRITE_ZEROES) flags 0x800800
phys_seg 0 prio class 0". The error can be reproduced with this command:
"truncate -s 1GiB /tmp/file; losetup /dev/loop0 /tmp/file; blkdiscard -z
/dev/loop0"
Which I'm assuming comes from this:
if (unlikely(error && !blk_rq_is_passthrough(req) &&
!(req->rq_flags & RQF_QUIET)))
print_req_error(req, error, __func__);
Which means we could supress the error message quite easily in
lo_fallocate() by doing:
out:
if (ret == -EOPNOTSUPP)
rq->rq_flags |= RQF_QUIET;
return ret;
And then we can also run blk_queue_flag_clear(QUEUE_FLAG_DISCARD)
(and whatever else is needed to kill discards) to turn off future
discard attempts on that loopback device. This way the problem is
just quietly and correctly handled by the loop device and everything
is good...
Thoughts?
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
On Wed, Oct 13, 2021 at 05:28:36AM -0400, Mikulas Patocka wrote:
quoted
Hi
Here I'm sending version 4 of the patch. It adds #include <linux/falloc.h>
to cifs and overlayfs to fix the bugs found out by the kernel test robot.
Mikulas
From: Mikulas Patocka <mpatocka@redhat.com>
The loop driver checks for the fallocate method and if it is present, it
assumes that the filesystem can do FALLOC_FL_ZERO_RANGE and
FALLOC_FL_PUNCH_HOLE requests. However, some filesystems (such as fat, or
tmpfs) have the fallocate method, but lack the capability to do
FALLOC_FL_ZERO_RANGE and/or FALLOC_FL_PUNCH_HOLE.
This seems like a loopback driver level problem, not something
filesystems need to solve. fallocate() is defined to return
-EOPNOTSUPP if a flag is passed that it does not support and that's
the mechanism used to inform callers that a fallocate function is
not supported by the underlying filesystem/storage.
Indeed, filesystems can support hole punching at the ->fallocate(),
but then return EOPNOTSUPP because certain dynamic conditions are
not met e.g. CIFS needs sparse file support on the server to support
hole punching, but we don't know this until we actually try to
sparsify the file. IOWs, this patch doesn't address all the cases
where EOPNOTSUPP might actually get returned from filesystems and/or
storage.
quoted
This results in syslog warnings "blk_update_request: operation not
supported error, dev loop0, sector 0 op 0x9:(WRITE_ZEROES) flags 0x800800
phys_seg 0 prio class 0". The error can be reproduced with this command:
"truncate -s 1GiB /tmp/file; losetup /dev/loop0 /tmp/file; blkdiscard -z
/dev/loop0"
Which I'm assuming comes from this:
if (unlikely(error && !blk_rq_is_passthrough(req) &&
!(req->rq_flags & RQF_QUIET)))
print_req_error(req, error, __func__);
Which means we could supress the error message quite easily in
lo_fallocate() by doing:
out:
if (ret == -EOPNOTSUPP)
rq->rq_flags |= RQF_QUIET;
return ret;
And then we can also run blk_queue_flag_clear(QUEUE_FLAG_DISCARD)
(and whatever else is needed to kill discards) to turn off future
discard attempts on that loopback device. This way the problem is
just quietly and correctly handled by the loop device and everything
is good...
Thoughts?
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
From: Dave Chinner <david@fromorbit.com> Date: 2021-10-28 04:15:22
On Wed, Oct 27, 2021 at 04:28:03AM -0400, Mikulas Patocka wrote:
On Wed, 27 Oct 2021, Dave Chinner wrote:
quoted
On Wed, Oct 13, 2021 at 05:28:36AM -0400, Mikulas Patocka wrote:
quoted
Hi
Here I'm sending version 4 of the patch. It adds #include <linux/falloc.h>
to cifs and overlayfs to fix the bugs found out by the kernel test robot.
Mikulas
From: Mikulas Patocka <mpatocka@redhat.com>
The loop driver checks for the fallocate method and if it is present, it
assumes that the filesystem can do FALLOC_FL_ZERO_RANGE and
FALLOC_FL_PUNCH_HOLE requests. However, some filesystems (such as fat, or
tmpfs) have the fallocate method, but lack the capability to do
FALLOC_FL_ZERO_RANGE and/or FALLOC_FL_PUNCH_HOLE.
This seems like a loopback driver level problem, not something
filesystems need to solve. fallocate() is defined to return
-EOPNOTSUPP if a flag is passed that it does not support and that's
the mechanism used to inform callers that a fallocate function is
not supported by the underlying filesystem/storage.
Indeed, filesystems can support hole punching at the ->fallocate(),
but then return EOPNOTSUPP because certain dynamic conditions are
not met e.g. CIFS needs sparse file support on the server to support
hole punching, but we don't know this until we actually try to
sparsify the file. IOWs, this patch doesn't address all the cases
where EOPNOTSUPP might actually get returned from filesystems and/or
storage.
quoted
This results in syslog warnings "blk_update_request: operation not
supported error, dev loop0, sector 0 op 0x9:(WRITE_ZEROES) flags 0x800800
phys_seg 0 prio class 0". The error can be reproduced with this command:
"truncate -s 1GiB /tmp/file; losetup /dev/loop0 /tmp/file; blkdiscard -z
/dev/loop0"
Which I'm assuming comes from this:
if (unlikely(error && !blk_rq_is_passthrough(req) &&
!(req->rq_flags & RQF_QUIET)))
print_req_error(req, error, __func__);
Which means we could supress the error message quite easily in
lo_fallocate() by doing:
out:
if (ret == -EOPNOTSUPP)
rq->rq_flags |= RQF_QUIET;
return ret;
Looking at the code that has resulted, I think Christoph's
suggestion is a poor one. Code duplication is bad enough, worse is
that it's duplicating the open coding of non-trivial flag
combinations. Given that it is only needed for a single calling
context and it is unnecessary to solve the unique problem at hand
(suppress warning and turn off discard support) this makes it seem
like a case of over-engineering.
Further, it doesn't avoid the need for the loop device to handle
EOPNOTSUPP from fallocate directly, either, because as I explained
above "filesystem type supports the FALLOC_FL_PUNCH_HOLE API flag"
is not the same as "filesystem and/or file instance can execute
FALLOC_FL_PUNCH_HOLE"....
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com