mirror of
https://github.com/torvalds/linux.git
synced 2026-07-28 01:55:51 +02:00
btrfs: fix a regression where PAGECACHE_TAG_DIRTY is never cleared
[BUG] The following script (already submitted as generic/798) will report incorrect dirty page numbers, with 64K page size systems and 4K fs block size: # mkfs.btrfs -s 4k -f $dev # mount $dev $mnt # xfs_io -f -c "pwrite 0 64K" -c fsync -c "cachestat 0 64K" $mnt/foobar Cached: 1, Dirty: 1, Writeback: 0, Evicted: 0, Recently Evicted: 0 Note that the dirtied page number is still 1. [CAUSE] The cachestat() goes through the XArray of the page cache, but instead of checking each folio's flag, it uses the PAGECACHE_TAG_DIRTY tag to report dirty pages. Since commit095be159f3("btrfs: unify folio dirty flag clearing"), btrfs replaced a folio_clear_dirty_for_io() call inside extent_write_cache_pages() with folio_test_dirty(). This will cause the following call sequence for the folio at file offset 0: extent_write_cache_pages() |- folio_test_dirty() | The folio is still dirty, continue to writeback. | |- extent_writepage() |- extent_writepage_io() |- submit_one_sector() for range [0, 4K) | |- btrfs_folio_clear_dirty() | |- btrfs_folio_set_writeback() | |- folio_start_writeback() | It's the first writeback block, we set the writeback | flag for the folio. | But the folio is still dirty, PAGECACHE_TAG_DIRTY is | kept | |- submit_one_sector() for range [4K, 8K) | |- btrfs_folio_clear_dirty() | |- btrfs_folio_set_writeback() | The folio already has writeback flag, no need to call | folio_start_writeback() | | ... |- submit_one_sector() for range [60K, 64K) |- btrfs_folio_clear_dirty() |- btrfs_folio_set_writeback() The folio already has writeback flag, no need to call folio_start_writeback() So the PAGECACHE_TAG_DIRTY is never cleared. Meanwhile for the old code, before that commit, the sequence looks like: extent_write_cache_pages() |- folio_clear_dirty_for_io() | The folio is still dirty, so continue to writeback. | But the folio dirty flag is cleared now. | |- extent_writepage() |- extent_writepage_io() |- submit_one_sector() for range [0, 4K) | |- btrfs_folio_clear_dirty() | |- btrfs_folio_set_writeback() | |- folio_start_writeback() | |- xas_clear(PAGECACHE_TAG) | | It's the first writeback block, we set the writeback | flag for the folio. | And the folio is not dirty, PAGECACHE_TAG_DIRTY is | cleared | |- submit_one_sector() for range [4K, 8K) | |- btrfs_folio_clear_dirty() | |- btrfs_folio_set_writeback() | The folio already has writeback flag, no need to call | folio_start_writeback() | | ... |- submit_one_sector() for range [60K, 64K) |- btrfs_folio_clear_dirty() |- btrfs_folio_set_writeback() The folio already has writeback flag, no need to call folio_start_writeback() Unlike the new code, old code will clear PAGECACHE_TAG_DIRTY for the first writeback block. There is a deeper problem, dirty and writeback folio flags are updated at very different timing. The dirty flag is only cleared when the last sub-folio block has dirty flag cleared. But the writeback flag is set when the first block starts writeback, and later blocks that go through writeback will not call folio_start_writeback() again. If we rely on folio_start_writeback() to update the PAGECACHE_TAG_DIRTY and PAGECACHE_TAG_TOWRITE, it will always be incorrect in one way or another. [FIX] Do not let folio_start_writeback() do any PAGECACHE_TAG_TOWRITE handling. Instead, manually clear both PAGECACHE_TAG_TOWRITE and PAGECACHE_TAG_DIRTY flags when the folio is no longer dirty during btrfs_subpage_set_writeback(). However this is only a hot-fix, for the long term solution we will follow iomap, by calling folio_start_writeback() immediately for the whole folio, and folio_end_writeback() after all writeback finished for the folio. Fixes:095be159f3("btrfs: unify folio dirty flag clearing") Reviewed-by: Boris Burkov <boris@bur.io> Signed-off-by: Qu Wenruo <wqu@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
This commit is contained in:
parent
5eff4d5b17
commit
9b73625a4f
|
|
@ -359,6 +359,23 @@ void btrfs_subpage_set_dirty(const struct btrfs_fs_info *fs_info,
|
|||
folio_mark_dirty(folio);
|
||||
}
|
||||
|
||||
static void folio_clear_tags(struct folio *folio)
|
||||
{
|
||||
struct address_space *mapping = folio_mapping(folio);
|
||||
XA_STATE(xas, &mapping->i_pages, folio->index);
|
||||
unsigned long flags;
|
||||
|
||||
ASSERT(folio_test_locked(folio));
|
||||
ASSERT(mapping);
|
||||
ASSERT(mapping_use_writeback_tags(mapping));
|
||||
|
||||
xas_lock_irqsave(&xas, flags);
|
||||
xas_load(&xas);
|
||||
xas_clear_mark(&xas, PAGECACHE_TAG_DIRTY);
|
||||
xas_clear_mark(&xas, PAGECACHE_TAG_TOWRITE);
|
||||
xas_unlock_irqrestore(&xas, flags);
|
||||
}
|
||||
|
||||
/*
|
||||
* Extra clear_and_test function for subpage dirty bitmap.
|
||||
*
|
||||
|
|
@ -403,7 +420,6 @@ void btrfs_subpage_set_writeback(const struct btrfs_fs_info *fs_info,
|
|||
unsigned int start_bit = subpage_calc_start_bit(fs_info, folio,
|
||||
writeback, start, len);
|
||||
unsigned long flags;
|
||||
bool keep_write;
|
||||
|
||||
spin_lock_irqsave(&bfs->lock, flags);
|
||||
bitmap_set(bfs->bitmaps, start_bit, len >> fs_info->sectorsize_bits);
|
||||
|
|
@ -413,10 +429,14 @@ void btrfs_subpage_set_writeback(const struct btrfs_fs_info *fs_info,
|
|||
* folio. Doing so can cause WB_SYNC_ALL writepages() to overlook it,
|
||||
* assume writeback is complete, and exit too early — violating sync
|
||||
* ordering guarantees.
|
||||
*
|
||||
* Instead we manually clear the DIRTY and TOWRITE tags after the folio
|
||||
* is no longer dirty.
|
||||
*/
|
||||
keep_write = folio_test_dirty(folio);
|
||||
if (!folio_test_writeback(folio))
|
||||
__folio_start_writeback(folio, keep_write);
|
||||
__folio_start_writeback(folio, true);
|
||||
if (!folio_test_dirty(folio))
|
||||
folio_clear_tags(folio);
|
||||
spin_unlock_irqrestore(&bfs->lock, flags);
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user