diff mbox series

btrfs: remove the nr_ret parameter from __extent_writepage_io()

Message ID 248988e6643ee3c3d07e884385f503e9d8ebbcc7.1723598419.git.wqu@suse.com (mailing list archive)
State New, archived
Headers show
Series btrfs: remove the nr_ret parameter from __extent_writepage_io() | expand

Commit Message

Qu Wenruo Aug. 14, 2024, 1:20 a.m. UTC
The parameter @nr_ret is used to tell the caller how many sectors have
been submitted for IO.

Then callers check @nr_ret value to determine if we need to manually
clear the PAGECACHE_TAG_DIRTY, as if we submitted no sector (e.g. all
sectors are beyond i_size) there is no folio_clear_writeback() called thus
PAGECACHE_TAG_DIRTY tag will not be cleared.

Remove this parameter by:

- Moving the btrfs_folio_clear_writeback() call into
  __extent_writepage_io()
  So that if we didn't submit any IO, then manually call
  btrfs_folio_set_writeback() to clear PAGECACHE_TAG_DIRTY when
  the page is no longer dirty.

- Use a bool to record if we have submitted any sector
  Instead of an int.

- Use subpage compatible helpers to end folio writeback.
  This brings no change to the behavior, just for the sake of consistency.

  As for the call site inside __extent_writepage(), we're always called
  for the whole page, so the existing full page helper
  folio_(start|end)_writeback() is totally fine.

  For the call site inside extent_write_locked_range(), although we can
  have subpage range, folio_start_writeback() will only clear
  PAGECACHE_TAG_DIRTY if the page is no longer dirty, and the full folio
  will still be dirty if there is any subpage dirty range.
  Only when the last dirty subpage sector is cleared, the
  folio_start_writeback() will clear PAGECACHE_TAG_DIRTY.

  So no matter if we call the full page or subpage helper, the result
  is still the same, then just use the subpage helpers for consistency.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/extent_io.c | 41 +++++++++++++++++------------------------
 1 file changed, 17 insertions(+), 24 deletions(-)

Comments

David Sterba Aug. 19, 2024, 7:14 p.m. UTC | #1
On Wed, Aug 14, 2024 at 10:50:21AM +0930, Qu Wenruo wrote:
> The parameter @nr_ret is used to tell the caller how many sectors have
> been submitted for IO.
> 
> Then callers check @nr_ret value to determine if we need to manually
> clear the PAGECACHE_TAG_DIRTY, as if we submitted no sector (e.g. all
> sectors are beyond i_size) there is no folio_clear_writeback() called thus
> PAGECACHE_TAG_DIRTY tag will not be cleared.
> 
> Remove this parameter by:
> 
> - Moving the btrfs_folio_clear_writeback() call into
>   __extent_writepage_io()
>   So that if we didn't submit any IO, then manually call
>   btrfs_folio_set_writeback() to clear PAGECACHE_TAG_DIRTY when
>   the page is no longer dirty.
> 
> - Use a bool to record if we have submitted any sector
>   Instead of an int.
> 
> - Use subpage compatible helpers to end folio writeback.
>   This brings no change to the behavior, just for the sake of consistency.
> 
>   As for the call site inside __extent_writepage(), we're always called
>   for the whole page, so the existing full page helper
>   folio_(start|end)_writeback() is totally fine.
> 
>   For the call site inside extent_write_locked_range(), although we can
>   have subpage range, folio_start_writeback() will only clear
>   PAGECACHE_TAG_DIRTY if the page is no longer dirty, and the full folio
>   will still be dirty if there is any subpage dirty range.
>   Only when the last dirty subpage sector is cleared, the
>   folio_start_writeback() will clear PAGECACHE_TAG_DIRTY.
> 
>   So no matter if we call the full page or subpage helper, the result
>   is still the same, then just use the subpage helpers for consistency.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Reviewed-by: David Sterba <dsterba@suse.com>
diff mbox series

