loop: fix min directio size detection for nested loop devices

Commit Message

Darrick J. Wong April 17, 2025, 3:46 a.m. UTC

From: Darrick J. Wong <[e-mail]>

fstest generic/563 sets up a loop device in front of the SCRATCH_DEV
block device to test cgroup I/O accounting, because SCRATCH_DEV could be
a partition and cgroup accounting apparently only works on full block
devices (e.g. sda, not sda1).

If however SCRATCH_DEV is itself a loop device that we've used to
simulate 4k LBA block devices, the minimum directio size discovery
introduced in commit f4774e92aab85d is wrong -- we should query the
logical block size of the underlying block device because file->f_path
points whatever filesystem /dev is.

Otherwise, you get a weird losetup config:

$ losetup -f /dev/sda
$ losetup --sector-size 4096 /dev/loop0
$ losetup -f /dev/loop0
$ losetup --raw
NAME SIZELIMIT OFFSET AUTOCLEAR RO BACK-FILE DIO LOG-SEC
/dev/loop0 0 0 0 0 /dev/sda 1 4096
/dev/loop1 0 0 0 0 /dev/loop0 1 512

(Note loop1 can try to send 512b writes to loop0 which has a sector size
of 4k)

and mkfs failures like this:

error reading existing superblock: Invalid argument
mkfs.xfs: pwrite failed: Invalid argument
libxfs_bwrite: write failed on (unknown) bno 0x42a3ef8/0x100, err=22
mkfs.xfs: Releasing dirty buffer to free list!
found dirty buffer (bulk) on free list!
mkfs.xfs: pwrite failed: Invalid argument
libxfs_bwrite: write failed on (unknown) bno 0x0/0x100, err=22

Cc: <[e-mail]> # v6.15-rc1
Fixes: f4774e92aab85d ("loop: take the file system minimum dio alignment into account")
Signed-off-by: "Darrick J. Wong" <[e-mail]>
---
 drivers/block/loop.c |   10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

Comments

Christoph Hellwig April 17, 2025, 6:25 a.m. UTC | #1

On Wed, Apr 16, 2025 at 08:46:39PM -0700, Darrick J. Wong wrote:
> If however SCRATCH_DEV is itself a loop device that we've used to
> simulate 4k LBA block devices, the minimum directio size discovery
> introduced in commit f4774e92aab85d is wrong -- we should query the
> logical block size of the underlying block device because file->f_path
> points whatever filesystem /dev is.

No, the problem is that special handling of block devices in stat
sits in vfs_statx_path and not the exported vfs_getattr helper,
and thus we don't call into bdev_statx which would return the right
value.  I'll send a separate VFS-level fix for this.

Patch

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 174e67ac729f3d..59d3e713c574b0 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -443,9 +443,17 @@  static void loop_reread_partitions(struct loop_device *lo)
 static unsigned int loop_query_min_dio_size(struct loop_device *lo)
 {
 	struct file *file = lo->lo_backing_file;
-	struct block_device *sb_bdev = file->f_mapping->host->i_sb->s_bdev;
+	struct inode *inode = file->f_mapping->host;
+	struct block_device *sb_bdev = inode->i_sb->s_bdev;
 	struct kstat st;
 
+	/*
+	 * If the backing device is a block device, don't send directios
+	 * smaller than its LBA size.
+	 */
+	if (S_ISBLK(inode->i_mode))
+		return bdev_logical_block_size(I_BDEV(inode));
+
 	/*
 	 * Use the minimal dio alignment of the file system if provided.
 	 */