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 <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 5e7555732f16c0a97960af04552d2934cb8ac79f..9593996bfff43303f5e7e8f6b73b787429c6ba2b 100644 (file)
--- 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 e4c2c1e01b7deb4803284d6641674631031c466e..899db1215602c76a4794c01bae01669032ff8c63 100644 (file)
--- 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 201010ced09df3916206d923136af66c76c33919..c70e541d5d9abc5a59fe153943df06f546e918d6 100644 (file)
--- 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