linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] loop: don't print warnings if the underlying filesystem doesn't support discard
@ 2021-09-23 19:48 Mikulas Patocka
  2021-09-24  0:17 ` Ming Lei
  2021-09-24 15:58 ` Christoph Hellwig
  0 siblings, 2 replies; 12+ messages in thread
From: Mikulas Patocka @ 2021-09-23 19:48 UTC (permalink / raw)
  To: Jens Axboe, Christoph Hellwig, Ming Lei, Zdenek Kabelac; +Cc: linux-block

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
===================================================================
--- linux-2.6.orig/drivers/block/loop.c	2021-09-23 17:06:57.000000000 +0200
+++ linux-2.6/drivers/block/loop.c	2021-09-23 21:29:39.000000000 +0200
@@ -493,7 +493,16 @@ static int lo_fallocate(struct loop_devi
 	ret = file->f_op->fallocate(file, mode, pos, blk_rq_bytes(rq));
 	if (unlikely(ret && ret != -EINVAL && ret != -EOPNOTSUPP))
 		ret = -EIO;
- out:
+out:
+
+	/*
+	 * Some filesystems have the fallocate method, but lack the capability
+	 * to do FALLOC_FL_ZERO_RANGE and/or FALLOC_FL_PUNCH_HOLE requests.
+	 * We do not want a syslog warning in this case.
+	 */
+	if (ret == -EOPNOTSUPP)
+		rq->rq_flags |= RQF_QUIET;
+
 	return ret;
 }
 


^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH] loop: don't print warnings if the underlying filesystem doesn't support discard
  2021-09-23 19:48 [PATCH] loop: don't print warnings if the underlying filesystem doesn't support discard Mikulas Patocka
@ 2021-09-24  0:17 ` Ming Lei
  2021-09-24 15:58 ` Christoph Hellwig
  1 sibling, 0 replies; 12+ messages in thread
From: Ming Lei @ 2021-09-24  0:17 UTC (permalink / raw)
  To: Mikulas Patocka
  Cc: Jens Axboe, Christoph Hellwig, Zdenek Kabelac, linux-block

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.
> 
> 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
> ===================================================================
> --- linux-2.6.orig/drivers/block/loop.c	2021-09-23 17:06:57.000000000 +0200
> +++ linux-2.6/drivers/block/loop.c	2021-09-23 21:29:39.000000000 +0200
> @@ -493,7 +493,16 @@ static int lo_fallocate(struct loop_devi
>  	ret = file->f_op->fallocate(file, mode, pos, blk_rq_bytes(rq));
>  	if (unlikely(ret && ret != -EINVAL && ret != -EOPNOTSUPP))
>  		ret = -EIO;
> - out:
> +out:
> +
> +	/*
> +	 * Some filesystems have the fallocate method, but lack the capability
> +	 * to do FALLOC_FL_ZERO_RANGE and/or FALLOC_FL_PUNCH_HOLE requests.
> +	 * We do not want a syslog warning in this case.
> +	 */
> +	if (ret == -EOPNOTSUPP)
> +		rq->rq_flags |= RQF_QUIET;
> +

Looks fine,

Reviewed-by: Ming Lei <ming.lei@redhat.com>

-- 
Ming


^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH] loop: don't print warnings if the underlying filesystem doesn't support discard
  2021-09-23 19:48 [PATCH] loop: don't print warnings if the underlying filesystem doesn't support discard Mikulas Patocka
  2021-09-24  0:17 ` Ming Lei
@ 2021-09-24 15:58 ` Christoph Hellwig
  2021-10-04 13:01   ` Mikulas Patocka
  1 sibling, 1 reply; 12+ messages in thread
From: Christoph Hellwig @ 2021-09-24 15:58 UTC (permalink / raw)
  To: Mikulas Patocka
  Cc: Jens Axboe, Christoph Hellwig, Ming Lei, Zdenek Kabelac, linux-block

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.

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH] loop: don't print warnings if the underlying filesystem doesn't support discard
  2021-09-24 15:58 ` Christoph Hellwig
@ 2021-10-04 13:01   ` Mikulas Patocka
  2021-10-12  6:20     ` Christoph Hellwig
  0 siblings, 1 reply; 12+ messages in thread
From: Mikulas Patocka @ 2021-10-04 13:01 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jens Axboe, Ming Lei, Zdenek Kabelac, linux-block, dm-devel



On Fri, 24 Sep 2021, Christoph Hellwig wrote:

> 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.

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
===================================================================
--- linux-2.6.orig/block/fops.c
+++ linux-2.6/block/fops.c
@@ -628,6 +628,7 @@ const struct file_operations def_blk_fop
 	.splice_read	= generic_file_splice_read,
 	.splice_write	= iter_file_splice_write,
 	.fallocate	= blkdev_fallocate,
+	.fallocate_flags = BLKDEV_FALLOC_FL_SUPPORTED,
 };
 
 static __init int blkdev_init(void)
Index: linux-2.6/fs/btrfs/file.c
===================================================================
--- linux-2.6.orig/fs/btrfs/file.c
+++ linux-2.6/fs/btrfs/file.c
@@ -3688,6 +3688,7 @@ const struct file_operations btrfs_file_
 	.release	= btrfs_release_file,
 	.fsync		= btrfs_sync_file,
 	.fallocate	= btrfs_fallocate,
+	.fallocate_flags = FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE | FALLOC_FL_ZERO_RANGE,
 	.unlocked_ioctl	= btrfs_ioctl,
 #ifdef CONFIG_COMPAT
 	.compat_ioctl	= btrfs_compat_ioctl,
Index: linux-2.6/fs/ceph/file.c
===================================================================
--- linux-2.6.orig/fs/ceph/file.c
+++ linux-2.6/fs/ceph/file.c
@@ -2492,5 +2492,6 @@ const struct file_operations ceph_file_f
 	.unlocked_ioctl = ceph_ioctl,
 	.compat_ioctl = compat_ptr_ioctl,
 	.fallocate	= ceph_fallocate,
+	.fallocate_flags = FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE,
 	.copy_file_range = ceph_copy_file_range,
 };
Index: linux-2.6/fs/cifs/cifsfs.c
===================================================================
--- linux-2.6.orig/fs/cifs/cifsfs.c
+++ linux-2.6/fs/cifs/cifsfs.c
@@ -1281,6 +1281,7 @@ const struct file_operations cifs_file_o
 	.remap_file_range = cifs_remap_file_range,
 	.setlease = cifs_setlease,
 	.fallocate = cifs_fallocate,
