Re: [Patch v2 36/42] btrfs: disable inline extent creation for subpage
From: Qu Wenruo <hidden>
Date: 2021-05-04 04:28:56
On 2021/4/28 上午7:03, Qu Wenruo wrote:
[BUG]
When running the following fsx command (extracted from generic/127) on
subpage btrfs, it can create inline extent with regular extents:
fsx -q -l 262144 -o 65536 -S 191110531 -N 9057 -R -W $mnt/file > /tmp/fsx
The offending extent would look like:
item 9 key (257 INODE_REF 256) itemoff 15703 itemsize 14
index 2 namelen 4 name: file
item 10 key (257 EXTENT_DATA 0) itemoff 14975 itemsize 728
generation 7 type 0 (inline)
inline extent data size 707 ram_bytes 707 compression 0 (none)
item 11 key (257 EXTENT_DATA 4096) itemoff 14922 itemsize 53
generation 7 type 2 (prealloc)
prealloc data disk byte 102346752 nr 4096
prealloc data offset 0 nr 4096
[CAUSE]
For subpage btrfs, the writeback is triggered in page unit, which means,
even if we just want to writeback range [16K, 20K) for 64K page system,
we will still try to writeback any dirty sector of range [0, 64K).
This is never a problem if sectorsize == PAGE_SIZE, but for subpage,
this can cause unexpected problems.
For above test case, the last several operations from fsx are:
9055 trunc from 0x40000 to 0x2c3
9057 falloc from 0x164c to 0x19d2 (0x386 bytes)With more investigation into this specific problem, it turns out it's really something specific to falloc() (and maybe reflink)
In operation 9055, we dirtied sector [0, 4096), then in falloc, we call btrfs_wait_ordered_range(inode, start=4096, len=4096), only expecting to writeback any dirty data in [4096, 8192), but nothing else.
This part still stands.
Unfortunately, in subpage case, above btrfs_wait_ordered_range() will trigger writeback of the range [0, 64K), which includes the data at [0, 4096).
But the problem is really in the sequence of btrfs_wait_ordered_range() and btrfs_cont_expand(). Currently, we call btrfs_cont_expand() first, then btrfs_wait_ordered_range(), which leads to the inline extent then regular extent. But the truth is, if we just call btrfs_wait_ordered_range() then btrfs_cont_expand() we will no longer got the problem. As btrfs_wait_ordered_range() will writeback the first sector as inline, then btrfs_cont_expand() re-dirty the first sector, so that it will be re-written as regular extent, after we enlarge the isize. I also checked reflink, which is doing the same cont_expand() then wait_ordered_extent(). AFAIK we could just change the sequence so we don't need to disable inline extent completely. But I'm not yet 100% sure, thus I'd prefer to make btrfs-check to report such inline + regular layout as an error, then do more tests to make sure it will work as expected. Thanks, Qu
quoted hunk ↗ jump to hunk
And since at the call site, we haven't yet increased i_size, which is still 707, this means cow_file_range() can insert an inline extent. Resulting above inline + regular extent. [WORKAROUND] I don't really have any good short-term solution yet, as this means all operations that would trigger writeback need to be reviewed for any isize change. So here I choose to disable inline extent creation for subpage case as a workaround. We have done tons of work just to avoid such extent, so I don't to create an exception just for subpage. This only affects inline extent creation, btrfs subpage support has no problem reading existing inline extents at all. Signed-off-by: Qu Wenruo <redacted> --- fs/btrfs/inode.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-)diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index fd648f2c0242..a2ac8d6eeba5 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c@@ -663,7 +663,11 @@ static noinline int compress_file_range(struct async_chunk *async_chunk) } } cont: - if (start == 0) { + /* + * Check cow_file_range() for why we don't even try to create + * inline extent for subpage case. + */ + if (start == 0 && fs_info->sectorsize == PAGE_SIZE) { /* lets try to make an inline extent */ if (ret || total_in < actual_end) { /* we didn't compress the entire range, try@@ -1061,7 +1065,17 @@ static noinline int cow_file_range(struct btrfs_inode *inode, inode_should_defrag(inode, start, end, num_bytes, SZ_64K); - if (start == 0) { + /* + * Due to the page size limit, for subpage we can only trigger the + * writeback for the dirty sectors of page, that means data writeback + * is doing more writeback than what we want. + * + * This is especially unexpected for some call sites like fallocate, + * where we only increase isize after everything is done. + * This means we can trigger inline extent even we didn't want. + * So here we skip inline extent creation completely. + */ + if (start == 0 && fs_info->sectorsize == PAGE_SIZE) { /* lets try to make an inline extent */ ret = cow_file_range_inline(inode, start, end, 0, BTRFS_COMPRESS_NONE, NULL);