On 26.09.19 14:59, Brian Foster wrote: > On Thu, Sep 26, 2019 at 12:57:49PM +0200, Max Reitz wrote: >> 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 >> > > That looks like a reasonable fix to me and it's in the spirit of how > xfs_free_file_space() works as well (outside of the obvious difference > in how unaligned boundary blocks are handled). Care to send a proper > patch? I’ve never sent a kernel patch before, but I’ll give it a go. Max
Attachment:
signature.asc
Description: OpenPGP digital signature