+	.fallocate_flags = FALLOC_FL_PUNCH_HOLE | FALLOC_FL_ZERO_RANGE | FALLOC_FL_KEEP_SIZE | FALLOC_FL_COLLAPSE_RANGE | FALLOC_FL_INSERT_RANGE,
 };
 
 const struct file_operations cifs_file_strict_ops = {
@@ -1301,6 +1302,7 @@ const struct file_operations cifs_file_s
 	.remap_file_range = cifs_remap_file_range,
 	.setlease = cifs_setlease,
 	.fallocate = cifs_fallocate,
+	.fallocate_flags = FALLOC_FL_PUNCH_HOLE | FALLOC_FL_ZERO_RANGE | FALLOC_FL_KEEP_SIZE | FALLOC_FL_COLLAPSE_RANGE | FALLOC_FL_INSERT_RANGE,
 };
 
 const struct file_operations cifs_file_direct_ops = {
@@ -1321,6 +1323,7 @@ const struct file_operations cifs_file_d
 	.llseek = cifs_llseek,
 	.setlease = cifs_setlease,
 	.fallocate = cifs_fallocate,
+	.fallocate_flags = FALLOC_FL_PUNCH_HOLE | FALLOC_FL_ZERO_RANGE | FALLOC_FL_KEEP_SIZE | FALLOC_FL_COLLAPSE_RANGE | FALLOC_FL_INSERT_RANGE,
 };
 
 const struct file_operations cifs_file_nobrl_ops = {
@@ -1339,6 +1342,7 @@ const struct file_operations cifs_file_n
 	.remap_file_range = cifs_remap_file_range,
 	.setlease = cifs_setlease,
 	.fallocate = cifs_fallocate,
+	.fallocate_flags = FALLOC_FL_PUNCH_HOLE | FALLOC_FL_ZERO_RANGE | FALLOC_FL_KEEP_SIZE | FALLOC_FL_COLLAPSE_RANGE | FALLOC_FL_INSERT_RANGE,
 };
 
 const struct file_operations cifs_file_strict_nobrl_ops = {
@@ -1357,6 +1361,7 @@ const struct file_operations cifs_file_s
 	.remap_file_range = cifs_remap_file_range,
 	.setlease = cifs_setlease,
 	.fallocate = cifs_fallocate,
+	.fallocate_flags = FALLOC_FL_PUNCH_HOLE | FALLOC_FL_ZERO_RANGE | FALLOC_FL_KEEP_SIZE | FALLOC_FL_COLLAPSE_RANGE | FALLOC_FL_INSERT_RANGE,
 };
 
 const struct file_operations cifs_file_direct_nobrl_ops = {
@@ -1375,6 +1380,7 @@ const struct file_operations cifs_file_d
 	.llseek = cifs_llseek,
 	.setlease = cifs_setlease,
 	.fallocate = cifs_fallocate,
+	.fallocate_flags = FALLOC_FL_PUNCH_HOLE | FALLOC_FL_ZERO_RANGE | FALLOC_FL_KEEP_SIZE | FALLOC_FL_COLLAPSE_RANGE | FALLOC_FL_INSERT_RANGE,
 };
 
 const struct file_operations cifs_dir_ops = {
Index: linux-2.6/fs/ext4/file.c
===================================================================
--- linux-2.6.orig/fs/ext4/file.c
+++ linux-2.6/fs/ext4/file.c
@@ -929,6 +929,7 @@ const struct file_operations ext4_file_o
 	.splice_read	= generic_file_splice_read,
 	.splice_write	= iter_file_splice_write,
 	.fallocate	= ext4_fallocate,
+	.fallocate_flags = FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE | FALLOC_FL_COLLAPSE_RANGE | FALLOC_FL_ZERO_RANGE | FALLOC_FL_INSERT_RANGE,
 };
 
 const struct inode_operations ext4_file_inode_operations = {
Index: linux-2.6/fs/f2fs/file.c
===================================================================
--- linux-2.6.orig/fs/f2fs/file.c
+++ linux-2.6/fs/f2fs/file.c
@@ -4499,6 +4499,7 @@ const struct file_operations f2fs_file_o
 	.flush		= f2fs_file_flush,
 	.fsync		= f2fs_sync_file,
 	.fallocate	= f2fs_fallocate,
+	.fallocate_flags = FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE | FALLOC_FL_COLLAPSE_RANGE | FALLOC_FL_ZERO_RANGE | FALLOC_FL_INSERT_RANGE,
 	.unlocked_ioctl	= f2fs_ioctl,
 #ifdef CONFIG_COMPAT
 	.compat_ioctl	= f2fs_compat_ioctl,
Index: linux-2.6/fs/fat/file.c
===================================================================
--- linux-2.6.orig/fs/fat/file.c
+++ linux-2.6/fs/fat/file.c
@@ -211,6 +211,7 @@ const struct file_operations fat_file_op
 	.splice_read	= generic_file_splice_read,
 	.splice_write	= iter_file_splice_write,
 	.fallocate	= fat_fallocate,
+	.fallocate_flags = FALLOC_FL_KEEP_SIZE,
 };
 
 static int fat_cont_expand(struct inode *inode, loff_t size)
Index: linux-2.6/fs/fuse/file.c
===================================================================
--- linux-2.6.orig/fs/fuse/file.c
+++ linux-2.6/fs/fuse/file.c
@@ -3147,6 +3147,7 @@ static const struct file_operations fuse
 	.compat_ioctl	= fuse_file_compat_ioctl,
 	.poll		= fuse_file_poll,
 	.fallocate	= fuse_file_fallocate,
+	.fallocate_flags = FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE | FALLOC_FL_ZERO_RANGE,
 	.copy_file_range = fuse_copy_file_range,
 };
 
Index: linux-2.6/fs/gfs2/file.c
===================================================================
--- linux-2.6.orig/fs/gfs2/file.c
+++ linux-2.6/fs/gfs2/file.c
@@ -1366,6 +1366,7 @@ const struct file_operations gfs2_file_f
 	.splice_write	= gfs2_file_splice_write,
 	.setlease	= simple_nosetlease,
 	.fallocate	= gfs2_fallocate,
+	.fallocate_flags = FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
 };
 
 const struct file_operations gfs2_dir_fops = {
Index: linux-2.6/fs/hugetlbfs/inode.c
===================================================================
--- linux-2.6.orig/fs/hugetlbfs/inode.c
+++ linux-2.6/fs/hugetlbfs/inode.c
@@ -1163,6 +1163,7 @@ const struct file_operations hugetlbfs_f
 	.get_unmapped_area	= hugetlb_get_unmapped_area,
 	.llseek			= default_llseek,
 	.fallocate		= hugetlbfs_fallocate,
+	.fallocate_flags	= FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE,
 };
 
 static const struct inode_operations hugetlbfs_dir_inode_operations = {
Index: linux-2.6/fs/nfs/nfs4file.c
===================================================================
--- linux-2.6.orig/fs/nfs/nfs4file.c
+++ linux-2.6/fs/nfs/nfs4file.c
@@ -457,6 +457,7 @@ const struct file_operations nfs4_file_o
 	.copy_file_range = nfs4_copy_file_range,
 	.llseek		= nfs4_file_llseek,
 	.fallocate	= nfs42_fallocate,
+	.fallocate_flags = FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
 	.remap_file_range = nfs42_remap_file_range,
 #else
 	.llseek		= nfs_file_llseek,
Index: linux-2.6/fs/ntfs3/file.c
===================================================================
--- linux-2.6.orig/fs/ntfs3/file.c
+++ linux-2.6/fs/ntfs3/file.c
@@ -1246,6 +1246,7 @@ const struct file_operations ntfs_file_o
 	.fsync		= generic_file_fsync,
 	.splice_write	= iter_file_splice_write,
 	.fallocate	= ntfs_fallocate,
+	.fallocate_flags = FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE | FALLOC_FL_COLLAPSE_RANGE,
 	.release	= ntfs_file_release,
 };
 // clang-format on
Index: linux-2.6/fs/ocfs2/file.c
===================================================================
--- linux-2.6.orig/fs/ocfs2/file.c
+++ linux-2.6/fs/ocfs2/file.c
@@ -2746,6 +2746,7 @@ const struct file_operations ocfs2_fops
 	.splice_read	= generic_file_splice_read,
 	.splice_write	= iter_file_splice_write,
 	.fallocate	= ocfs2_fallocate,
+	.fallocate_flags = FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE,
 	.remap_file_range = ocfs2_remap_file_range,
 };
 
@@ -2792,6 +2793,7 @@ const struct file_operations ocfs2_fops_
 	.splice_read	= generic_file_splice_read,
 	.splice_write	= iter_file_splice_write,
 	.fallocate	= ocfs2_fallocate,
+	.fallocate_flags = FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE,
 	.remap_file_range = ocfs2_remap_file_range,
 };
 
Index: linux-2.6/fs/overlayfs/file.c
===================================================================
--- linux-2.6.orig/fs/overlayfs/file.c
+++ linux-2.6/fs/overlayfs/file.c
@@ -645,6 +645,7 @@ const struct file_operations ovl_file_op
 	.fsync		= ovl_fsync,
 	.mmap		= ovl_mmap,
 	.fallocate	= ovl_fallocate,
+	.fallocate_flags = FALLOC_FL_SUPPORTED_MASK,
 	.fadvise	= ovl_fadvise,
 	.flush		= ovl_flush,
 	.splice_read    = generic_file_splice_read,
Index: linux-2.6/fs/xfs/xfs_file.c
===================================================================
--- linux-2.6.orig/fs/xfs/xfs_file.c
+++ linux-2.6/fs/xfs/xfs_file.c
@@ -1464,6 +1464,7 @@ const struct file_operations xfs_file_op
 	.fsync		= xfs_file_fsync,
 	.get_unmapped_area = thp_get_unmapped_area,
 	.fallocate	= xfs_file_fallocate,
+	.fallocate_flags = XFS_FALLOC_FL_SUPPORTED,
 	.fadvise	= xfs_file_fadvise,
 	.remap_file_range = xfs_file_remap_range,
 };
