From: Qu Wenruo Date: Thu, 7 May 2026 05:29:18 +0000 (+0930) Subject: btrfs: unify folio dirty flag clearing X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;h=095be159f3eb4670fab75f795ce9539a381ebd3f;p=thirdparty%2Flinux.git btrfs: unify folio dirty flag clearing Currently during folio writeback, we call folio_clear_dirty_for_io() before extent_writepage(), which causes folio dirty flag to be cleared, but without touching the subpage bitmaps. This works fine for the bio submission path, as we always call btrfs_folio_clear_dirty() to clear the subpage bitmap. But this is far from consistent, thus this patch is going to unify the behavior to always use btrfs_folio_clear_dirty() helper to clear both folio flag and subpage bitmap. This involves: - Replace folio_clear_dirty_for_io() with folio_test_dirty() There is only one call site calling folio_clear_dirty_for_io() outside of subpage.c, that's inside extent_write_cache_pages() just before extent_writepage(). - Make btrfs_invalidate_folio() clear dirty range for the whole folio The function btrfs_invalidate_folio() is also called during extent_writepage(). If we had a folio completely beyond isize, we call folio_invalidate() -> btrfs_invalidate_folio() to free the folio. Since we no longer have folio_clear_dirty_for_io() to clear the folio dirty flag, we must manually clear the folio dirty flag for the to-be-invalidated folio, and also clear the PAGECACHE_TAG_DIRTY tag. The tag clearing is done using a new helper, btrfs_clear_folio_dirty_tag(), which is almost the same as the old btree_clear_folio_dirty_tag(), but with minor improvements including: * Remove the folio_test_dirty() check We have already done an ASSERT(). * Add an ASSERT() to make sure folio is mapped - Add extra ASSERT()s before clearing folio private During development I hit dirty folios without the private flag set, and that caused a lot of ASSERT()s. The reason is that btrfs_invalidate_folio() is relying on the dirty flag being cleared when it's called from extent_writepage(). Add extra ASSERT()s inside clear_folio_extent_mapped() to catch wild dirty/writeback flags. Signed-off-by: Qu Wenruo Signed-off-by: David Sterba --- diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c index 5e7555732f16c..9593996bfff43 100644 --- a/fs/btrfs/extent_io.c +++ b/fs/btrfs/extent_io.c @@ -957,6 +957,17 @@ void clear_folio_extent_mapped(struct folio *folio) struct btrfs_fs_info *fs_info; ASSERT(folio->mapping); + /* + * The folio should not have writeback nor dirty flag set. + * + * If dirty flag is set, the folio can be written back again and we + * expect the private flag set for the folio. + * + * If writeback flag is set, the endio may need to utilize the + * private for btrfs_folio_state. + */ + ASSERT(!folio_test_dirty(folio)); + ASSERT(!folio_test_writeback(folio)); if (!folio_test_private(folio)) return; @@ -2572,7 +2583,7 @@ retry: } if (folio_test_writeback(folio) || - !folio_clear_dirty_for_io(folio)) { + !folio_test_dirty(folio)) { folio_unlock(folio); continue; } @@ -3729,17 +3740,6 @@ void free_extent_buffer_stale(struct extent_buffer *eb) release_extent_buffer(eb); } -static void btree_clear_folio_dirty_tag(struct folio *folio) -{ - ASSERT(!folio_test_dirty(folio)); - ASSERT(folio_test_locked(folio)); - xa_lock_irq(&folio->mapping->i_pages); - if (!folio_test_dirty(folio)) - __xa_clear_mark(&folio->mapping->i_pages, folio->index, - PAGECACHE_TAG_DIRTY); - xa_unlock_irq(&folio->mapping->i_pages); -} - void btrfs_clear_buffer_dirty(struct btrfs_trans_handle *trans, struct extent_buffer *eb) { @@ -3780,7 +3780,7 @@ void btrfs_clear_buffer_dirty(struct btrfs_trans_handle *trans, folio_lock(folio); last = btrfs_meta_folio_clear_and_test_dirty(folio, eb); if (last) - btree_clear_folio_dirty_tag(folio); + btrfs_clear_folio_dirty_tag(folio); folio_unlock(folio); } WARN_ON(refcount_read(&eb->refs) == 0); diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h index e4c2c1e01b7de..899db1215602c 100644 --- a/fs/btrfs/extent_io.h +++ b/fs/btrfs/extent_io.h @@ -390,6 +390,17 @@ void extent_clear_unlock_delalloc(struct btrfs_inode *inode, u64 start, u64 end, void btrfs_clear_buffer_dirty(struct btrfs_trans_handle *trans, struct extent_buffer *buf); +static inline void btrfs_clear_folio_dirty_tag(struct folio *folio) +{ + ASSERT(!folio_test_dirty(folio)); + ASSERT(folio_test_locked(folio)); + ASSERT(folio->mapping); + xa_lock_irq(&folio->mapping->i_pages); + __xa_clear_mark(&folio->mapping->i_pages, folio->index, + PAGECACHE_TAG_DIRTY); + xa_unlock_irq(&folio->mapping->i_pages); +} + int btrfs_alloc_page_array(unsigned int nr_pages, struct page **page_array, bool nofail); int btrfs_alloc_folio_array(unsigned int nr_folios, unsigned int order, diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 201010ced09df..c70e541d5d9ab 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -7665,6 +7665,8 @@ next: &cached_state); cur = range_end + 1; } + btrfs_folio_clear_dirty(fs_info, folio, page_start, folio_size(folio)); + btrfs_clear_folio_dirty_tag(folio); /* * We have iterated through all ordered extents of the page, the page * should not have Ordered anymore, or the above iteration