Patch

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 822e2bf8bc99..b04e0aa97ebd 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -1409,7 +1409,7 @@  static noinline_for_stack int __extent_writepage_io(struct btrfs_inode *inode,
 						    struct folio *folio,
 						    u64 start, u32 len,
 						    struct btrfs_bio_ctrl *bio_ctrl,
-						    loff_t i_size, int *nr_ret)
+						    loff_t i_size)
 {
 	struct btrfs_fs_info *fs_info = inode->root->fs_info;
 	unsigned long range_bitmap = 0;
@@ -1422,11 +1422,11 @@  static noinline_for_stack int __extent_writepage_io(struct btrfs_inode *inode,
 	 */
 	unsigned long dirty_bitmap = 1;
 	unsigned int bitmap_size = 1;
+	bool submitted_io = false;
 	const u64 folio_start = folio_pos(folio);
 	u64 cur;
 	int bit;
 	int ret = 0;
-	int nr = 0;
 
 	ASSERT(start >= folio_start &&
 	       start + len <= folio_start + folio_size(folio));
@@ -1470,20 +1470,24 @@  static noinline_for_stack int __extent_writepage_io(struct btrfs_inode *inode,
 		}
 		ret = submit_one_sector(inode, folio, cur, bio_ctrl, i_size);
 		if (ret < 0)
-			goto out_error;
-		nr++;
+			goto out;
+		submitted_io = true;
 	}
 
 	btrfs_folio_assert_not_dirty(fs_info, folio, start, len);
-	*nr_ret = nr;
-	return 0;
-
-out_error:
+out:
 	/*
-	 * If we finish without problem, we should not only clear folio dirty,
-	 * but also empty subpage dirty bits
+	 * If we didn't submitted any sector (>= i_size), folio dirty get
+	 * cleared but PAGECACHE_TAG_DIRTY is not cleared (only cleared
+	 * by folio_start_writeback() if the folio is not dirty).
+	 *
+	 * Here we set writeback and clear for the range. If the full folio
+	 * is no longer dirty then we clear the PAGECACHE_TAG_DIRTY tag.
 	 */
-	*nr_ret = nr;
+	if (!submitted_io) {
+		btrfs_folio_set_writeback(fs_info, folio, start, len);
+		btrfs_folio_clear_writeback(fs_info, folio, start, len);
+	}
 	return ret;
 }
 
@@ -1501,7 +1505,6 @@  static int __extent_writepage(struct folio *folio, struct btrfs_bio_ctrl *bio_ct
 	struct inode *inode = folio->mapping->host;
 	const u64 page_start = folio_pos(folio);
 	int ret;
-	int nr = 0;
 	size_t pg_offset;
 	loff_t i_size = i_size_read(inode);
 	unsigned long end_index = i_size >> PAGE_SHIFT;
@@ -1532,18 +1535,13 @@  static int __extent_writepage(struct folio *folio, struct btrfs_bio_ctrl *bio_ct
 		goto done;
 
 	ret = __extent_writepage_io(BTRFS_I(inode), folio, folio_pos(folio),
-				    PAGE_SIZE, bio_ctrl, i_size, &nr);
+				    PAGE_SIZE, bio_ctrl, i_size);
 	if (ret == 1)
 		return 0;
 
 	bio_ctrl->wbc->nr_to_write--;
 
 done:
-	if (nr == 0) {
-		/* make sure the mapping tag for page dirty gets cleared */
-		folio_start_writeback(folio);
-		folio_end_writeback(folio);
-	}
 	if (ret) {
 		btrfs_mark_ordered_io_finished(BTRFS_I(inode), folio,
 					       page_start, PAGE_SIZE, !ret);
@@ -2297,15 +2295,10 @@  void extent_write_locked_range(struct inode *inode, const struct folio *locked_f
 			ASSERT(folio_test_dirty(folio));
 
 		ret = __extent_writepage_io(BTRFS_I(inode), folio, cur, cur_len,
-					    &bio_ctrl, i_size, &nr);
+					    &bio_ctrl, i_size);
 		if (ret == 1)
 			goto next_page;
 
-		/* Make sure the mapping tag for page dirty gets cleared. */
-		if (nr == 0) {
-			btrfs_folio_set_writeback(fs_info, folio, cur, cur_len);
-			btrfs_folio_clear_writeback(fs_info, folio, cur, cur_len);
-		}
 		if (ret) {
 			btrfs_mark_ordered_io_finished(BTRFS_I(inode), folio,
 						       cur, cur_len, !ret);