Index: linux-2.6/include/linux/fs.h
===================================================================
--- linux-2.6.orig/include/linux/fs.h
+++ linux-2.6/include/linux/fs.h
@@ -2098,6 +2098,7 @@ struct file_operations {
 	int (*setlease)(struct file *, long, struct file_lock **, void **);
 	long (*fallocate)(struct file *file, int mode, loff_t offset,
 			  loff_t len);
+	unsigned fallocate_flags;
 	void (*show_fdinfo)(struct seq_file *m, struct file *f);
 #ifndef CONFIG_MMU
 	unsigned (*mmap_capabilities)(struct file *);
Index: linux-2.6/ipc/shm.c
===================================================================
--- linux-2.6.orig/ipc/shm.c
+++ linux-2.6/ipc/shm.c
@@ -44,6 +44,7 @@
 #include <linux/mount.h>
 #include <linux/ipc_namespace.h>
 #include <linux/rhashtable.h>
+#include <linux/falloc.h>
 
 #include <linux/uaccess.h>
 
@@ -558,6 +559,7 @@ static const struct file_operations shm_
 	.get_unmapped_area	= shm_get_unmapped_area,
 	.llseek		= noop_llseek,
 	.fallocate	= shm_fallocate,
+	.fallocate_flags = FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE,
 };
 
 /*
@@ -571,6 +573,7 @@ static const struct file_operations shm_
 	.get_unmapped_area	= shm_get_unmapped_area,
 	.llseek		= noop_llseek,
 	.fallocate	= shm_fallocate,
+	.fallocate_flags = FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE,
 };
 
 bool is_file_shm_hugepages(struct file *file)
Index: linux-2.6/mm/shmem.c
===================================================================
--- linux-2.6.orig/mm/shmem.c
+++ linux-2.6/mm/shmem.c
@@ -3797,6 +3797,7 @@ static const struct file_operations shme
 	.splice_read	= generic_file_splice_read,
 	.splice_write	= iter_file_splice_write,
 	.fallocate	= shmem_fallocate,
+	.fallocate_flags = FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE,
 #endif
 };
 
Index: linux-2.6/drivers/block/loop.c
===================================================================
--- linux-2.6.orig/drivers/block/loop.c
+++ linux-2.6/drivers/block/loop.c
@@ -947,7 +947,9 @@ static void loop_config_discard(struct l
 	 * encryption is enabled, because it may give an attacker
 	 * useful information.
 	 */
-	} else if (!file->f_op->fallocate || lo->lo_encrypt_key_size) {
+	} else if ((file->f_op->fallocate_flags & (FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE)) !=
+						  (FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE) ||
+		   lo->lo_encrypt_key_size) {
 		max_discard_sectors = 0;
 		granularity = 0;
 
@@ -959,7 +961,10 @@ static void loop_config_discard(struct l
 	if (max_discard_sectors) {
 		q->limits.discard_granularity = granularity;
 		blk_queue_max_discard_sectors(q, max_discard_sectors);
-		blk_queue_max_write_zeroes_sectors(q, max_discard_sectors);
+		if (file->f_op->fallocate_flags & FALLOC_FL_ZERO_RANGE)
+			blk_queue_max_write_zeroes_sectors(q, max_discard_sectors);
+		else
+			blk_queue_max_write_zeroes_sectors(q, 0);
 		blk_queue_flag_set(QUEUE_FLAG_DISCARD, q);
 	} else {
 		q->limits.discard_granularity = 0;


^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH] loop: don't print warnings if the underlying filesystem doesn't support discard
  2021-10-04 13:01   ` Mikulas Patocka
@ 2021-10-12  6:20     ` Christoph Hellwig
  2021-10-12 20:25       ` [PATCH v3] " Mikulas Patocka
  0 siblings, 1 reply; 12+ messages in thread
From: Christoph Hellwig @ 2021-10-12  6:20 UTC (permalink / raw)
  To: Mikulas Patocka
  Cc: Christoph Hellwig, Jens Axboe, Ming Lei, Zdenek Kabelac,
	linux-block, dm-devel

On Mon, Oct 04, 2021 at 09:01:33AM -0400, Mikulas Patocka wrote:
> Do you want this patch?

Yes, this looks like what I want.  Minor nitpicks below:

> +	.fallocate_flags = BLKDEV_FALLOC_FL_SUPPORTED,

I'd probably call this fallocate_supported_flags.

> +	.fallocate_flags = FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE | FALLOC_FL_ZERO_RANGE,

Please avoid over 80 lines for a plain list of flags.

^ permalink raw reply	[flat|nested] 12+ messages in thread
* [PATCH v3] loop: don't print warnings if the underlying filesystem doesn't support discard
  2021-10-12  6:20     ` Christoph Hellwig
@ 2021-10-12 20:25       ` Mikulas Patocka
  2021-10-13  2:56         ` kernel test robot
                           ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Mikulas Patocka @ 2021-10-12 20:25 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jens Axboe, Ming Lei, Zdenek Kabelac, linux-block, dm-devel,
	linux-fsdevel



On Tue, 12 Oct 2021, Christoph Hellwig wrote:

> On Mon, Oct 04, 2021 at 09:01:33AM -0400, Mikulas Patocka wrote:
> > Do you want this patch?
> 
> Yes, this looks like what I want.  Minor nitpicks below:
> 
> > +	.fallocate_flags = BLKDEV_FALLOC_FL_SUPPORTED,
> 
> I'd probably call this fallocate_supported_flags.
> 
> > +	.fallocate_flags = FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE | FALLOC_FL_ZERO_RANGE,
> 
> 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
===================================================================
--- linux-2.6.orig/block/fops.c
+++ linux-2.6/block/fops.c
@@ -628,6 +628,7 @@ const struct file_operations def_blk_fop
 	.splice_read	= generic_file_splice_read,
 	.splice_write	= iter_file_splice_write,
 	.fallocate	= blkdev_fallocate,
+	.fallocate_supported_flags = BLKDEV_FALLOC_FL_SUPPORTED,
 };
 
 static __init int blkdev_init(void)
Index: linux-2.6/fs/btrfs/file.c
===================================================================
--- linux-2.6.orig/fs/btrfs/file.c
+++ linux-2.6/fs/btrfs/file.c
@@ -3688,6 +3688,8 @@ const struct file_operations btrfs_file_
 	.release	= btrfs_release_file,
 	.fsync		= btrfs_sync_file,
 	.fallocate	= btrfs_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_KEEP_SIZE |
+		FALLOC_FL_PUNCH_HOLE | FALLOC_FL_ZERO_RANGE,
 	.unlocked_ioctl	= btrfs_ioctl,
 #ifdef CONFIG_COMPAT
 	.compat_ioctl	= btrfs_compat_ioctl,
Index: linux-2.6/fs/ceph/file.c
===================================================================
--- linux-2.6.orig/fs/ceph/file.c
+++ linux-2.6/fs/ceph/file.c
@@ -2492,5 +2492,6 @@ const struct file_operations ceph_file_f
 	.unlocked_ioctl = ceph_ioctl,
 	.compat_ioctl = compat_ptr_ioctl,
 	.fallocate	= ceph_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE,
 	.copy_file_range = ceph_copy_file_range,
 };
Index: linux-2.6/fs/cifs/cifsfs.c
===================================================================
--- linux-2.6.orig/fs/cifs/cifsfs.c
+++ linux-2.6/fs/cifs/cifsfs.c
@@ -1281,6 +1281,9 @@ const struct file_operations cifs_file_o
 	.remap_file_range = cifs_remap_file_range,
 	.setlease = cifs_setlease,
 	.fallocate = cifs_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_PUNCH_HOLE |
