On 2020/12/17 下午10:51, Josef Bacik wrote:
On 12/16/20 11:57 PM, Qu Wenruo wrote:
[BUG]
With current subpage RW patchset, the following script can lead to
filesystem hang:
# mkfs.btrfs -f -s 4k $dev
# mount $dev -o nospace_cache $mnt
# fsstress -w -n 100 -p 1 -s 1608140256 -v -d $mnt
The file system will hang at wait_event() of
btrfs_start_ordered_extent().
[CAUSE]
The root cause is, btrfs_invalidatepage() is freeing page::private which
still has subpage dirty bit set.
The offending situation happens like this:
btrfs_fllocate()
|- btrfs_zero_range()
|- btrfs_punch_hole_lock_range()
|- truncate_pagecache_range()
|- btrfs_invalidatepage()
The involved range looks like:
0 32K 64K 96K 128K
|///////||//////|
| Range to drop |
For the [32K, 64K) range, since the offset is 32K, the page won't be
invalidated.
But for the [64K, 96K) range, the offset is 0, current
btrfs_invalidatepage() will call clear_page_extent_mapped() which will
detach page::private, making the subpage dirty bitmap being cleared.
This prevents later __extent_writepage_io() to locate any range to
write, thus no way to wake up the ordered extents.
[FIX]
To fix the problem this patch will:
- Only clear page status and detach page private when the full page
is invalidated
- Change how we handle unfinished ordered extent
If there is any ordered extent unfinished in the page range, we can't
call clear_extent_bit() with delete == true.
[REASON FOR RFC]
There is still uncertainty around the btrfs_releasepage() call.
1. Why we need btrfs_releasepage() call for non-full-page condition?
Other fs (aka. xfs) just exit without doing special handling if
invalidatepage() is called with part of the page.
Thus I didn't completely understand why btrfs_releasepage() here is
needed for non-full page call.
2. Why "if (offset)" is not causing problem for current code?
This existing if (offset) call can be skipped for cases like
offset == 0 length == 2K.
As MM layer can call invalidatepage() with unaligned offset/length,
for cases like truncate_inode_pages_range().
This will make btrfs_invalidatepage() to truncate the whole page when
we only need to zero part of the page.
Are we ever calling with a different length when pagesize ==
sectorsize? That's probably why it works fine now.
The range passed in can be unaligned at all.
MM layer functions like truncate_inode_pages_range() relies on that.
That's why I'm wondering why the current code is working.
As for start == 0 and length != PAGE_SIZE case it may clear the
Private2/Checked bit unintentionally.
Or is that CoW fixup saving the problem?
But I think we should follow what all the other file systems do, if len
!= PAGE_SIZE || offset != 0 then just skip it, that would probably be
easier and work for you as well? Thanks,
Definitely it would work for me.
Thanks,
Qu
Josef
