xfs_alloc_file_space() rounds len independently of offset

[Max Reitz <mreitz@xxxxxxxxxx>]

Hi,

I’ve noticed that fallocating some range on XFS sometimes does not
include the last block covered by the range, when the start offset is
unaligned.

(Tested on 5.3.0-gf41def397.)

This happens whenever ceil((offset + len) / block_size) - floor(offset /
block_size) > ceil(len / block_size), for example:

Let block_size be 4096.  Then (on XFS):

$ fallocate -o 2048 -l 4096 foo   # Range [2048, 6144)
$ xfs_bmap foo
foo:
        0: [0..7]: 80..87
        1: [8..15]: hole

There should not be a hole there.  Both of the first two blocks should
be allocated.  XFS will do that if I just let the range start one byte
sooner and increase the length by one byte:

$ rm -f foo
$ fallocate -o 2047 -l 4097 foo   # Range [2047, 6144)
$ xfs_bmap foo
foo:
        0: [0..15]: 88..103


(See [1] for a more extensive reasoning why this is a bug.)


The problem is (as far as I can see) that xfs_alloc_file_space() rounds
count (which equals len) independently of the offset.  So in the
examples above, 4096 is rounded to one block and 4097 is rounded to two;
even though the first example actually touches two blocks because of the
misaligned offset.

Therefore, this should fix the problem (and does fix it for me):

diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index 0910cb75b..4f4437030 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -864,6 +864,7 @@ xfs_alloc_file_space(
 	xfs_filblks_t		allocatesize_fsb;
 	xfs_extlen_t		extsz, temp;
 	xfs_fileoff_t		startoffset_fsb;
+	xfs_fileoff_t		endoffset_fsb;
 	int			nimaps;
 	int			quota_flag;
 	int			rt;
@@ -891,7 +892,8 @@ xfs_alloc_file_space(
 	imapp = &imaps[0];
 	nimaps = 1;
 	startoffset_fsb	= XFS_B_TO_FSBT(mp, offset);
-	allocatesize_fsb = XFS_B_TO_FSB(mp, count);
+	endoffset_fsb = XFS_B_TO_FSB(mp, offset + count);
+	allocatesize_fsb = endoffset_fsb - startoffset_fsb;

 	/*
 	 * Allocate file space until done or until there is an error


Thanks and kind regards,

Max


[1] That this is a bug can be proven as follows:

1. The fallocate(2) man page states "subsequent writes into the range
specified by offset and len are guaranteed not to fail because of lack
of disk space."

2. Run this test (anywhere, e.g. tmpfs):

$ truncate -s $((4096 * 4096)) test_fs
$ mkfs.xfs -b size=4096 test_fs
[Success-indicating output, I hope]

$ mkdir mount_point
$ sudo mount -o loop test_fs mount_point
$ sudo chmod go+rwx mount_point
$ cd mount_point

$ free_blocks=$(df -B4k . | tail -n 1 \
      | awk '{ split($0, f); print f[4] }')

$ falloc_length=$((free_blocks * 4096))

$ while true; do \
     fallocate -o 2048 -l $falloc_length test_file && break; \
     falloc_length=$((falloc_length - 4096)); \
done
fallocate: fallocate failed: No space left on device
fallocate: fallocate failed: No space left on device
fallocate: fallocate failed: No space left on device
fallocate: fallocate failed: No space left on device

  # Now we have a test_file with an fallocated range of
  # [2048, 2048 + $falloc_length)
  # So we should be able to write anywhere in that area without
  # encountering ENOSPC; but that is what happens when we write
  # to the last block covered by the range:

$ dd if=/dev/zero of=test_file bs=1 conv=notrunc \
    seek=$falloc_length count=2048
dd: error writing 'test_file': No space left on device
1+0 records in
0+0 records out
0 bytes copied, 0.000164691 s, 0.0 kB/s


When I apply the diff shown above, I get one more “No space left on
device” line (indicating that fallocate consistently takes one
additional block), and then:

$ uname -sr
Linux 5.3.0-gf41def397-dirty

$ dd if=/dev/zero of=test_file bs=1 conv=notrunc \
    seek=$falloc_length count=2048
2048+0 records in
2048+0 records out
2048 bytes (2.0 kB, 2.0 KiB) copied, 0.0121903 s, 168 kB/s

(i.e., what I’d expect)