+		FALLOC_FL_ZERO_RANGE | FALLOC_FL_KEEP_SIZE |
+		FALLOC_FL_COLLAPSE_RANGE | FALLOC_FL_INSERT_RANGE,
 };
 
 const struct file_operations cifs_file_strict_ops = {
@@ -1301,6 +1304,9 @@ const struct file_operations cifs_file_s
 	.remap_file_range = cifs_remap_file_range,
 	.setlease = cifs_setlease,
 	.fallocate = cifs_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_PUNCH_HOLE |
+		FALLOC_FL_ZERO_RANGE | FALLOC_FL_KEEP_SIZE |
+		FALLOC_FL_COLLAPSE_RANGE | FALLOC_FL_INSERT_RANGE,
 };
 
 const struct file_operations cifs_file_direct_ops = {
@@ -1321,6 +1327,9 @@ const struct file_operations cifs_file_d
 	.llseek = cifs_llseek,
 	.setlease = cifs_setlease,
 	.fallocate = cifs_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_PUNCH_HOLE |
+		FALLOC_FL_ZERO_RANGE | FALLOC_FL_KEEP_SIZE |
+		FALLOC_FL_COLLAPSE_RANGE | FALLOC_FL_INSERT_RANGE,
 };
 
 const struct file_operations cifs_file_nobrl_ops = {
@@ -1339,6 +1348,9 @@ const struct file_operations cifs_file_n
 	.remap_file_range = cifs_remap_file_range,
 	.setlease = cifs_setlease,
 	.fallocate = cifs_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_PUNCH_HOLE |
+		FALLOC_FL_ZERO_RANGE | FALLOC_FL_KEEP_SIZE |
+		FALLOC_FL_COLLAPSE_RANGE | FALLOC_FL_INSERT_RANGE,
 };
 
 const struct file_operations cifs_file_strict_nobrl_ops = {
@@ -1357,6 +1369,9 @@ const struct file_operations cifs_file_s
 	.remap_file_range = cifs_remap_file_range,
 	.setlease = cifs_setlease,
 	.fallocate = cifs_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_PUNCH_HOLE |
+		FALLOC_FL_ZERO_RANGE | FALLOC_FL_KEEP_SIZE |
+		FALLOC_FL_COLLAPSE_RANGE | FALLOC_FL_INSERT_RANGE,
 };
 
 const struct file_operations cifs_file_direct_nobrl_ops = {
@@ -1375,6 +1390,9 @@ const struct file_operations cifs_file_d
 	.llseek = cifs_llseek,
 	.setlease = cifs_setlease,
 	.fallocate = cifs_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_PUNCH_HOLE |
+		FALLOC_FL_ZERO_RANGE | FALLOC_FL_KEEP_SIZE |
+		FALLOC_FL_COLLAPSE_RANGE | FALLOC_FL_INSERT_RANGE,
 };
 
 const struct file_operations cifs_dir_ops = {
Index: linux-2.6/fs/ext4/file.c
===================================================================
--- linux-2.6.orig/fs/ext4/file.c
+++ linux-2.6/fs/ext4/file.c
@@ -929,6 +929,9 @@ const struct file_operations ext4_file_o
 	.splice_read	= generic_file_splice_read,
 	.splice_write	= iter_file_splice_write,
 	.fallocate	= ext4_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_KEEP_SIZE |
+		FALLOC_FL_PUNCH_HOLE | FALLOC_FL_COLLAPSE_RANGE |
+		FALLOC_FL_ZERO_RANGE | FALLOC_FL_INSERT_RANGE,
 };
 
 const struct inode_operations ext4_file_inode_operations = {
Index: linux-2.6/fs/f2fs/file.c
===================================================================
--- linux-2.6.orig/fs/f2fs/file.c
+++ linux-2.6/fs/f2fs/file.c
@@ -4499,6 +4499,9 @@ const struct file_operations f2fs_file_o
 	.flush		= f2fs_file_flush,
 	.fsync		= f2fs_sync_file,
 	.fallocate	= f2fs_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_KEEP_SIZE |
+		FALLOC_FL_PUNCH_HOLE | FALLOC_FL_COLLAPSE_RANGE |
+		FALLOC_FL_ZERO_RANGE | FALLOC_FL_INSERT_RANGE,
 	.unlocked_ioctl	= f2fs_ioctl,
 #ifdef CONFIG_COMPAT
 	.compat_ioctl	= f2fs_compat_ioctl,
Index: linux-2.6/fs/fat/file.c
===================================================================
--- linux-2.6.orig/fs/fat/file.c
+++ linux-2.6/fs/fat/file.c
@@ -211,6 +211,7 @@ const struct file_operations fat_file_op
 	.splice_read	= generic_file_splice_read,
 	.splice_write	= iter_file_splice_write,
 	.fallocate	= fat_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_KEEP_SIZE,
 };
 
 static int fat_cont_expand(struct inode *inode, loff_t size)
Index: linux-2.6/fs/fuse/file.c
===================================================================
--- linux-2.6.orig/fs/fuse/file.c
+++ linux-2.6/fs/fuse/file.c
@@ -3147,6 +3147,8 @@ static const struct file_operations fuse
 	.compat_ioctl	= fuse_file_compat_ioctl,
 	.poll		= fuse_file_poll,
 	.fallocate	= fuse_file_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_KEEP_SIZE |
+		FALLOC_FL_PUNCH_HOLE | FALLOC_FL_ZERO_RANGE,
 	.copy_file_range = fuse_copy_file_range,
 };
 
Index: linux-2.6/fs/gfs2/file.c
===================================================================
--- linux-2.6.orig/fs/gfs2/file.c
+++ linux-2.6/fs/gfs2/file.c
@@ -1366,6 +1366,7 @@ const struct file_operations gfs2_file_f
 	.splice_write	= gfs2_file_splice_write,
 	.setlease	= simple_nosetlease,
 	.fallocate	= gfs2_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
 };
 
 const struct file_operations gfs2_dir_fops = {
Index: linux-2.6/fs/hugetlbfs/inode.c
===================================================================
--- linux-2.6.orig/fs/hugetlbfs/inode.c
+++ linux-2.6/fs/hugetlbfs/inode.c
@@ -1163,6 +1163,7 @@ const struct file_operations hugetlbfs_f
 	.get_unmapped_area	= hugetlb_get_unmapped_area,
 	.llseek			= default_llseek,
 	.fallocate		= hugetlbfs_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE,
 };
 
 static const struct inode_operations hugetlbfs_dir_inode_operations = {
Index: linux-2.6/fs/nfs/nfs4file.c
===================================================================
--- linux-2.6.orig/fs/nfs/nfs4file.c
+++ linux-2.6/fs/nfs/nfs4file.c
@@ -457,6 +457,7 @@ const struct file_operations nfs4_file_o
 	.copy_file_range = nfs4_copy_file_range,
 	.llseek		= nfs4_file_llseek,
 	.fallocate	= nfs42_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
 	.remap_file_range = nfs42_remap_file_range,
 #else
 	.llseek		= nfs_file_llseek,
Index: linux-2.6/fs/ntfs3/file.c
===================================================================
--- linux-2.6.orig/fs/ntfs3/file.c
+++ linux-2.6/fs/ntfs3/file.c
@@ -1246,6 +1246,8 @@ const struct file_operations ntfs_file_o
 	.fsync		= generic_file_fsync,
 	.splice_write	= iter_file_splice_write,
 	.fallocate	= ntfs_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_KEEP_SIZE |
+		FALLOC_FL_PUNCH_HOLE | FALLOC_FL_COLLAPSE_RANGE,
 	.release	= ntfs_file_release,
 };
 // clang-format on
Index: linux-2.6/fs/ocfs2/file.c
===================================================================
--- linux-2.6.orig/fs/ocfs2/file.c
+++ linux-2.6/fs/ocfs2/file.c
@@ -2746,6 +2746,7 @@ const struct file_operations ocfs2_fops
 	.splice_read	= generic_file_splice_read,
 	.splice_write	= iter_file_splice_write,
 	.fallocate	= ocfs2_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE,
 	.remap_file_range = ocfs2_remap_file_range,
 };
 
@@ -2792,6 +2793,7 @@ const struct file_operations ocfs2_fops_
 	.splice_read	= generic_file_splice_read,
 	.splice_write	= iter_file_splice_write,
 	.fallocate	= ocfs2_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE,
 	.remap_file_range = ocfs2_remap_file_range,
 };
 
Index: linux-2.6/fs/overlayfs/file.c
===================================================================
--- linux-2.6.orig/fs/overlayfs/file.c
+++ linux-2.6/fs/overlayfs/file.c
@@ -658,6 +658,7 @@ const struct file_operations ovl_file_op
 	.fsync		= ovl_fsync,
 	.mmap		= ovl_mmap,
 	.fallocate	= ovl_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_SUPPORTED_MASK,
 	.fadvise	= ovl_fadvise,
 	.flush		= ovl_flush,
 	.splice_read    = generic_file_splice_read,
Index: linux-2.6/fs/xfs/xfs_file.c
===================================================================
--- linux-2.6.orig/fs/xfs/xfs_file.c
+++ linux-2.6/fs/xfs/xfs_file.c
@@ -1464,6 +1464,7 @@ const struct file_operations xfs_file_op
 	.fsync		= xfs_file_fsync,
 	.get_unmapped_area = thp_get_unmapped_area,
 	.fallocate	= xfs_file_fallocate,
+	.fallocate_supported_flags = XFS_FALLOC_FL_SUPPORTED,
 	.fadvise	= xfs_file_fadvise,
 	.remap_file_range = xfs_file_remap_range,
 };
