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 $mntThe 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.
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,
Josef