Index: linux-2.6/include/linux/fs.h
===================================================================
--- linux-2.6.orig/include/linux/fs.h
+++ linux-2.6/include/linux/fs.h
@@ -2098,6 +2098,7 @@ struct file_operations {
 	int (*setlease)(struct file *, long, struct file_lock **, void **);
 	long (*fallocate)(struct file *file, int mode, loff_t offset,
 			  loff_t len);
+	unsigned fallocate_supported_flags;
 	void (*show_fdinfo)(struct seq_file *m, struct file *f);
 #ifndef CONFIG_MMU
 	unsigned (*mmap_capabilities)(struct file *);
Index: linux-2.6/ipc/shm.c
===================================================================
--- linux-2.6.orig/ipc/shm.c
+++ linux-2.6/ipc/shm.c
@@ -44,6 +44,7 @@
 #include <linux/mount.h>
 #include <linux/ipc_namespace.h>
 #include <linux/rhashtable.h>
+#include <linux/falloc.h>
 
 #include <linux/uaccess.h>
 
@@ -558,6 +559,7 @@ static const struct file_operations shm_
 	.get_unmapped_area	= shm_get_unmapped_area,
 	.llseek		= noop_llseek,
 	.fallocate	= shm_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE,
 };
 
 /*
@@ -571,6 +573,7 @@ static const struct file_operations shm_
 	.get_unmapped_area	= shm_get_unmapped_area,
 	.llseek		= noop_llseek,
 	.fallocate	= shm_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE,
 };
 
 bool is_file_shm_hugepages(struct file *file)
Index: linux-2.6/mm/shmem.c
===================================================================
--- linux-2.6.orig/mm/shmem.c
+++ linux-2.6/mm/shmem.c
@@ -3797,6 +3797,7 @@ static const struct file_operations shme
 	.splice_read	= generic_file_splice_read,
 	.splice_write	= iter_file_splice_write,
 	.fallocate	= shmem_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE,
 #endif
 };
 
Index: linux-2.6/drivers/block/loop.c
===================================================================
--- linux-2.6.orig/drivers/block/loop.c
+++ linux-2.6/drivers/block/loop.c
@@ -947,7 +947,10 @@ static void loop_config_discard(struct l
 	 * encryption is enabled, because it may give an attacker
 	 * useful information.
 	 */
-	} else if (!file->f_op->fallocate || lo->lo_encrypt_key_size) {
+	} else if ((file->f_op->fallocate_supported_flags &
+			(FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE)) !=
+			(FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE) ||
+		   lo->lo_encrypt_key_size) {
 		max_discard_sectors = 0;
 		granularity = 0;
 
@@ -959,7 +962,10 @@ static void loop_config_discard(struct l
 	if (max_discard_sectors) {
 		q->limits.discard_granularity = granularity;
 		blk_queue_max_discard_sectors(q, max_discard_sectors);
-		blk_queue_max_write_zeroes_sectors(q, max_discard_sectors);
+		if (file->f_op->fallocate_supported_flags & FALLOC_FL_ZERO_RANGE)
+			blk_queue_max_write_zeroes_sectors(q, max_discard_sectors);
+		else
+			blk_queue_max_write_zeroes_sectors(q, 0);
 		blk_queue_flag_set(QUEUE_FLAG_DISCARD, q);
 	} else {
 		q->limits.discard_granularity = 0;


^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH v3] loop: don't print warnings if the underlying filesystem doesn't support discard
  2021-10-12 20:25       ` [PATCH v3] " Mikulas Patocka
@ 2021-10-13  2:56         ` kernel test robot
  2021-10-13  5:06         ` kernel test robot
  2021-10-13  9:28         ` [PATCH v4] " Mikulas Patocka
  2 siblings, 0 replies; 12+ messages in thread
From: kernel test robot @ 2021-10-13  2:56 UTC (permalink / raw)
  To: Mikulas Patocka, Christoph Hellwig
  Cc: kbuild-all, Jens Axboe, Ming Lei, Zdenek Kabelac, linux-block,
	dm-devel, linux-fsdevel

[-- Attachment #1: Type: text/plain, Size: 2862 bytes --]

Hi Mikulas,

I love your patch! Yet something to improve:

[auto build test ERROR on axboe-block/for-next]
[also build test ERROR on kdave/for-next ceph-client/for-linus cifs/for-next tytso-ext4/dev jaegeuk-f2fs/dev-test mszeredi-fuse/for-next linus/master v5.15-rc5 next-20211012]
[cannot apply to hch-configfs/for-next gfs2/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Mikulas-Patocka/loop-don-t-print-warnings-if-the-underlying-filesystem-doesn-t-support-discard/20211013-042727
base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
config: arc-randconfig-r043-20211012 (attached as .config)
compiler: arc-elf-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/c381403746bc0dc3eb5db4b157408430febd6ecf
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Mikulas-Patocka/loop-don-t-print-warnings-if-the-underlying-filesystem-doesn-t-support-discard/20211013-042727
        git checkout c381403746bc0dc3eb5db4b157408430febd6ecf
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=arc SHELL=/bin/bash fs/fuse/ fs/overlayfs/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> fs/overlayfs/file.c:661:38: error: 'FALLOC_FL_SUPPORTED_MASK' undeclared here (not in a function)
     661 |         .fallocate_supported_flags = FALLOC_FL_SUPPORTED_MASK,
         |                                      ^~~~~~~~~~~~~~~~~~~~~~~~


vim +/FALLOC_FL_SUPPORTED_MASK +661 fs/overlayfs/file.c

   651	
   652	const struct file_operations ovl_file_operations = {
   653		.open		= ovl_open,
   654		.release	= ovl_release,
   655		.llseek		= ovl_llseek,
   656		.read_iter	= ovl_read_iter,
   657		.write_iter	= ovl_write_iter,
   658		.fsync		= ovl_fsync,
   659		.mmap		= ovl_mmap,
   660		.fallocate	= ovl_fallocate,
 > 661		.fallocate_supported_flags = FALLOC_FL_SUPPORTED_MASK,
   662		.fadvise	= ovl_fadvise,
   663		.flush		= ovl_flush,
   664		.splice_read    = generic_file_splice_read,
   665		.splice_write   = ovl_splice_write,
   666	
   667		.copy_file_range	= ovl_copy_file_range,
   668		.remap_file_range	= ovl_remap_file_range,
   669	};
   670	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 33898 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH v3] loop: don't print warnings if the underlying filesystem doesn't support discard
  2021-10-12 20:25       ` [PATCH v3] " Mikulas Patocka
  2021-10-13  2:56         ` kernel test robot
@ 2021-10-13  5:06         ` kernel test robot
  2021-10-13  9:28         ` [PATCH v4] " Mikulas Patocka
  2 siblings, 0 replies; 12+ messages in thread
From: kernel test robot @ 2021-10-13  5:06 UTC (permalink / raw)
  To: Mikulas Patocka, Christoph Hellwig
  Cc: llvm, kbuild-all, Jens Axboe, Ming Lei, Zdenek Kabelac,
	linux-block, dm-devel, linux-fsdevel

[-- Attachment #1: Type: text/plain, Size: 6578 bytes --]

Hi Mikulas,

I love your patch! Yet something to improve:

[auto build test ERROR on axboe-block/for-next]
[also build test ERROR on kdave/for-next ceph-client/for-linus cifs/for-next tytso-ext4/dev jaegeuk-f2fs/dev-test mszeredi-fuse/for-next linus/master v5.15-rc5 next-20211012]
[cannot apply to hch-configfs/for-next gfs2/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Mikulas-Patocka/loop-don-t-print-warnings-if-the-underlying-filesystem-doesn-t-support-discard/20211013-042727
base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
config: x86_64-randconfig-a015-20211012 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project adf55ac6657693f7bfbe3087b599b4031a765a44)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/c381403746bc0dc3eb5db4b157408430febd6ecf
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Mikulas-Patocka/loop-don-t-print-warnings-if-the-underlying-filesystem-doesn-t-support-discard/20211013-042727
        git checkout c381403746bc0dc3eb5db4b157408430febd6ecf
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash fs/cifs/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> fs/cifs/cifsfs.c:1284:31: error: use of undeclared identifier 'FALLOC_FL_PUNCH_HOLE'
           .fallocate_supported_flags = FALLOC_FL_PUNCH_HOLE |
                                        ^
>> fs/cifs/cifsfs.c:1285:3: error: use of undeclared identifier 'FALLOC_FL_ZERO_RANGE'
                   FALLOC_FL_ZERO_RANGE | FALLOC_FL_KEEP_SIZE |
                   ^
>> fs/cifs/cifsfs.c:1285:26: error: use of undeclared identifier 'FALLOC_FL_KEEP_SIZE'
                   FALLOC_FL_ZERO_RANGE | FALLOC_FL_KEEP_SIZE |
                                          ^
>> fs/cifs/cifsfs.c:1286:3: error: use of undeclared identifier 'FALLOC_FL_COLLAPSE_RANGE'
                   FALLOC_FL_COLLAPSE_RANGE | FALLOC_FL_INSERT_RANGE,
                   ^
>> fs/cifs/cifsfs.c:1286:30: error: use of undeclared identifier 'FALLOC_FL_INSERT_RANGE'
                   FALLOC_FL_COLLAPSE_RANGE | FALLOC_FL_INSERT_RANGE,
                                              ^
   fs/cifs/cifsfs.c:1307:31: error: use of undeclared identifier 'FALLOC_FL_PUNCH_HOLE'
           .fallocate_supported_flags = FALLOC_FL_PUNCH_HOLE |
                                        ^
   fs/cifs/cifsfs.c:1308:3: error: use of undeclared identifier 'FALLOC_FL_ZERO_RANGE'
                   FALLOC_FL_ZERO_RANGE | FALLOC_FL_KEEP_SIZE |
                   ^
   fs/cifs/cifsfs.c:1308:26: error: use of undeclared identifier 'FALLOC_FL_KEEP_SIZE'
                   FALLOC_FL_ZERO_RANGE | FALLOC_FL_KEEP_SIZE |
                                          ^
   fs/cifs/cifsfs.c:1309:3: error: use of undeclared identifier 'FALLOC_FL_COLLAPSE_RANGE'
                   FALLOC_FL_COLLAPSE_RANGE | FALLOC_FL_INSERT_RANGE,
                   ^
   fs/cifs/cifsfs.c:1309:30: error: use of undeclared identifier 'FALLOC_FL_INSERT_RANGE'
                   FALLOC_FL_COLLAPSE_RANGE | FALLOC_FL_INSERT_RANGE,
                                              ^
   fs/cifs/cifsfs.c:1330:31: error: use of undeclared identifier 'FALLOC_FL_PUNCH_HOLE'
           .fallocate_supported_flags = FALLOC_FL_PUNCH_HOLE |
                                        ^
   fs/cifs/cifsfs.c:1331:3: error: use of undeclared identifier 'FALLOC_FL_ZERO_RANGE'
                   FALLOC_FL_ZERO_RANGE | FALLOC_FL_KEEP_SIZE |
                   ^
   fs/cifs/cifsfs.c:1331:26: error: use of undeclared identifier 'FALLOC_FL_KEEP_SIZE'
                   FALLOC_FL_ZERO_RANGE | FALLOC_FL_KEEP_SIZE |
                                          ^
   fs/cifs/cifsfs.c:1332:3: error: use of undeclared identifier 'FALLOC_FL_COLLAPSE_RANGE'
                   FALLOC_FL_COLLAPSE_RANGE | FALLOC_FL_INSERT_RANGE,
                   ^
   fs/cifs/cifsfs.c:1332:30: error: use of undeclared identifier 'FALLOC_FL_INSERT_RANGE'
                   FALLOC_FL_COLLAPSE_RANGE | FALLOC_FL_INSERT_RANGE,
                                              ^
   fs/cifs/cifsfs.c:1351:31: error: use of undeclared identifier 'FALLOC_FL_PUNCH_HOLE'
           .fallocate_supported_flags = FALLOC_FL_PUNCH_HOLE |
                                        ^
   fs/cifs/cifsfs.c:1352:3: error: use of undeclared identifier 'FALLOC_FL_ZERO_RANGE'
                   FALLOC_FL_ZERO_RANGE | FALLOC_FL_KEEP_SIZE |
                   ^
   fs/cifs/cifsfs.c:1352:26: error: use of undeclared identifier 'FALLOC_FL_KEEP_SIZE'
                   FALLOC_FL_ZERO_RANGE | FALLOC_FL_KEEP_SIZE |
                                          ^
   fs/cifs/cifsfs.c:1353:3: error: use of undeclared identifier 'FALLOC_FL_COLLAPSE_RANGE'
                   FALLOC_FL_COLLAPSE_RANGE | FALLOC_FL_INSERT_RANGE,
                   ^
   fatal error: too many errors emitted, stopping now [-ferror-limit=]
   20 errors generated.


vim +/FALLOC_FL_PUNCH_HOLE +1284 fs/cifs/cifsfs.c

  1265	
  1266	const struct file_operations cifs_file_ops = {
  1267		.read_iter = cifs_loose_read_iter,
  1268		.write_iter = cifs_file_write_iter,
  1269		.open = cifs_open,
  1270		.release = cifs_close,
  1271		.lock = cifs_lock,
  1272		.flock = cifs_flock,
  1273		.fsync = cifs_fsync,
  1274		.flush = cifs_flush,
  1275		.mmap  = cifs_file_mmap,
  1276		.splice_read = generic_file_splice_read,
  1277		.splice_write = iter_file_splice_write,
  1278		.llseek = cifs_llseek,
  1279		.unlocked_ioctl	= cifs_ioctl,
  1280		.copy_file_range = cifs_copy_file_range,
  1281		.remap_file_range = cifs_remap_file_range,
  1282		.setlease = cifs_setlease,
  1283		.fallocate = cifs_fallocate,
> 1284		.fallocate_supported_flags = FALLOC_FL_PUNCH_HOLE |
> 1285			FALLOC_FL_ZERO_RANGE | FALLOC_FL_KEEP_SIZE |
> 1286			FALLOC_FL_COLLAPSE_RANGE | FALLOC_FL_INSERT_RANGE,
  1287	};
  1288	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 33439 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread
* [PATCH v4] loop: don't print warnings if the underlying filesystem doesn't support discard
  2021-10-12 20:25       ` [PATCH v3] " Mikulas Patocka
  2021-10-13  2:56         ` kernel test robot
  2021-10-13  5:06         ` kernel test robot
@ 2021-10-13  9:28         ` Mikulas Patocka
  2021-10-27  5:02           ` Dave Chinner
  2 siblings, 1 reply; 12+ messages in thread
From: Mikulas Patocka @ 2021-10-13  9:28 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jens Axboe, Ming Lei, Zdenek Kabelac, linux-block, dm-devel,
	linux-fsdevel

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
===================================================================
--- linux-2.6.orig/block/fops.c
+++ linux-2.6/block/fops.c
@@ -628,6 +628,7 @@ const struct file_operations def_blk_fop
 	.splice_read	= generic_file_splice_read,
 	.splice_write	= iter_file_splice_write,
 	.fallocate	= blkdev_fallocate,
+	.fallocate_supported_flags = BLKDEV_FALLOC_FL_SUPPORTED,
 };
 
 static __init int blkdev_init(void)
Index: linux-2.6/fs/btrfs/file.c
===================================================================
--- linux-2.6.orig/fs/btrfs/file.c
+++ linux-2.6/fs/btrfs/file.c
@@ -3688,6 +3688,8 @@ const struct file_operations btrfs_file_
 	.release	= btrfs_release_file,
 	.fsync		= btrfs_sync_file,
 	.fallocate	= btrfs_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_KEEP_SIZE |
+		FALLOC_FL_PUNCH_HOLE | FALLOC_FL_ZERO_RANGE,
 	.unlocked_ioctl	= btrfs_ioctl,
 #ifdef CONFIG_COMPAT
 	.compat_ioctl	= btrfs_compat_ioctl,
Index: linux-2.6/fs/ceph/file.c
===================================================================
--- linux-2.6.orig/fs/ceph/file.c
+++ linux-2.6/fs/ceph/file.c
@@ -2492,5 +2492,6 @@ const struct file_operations ceph_file_f
 	.unlocked_ioctl = ceph_ioctl,
 	.compat_ioctl = compat_ptr_ioctl,
 	.fallocate	= ceph_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE,
 	.copy_file_range = ceph_copy_file_range,
 };
Index: linux-2.6/fs/cifs/cifsfs.c
===================================================================
--- linux-2.6.orig/fs/cifs/cifsfs.c
+++ linux-2.6/fs/cifs/cifsfs.c
@@ -26,6 +26,7 @@
 #include <linux/random.h>
 #include <linux/uuid.h>
 #include <linux/xattr.h>
+#include <linux/falloc.h>
 #include <net/ipv6.h>
 #include "cifsfs.h"
 #include "cifspdu.h"
@@ -1281,6 +1282,9 @@ const struct file_operations cifs_file_o
 	.remap_file_range = cifs_remap_file_range,
 	.setlease = cifs_setlease,
 	.fallocate = cifs_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_PUNCH_HOLE |
+		FALLOC_FL_ZERO_RANGE | FALLOC_FL_KEEP_SIZE |
+		FALLOC_FL_COLLAPSE_RANGE | FALLOC_FL_INSERT_RANGE,
 };
 
 const struct file_operations cifs_file_strict_ops = {
@@ -1301,6 +1305,9 @@ const struct file_operations cifs_file_s
 	.remap_file_range = cifs_remap_file_range,
 	.setlease = cifs_setlease,
 	.fallocate = cifs_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_PUNCH_HOLE |
+		FALLOC_FL_ZERO_RANGE | FALLOC_FL_KEEP_SIZE |
+		FALLOC_FL_COLLAPSE_RANGE | FALLOC_FL_INSERT_RANGE,
 };
 
 const struct file_operations cifs_file_direct_ops = {
@@ -1321,6 +1328,9 @@ const struct file_operations cifs_file_d
 	.llseek = cifs_llseek,
 	.setlease = cifs_setlease,
 	.fallocate = cifs_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_PUNCH_HOLE |
+		FALLOC_FL_ZERO_RANGE | FALLOC_FL_KEEP_SIZE |
+		FALLOC_FL_COLLAPSE_RANGE | FALLOC_FL_INSERT_RANGE,
 };
 
 const struct file_operations cifs_file_nobrl_ops = {
@@ -1339,6 +1349,9 @@ const struct file_operations cifs_file_n
 	.remap_file_range = cifs_remap_file_range,
 	.setlease = cifs_setlease,
 	.fallocate = cifs_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_PUNCH_HOLE |
+		FALLOC_FL_ZERO_RANGE | FALLOC_FL_KEEP_SIZE |
+		FALLOC_FL_COLLAPSE_RANGE | FALLOC_FL_INSERT_RANGE,
 };
 
 const struct file_operations cifs_file_strict_nobrl_ops = {
@@ -1357,6 +1370,9 @@ const struct file_operations cifs_file_s
 	.remap_file_range = cifs_remap_file_range,
 	.setlease = cifs_setlease,
 	.fallocate = cifs_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_PUNCH_HOLE |
+		FALLOC_FL_ZERO_RANGE | FALLOC_FL_KEEP_SIZE |
+		FALLOC_FL_COLLAPSE_RANGE | FALLOC_FL_INSERT_RANGE,
 };
 
 const struct file_operations cifs_file_direct_nobrl_ops = {
@@ -1375,6 +1391,9 @@ const struct file_operations cifs_file_d
 	.llseek = cifs_llseek,
 	.setlease = cifs_setlease,
 	.fallocate = cifs_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_PUNCH_HOLE |
+		FALLOC_FL_ZERO_RANGE | FALLOC_FL_KEEP_SIZE |
+		FALLOC_FL_COLLAPSE_RANGE | FALLOC_FL_INSERT_RANGE,
 };
 
 const struct file_operations cifs_dir_ops = {
Index: linux-2.6/fs/ext4/file.c
===================================================================
--- linux-2.6.orig/fs/ext4/file.c
+++ linux-2.6/fs/ext4/file.c
@@ -929,6 +929,9 @@ const struct file_operations ext4_file_o
 	.splice_read	= generic_file_splice_read,
 	.splice_write	= iter_file_splice_write,
 	.fallocate	= ext4_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_KEEP_SIZE |
+		FALLOC_FL_PUNCH_HOLE | FALLOC_FL_COLLAPSE_RANGE |
+		FALLOC_FL_ZERO_RANGE | FALLOC_FL_INSERT_RANGE,
 };
 
 const struct inode_operations ext4_file_inode_operations = {
Index: linux-2.6/fs/f2fs/file.c
===================================================================
--- linux-2.6.orig/fs/f2fs/file.c
+++ linux-2.6/fs/f2fs/file.c
@@ -4499,6 +4499,9 @@ const struct file_operations f2fs_file_o
 	.flush		= f2fs_file_flush,
 	.fsync		= f2fs_sync_file,
 	.fallocate	= f2fs_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_KEEP_SIZE |
+		FALLOC_FL_PUNCH_HOLE | FALLOC_FL_COLLAPSE_RANGE |
+		FALLOC_FL_ZERO_RANGE | FALLOC_FL_INSERT_RANGE,
 	.unlocked_ioctl	= f2fs_ioctl,
 #ifdef CONFIG_COMPAT
 	.compat_ioctl	= f2fs_compat_ioctl,
Index: linux-2.6/fs/fat/file.c
===================================================================
--- linux-2.6.orig/fs/fat/file.c
+++ linux-2.6/fs/fat/file.c
@@ -211,6 +211,7 @@ const struct file_operations fat_file_op
 	.splice_read	= generic_file_splice_read,
 	.splice_write	= iter_file_splice_write,
 	.fallocate	= fat_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_KEEP_SIZE,
 };
 
 static int fat_cont_expand(struct inode *inode, loff_t size)
Index: linux-2.6/fs/fuse/file.c
===================================================================
--- linux-2.6.orig/fs/fuse/file.c
+++ linux-2.6/fs/fuse/file.c
@@ -3147,6 +3147,8 @@ static const struct file_operations fuse
 	.compat_ioctl	= fuse_file_compat_ioctl,
 	.poll		= fuse_file_poll,
 	.fallocate	= fuse_file_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_KEEP_SIZE |
+		FALLOC_FL_PUNCH_HOLE | FALLOC_FL_ZERO_RANGE,
 	.copy_file_range = fuse_copy_file_range,
 };
 
Index: linux-2.6/fs/gfs2/file.c
===================================================================
--- linux-2.6.orig/fs/gfs2/file.c
+++ linux-2.6/fs/gfs2/file.c
@@ -1366,6 +1366,7 @@ const struct file_operations gfs2_file_f
 	.splice_write	= gfs2_file_splice_write,
 	.setlease	= simple_nosetlease,
 	.fallocate	= gfs2_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
 };
 
 const struct file_operations gfs2_dir_fops = {
Index: linux-2.6/fs/hugetlbfs/inode.c
===================================================================
--- linux-2.6.orig/fs/hugetlbfs/inode.c
+++ linux-2.6/fs/hugetlbfs/inode.c
@@ -1163,6 +1163,7 @@ const struct file_operations hugetlbfs_f
 	.get_unmapped_area	= hugetlb_get_unmapped_area,
 	.llseek			= default_llseek,
 	.fallocate		= hugetlbfs_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE,
 };
 
 static const struct inode_operations hugetlbfs_dir_inode_operations = {
Index: linux-2.6/fs/nfs/nfs4file.c
===================================================================
--- linux-2.6.orig/fs/nfs/nfs4file.c
+++ linux-2.6/fs/nfs/nfs4file.c
@@ -457,6 +457,7 @@ const struct file_operations nfs4_file_o
 	.copy_file_range = nfs4_copy_file_range,
 	.llseek		= nfs4_file_llseek,
 	.fallocate	= nfs42_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
 	.remap_file_range = nfs42_remap_file_range,
 #else
 	.llseek		= nfs_file_llseek,
Index: linux-2.6/fs/ntfs3/file.c
===================================================================
--- linux-2.6.orig/fs/ntfs3/file.c
+++ linux-2.6/fs/ntfs3/file.c
@@ -1246,6 +1246,8 @@ const struct file_operations ntfs_file_o
 	.fsync		= generic_file_fsync,
 	.splice_write	= iter_file_splice_write,
 	.fallocate	= ntfs_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_KEEP_SIZE |
+		FALLOC_FL_PUNCH_HOLE | FALLOC_FL_COLLAPSE_RANGE,
 	.release	= ntfs_file_release,
 };
 // clang-format on
Index: linux-2.6/fs/ocfs2/file.c
===================================================================
--- linux-2.6.orig/fs/ocfs2/file.c
+++ linux-2.6/fs/ocfs2/file.c
@@ -2746,6 +2746,7 @@ const struct file_operations ocfs2_fops
 	.splice_read	= generic_file_splice_read,
 	.splice_write	= iter_file_splice_write,
 	.fallocate	= ocfs2_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE,
 	.remap_file_range = ocfs2_remap_file_range,
 };
 
@@ -2792,6 +2793,7 @@ const struct file_operations ocfs2_fops_
 	.splice_read	= generic_file_splice_read,
 	.splice_write	= iter_file_splice_write,
 	.fallocate	= ocfs2_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE,
 	.remap_file_range = ocfs2_remap_file_range,
 };
 
Index: linux-2.6/fs/overlayfs/file.c
===================================================================
--- linux-2.6.orig/fs/overlayfs/file.c
+++ linux-2.6/fs/overlayfs/file.c
@@ -13,6 +13,7 @@
 #include <linux/security.h>
 #include <linux/mm.h>
 #include <linux/fs.h>
+#include <linux/falloc.h>
 #include "overlayfs.h"
 
 struct ovl_aio_req {
@@ -658,6 +659,7 @@ const struct file_operations ovl_file_op
 	.fsync		= ovl_fsync,
 	.mmap		= ovl_mmap,
 	.fallocate	= ovl_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_SUPPORTED_MASK,
 	.fadvise	= ovl_fadvise,
 	.flush		= ovl_flush,
 	.splice_read    = generic_file_splice_read,
Index: linux-2.6/fs/xfs/xfs_file.c
===================================================================
--- linux-2.6.orig/fs/xfs/xfs_file.c
+++ linux-2.6/fs/xfs/xfs_file.c
@@ -1464,6 +1464,7 @@ const struct file_operations xfs_file_op
 	.fsync		= xfs_file_fsync,
 	.get_unmapped_area = thp_get_unmapped_area,
 	.fallocate	= xfs_file_fallocate,
+	.fallocate_supported_flags = XFS_FALLOC_FL_SUPPORTED,
 	.fadvise	= xfs_file_fadvise,
 	.remap_file_range = xfs_file_remap_range,
 };
Index: linux-2.6/include/linux/fs.h
===================================================================
--- linux-2.6.orig/include/linux/fs.h
+++ linux-2.6/include/linux/fs.h
@@ -2098,6 +2098,7 @@ struct file_operations {
 	int (*setlease)(struct file *, long, struct file_lock **, void **);
 	long (*fallocate)(struct file *file, int mode, loff_t offset,
 			  loff_t len);
+	unsigned fallocate_supported_flags;
 	void (*show_fdinfo)(struct seq_file *m, struct file *f);
 #ifndef CONFIG_MMU
 	unsigned (*mmap_capabilities)(struct file *);
Index: linux-2.6/ipc/shm.c
===================================================================
--- linux-2.6.orig/ipc/shm.c
+++ linux-2.6/ipc/shm.c
@@ -44,6 +44,7 @@
 #include <linux/mount.h>
 #include <linux/ipc_namespace.h>
 #include <linux/rhashtable.h>
+#include <linux/falloc.h>
 
 #include <linux/uaccess.h>
 
@@ -558,6 +559,7 @@ static const struct file_operations shm_
 	.get_unmapped_area	= shm_get_unmapped_area,
 	.llseek		= noop_llseek,
 	.fallocate	= shm_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE,
 };
 
 /*
@@ -571,6 +573,7 @@ static const struct file_operations shm_
 	.get_unmapped_area	= shm_get_unmapped_area,
 	.llseek		= noop_llseek,
 	.fallocate	= shm_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE,
 };
 
 bool is_file_shm_hugepages(struct file *file)
Index: linux-2.6/mm/shmem.c
===================================================================
--- linux-2.6.orig/mm/shmem.c
+++ linux-2.6/mm/shmem.c
@@ -3797,6 +3797,7 @@ static const struct file_operations shme
 	.splice_read	= generic_file_splice_read,
 	.splice_write	= iter_file_splice_write,
 	.fallocate	= shmem_fallocate,
+	.fallocate_supported_flags = FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE,
 #endif
 };
 
Index: linux-2.6/drivers/block/loop.c
===================================================================
--- linux-2.6.orig/drivers/block/loop.c
+++ linux-2.6/drivers/block/loop.c
@@ -947,7 +947,10 @@ static void loop_config_discard(struct l
 	 * encryption is enabled, because it may give an attacker
 	 * useful information.
 	 */
-	} else if (!file->f_op->fallocate || lo->lo_encrypt_key_size) {
+	} else if ((file->f_op->fallocate_supported_flags &
+			(FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE)) !=
+			(FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE) ||
+		   lo->lo_encrypt_key_size) {
 		max_discard_sectors = 0;
 		granularity = 0;
 
@@ -959,7 +962,10 @@ static void loop_config_discard(struct l
 	if (max_discard_sectors) {
 		q->limits.discard_granularity = granularity;
 		blk_queue_max_discard_sectors(q, max_discard_sectors);
-		blk_queue_max_write_zeroes_sectors(q, max_discard_sectors);
+		if (file->f_op->fallocate_supported_flags & FALLOC_FL_ZERO_RANGE)
+			blk_queue_max_write_zeroes_sectors(q, max_discard_sectors);
+		else
+			blk_queue_max_write_zeroes_sectors(q, 0);
 		blk_queue_flag_set(QUEUE_FLAG_DISCARD, q);
 	} else {
 		q->limits.discard_granularity = 0;


^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH v4] loop: don't print warnings if the underlying filesystem doesn't support discard
  2021-10-13  9:28         ` [PATCH v4] " Mikulas Patocka
@ 2021-10-27  5:02           ` Dave Chinner
  2021-10-27  8:28             ` Mikulas Patocka
  0 siblings, 1 reply; 12+ messages in thread
From: Dave Chinner @ 2021-10-27  5:02 UTC (permalink / raw)
  To: Mikulas Patocka
  Cc: Christoph Hellwig, Jens Axboe, Ming Lei, Zdenek Kabelac,
	linux-block, dm-devel, linux-fsdevel

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

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH v4] loop: don't print warnings if the underlying filesystem doesn't support discard
  2021-10-27  5:02           ` Dave Chinner
@ 2021-10-27  8:28             ` Mikulas Patocka
  2021-10-28  4:15               ` Dave Chinner
  0 siblings, 1 reply; 12+ messages in thread
From: Mikulas Patocka @ 2021-10-27  8:28 UTC (permalink / raw)
  To: Dave Chinner
  Cc: Christoph Hellwig, Jens Axboe, Ming Lei, Zdenek Kabelac,
	linux-block, dm-devel, linux-fsdevel



On Wed, 27 Oct 2021, Dave Chinner wrote:

> 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;

I did this (see 
https://lore.kernel.org/all/alpine.LRH.2.02.2109231539520.27863@file01.intranet.prod.int.rdu2.redhat.com/ 
) and Christoph Hellwig asked for a flag in the file_operations structure 
( https://lore.kernel.org/all/20210924155822.GA10064@lst.de/ ).

Mikulas

> 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
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH v4] loop: don't print warnings if the underlying filesystem doesn't support discard
  2021-10-27  8:28             ` Mikulas Patocka
@ 2021-10-28  4:15               ` Dave Chinner
  0 siblings, 0 replies; 12+ messages in thread
From: Dave Chinner @ 2021-10-28  4:15 UTC (permalink / raw)
  To: Mikulas Patocka
  Cc: Christoph Hellwig, Jens Axboe, Ming Lei, Zdenek Kabelac,
	linux-block, dm-devel, linux-fsdevel

On Wed, Oct 27, 2021 at 04:28:03AM -0400, Mikulas Patocka wrote:
> 
> 
> On Wed, 27 Oct 2021, Dave Chinner wrote:
> 
> > 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;
> 
> I did this (see 
> https://lore.kernel.org/all/alpine.LRH.2.02.2109231539520.27863@file01.intranet.prod.int.rdu2.redhat.com/ 

Ok, you need to keep a changelog with the patch so that it's clear
what the history of it is....

> ) and Christoph Hellwig asked for a flag in the file_operations structure 
> ( https://lore.kernel.org/all/20210924155822.GA10064@lst.de/ ).

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

^ permalink raw reply	[flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-10-28  4:15 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-23 19:48 [PATCH] loop: don't print warnings if the underlying filesystem doesn't support discard Mikulas Patocka
2021-09-24  0:17 ` Ming Lei
2021-09-24 15:58 ` Christoph Hellwig
2021-10-04 13:01   ` Mikulas Patocka
2021-10-12  6:20     ` Christoph Hellwig
2021-10-12 20:25       ` [PATCH v3] " Mikulas Patocka
2021-10-13  2:56         ` kernel test robot
2021-10-13  5:06         ` kernel test robot
2021-10-13  9:28         ` [PATCH v4] " Mikulas Patocka
2021-10-27  5:02           ` Dave Chinner
2021-10-27  8:28             ` Mikulas Patocka
2021-10-28  4:15               ` Dave Chinner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).