[PATCH v2 0/3] page to folio conversion

[PATCH v2 0/3] page to folio conversion

[Goldwyn Rodrigues]

From: Goldwyn Rodrigues <rgoldwyn@suse.com>

These patches transform some page usage to folio. All references and data
of page/folio is within the scope of the function changed.

Changes since v1:
Review comments - 
  * Added WARN_ON(folio_order(folio)) to ensure future development knows
    this code assumes folio_size(folio) == PAGE_SIZE
  * namespace restoration: prefix variable names with folio_
  * Line adjustments

Goldwyn Rodrigues (3):
  btrfs: page to folio conversion: prealloc_file_extent_cluster()
  btrfs: convert relocate_one_page() to relocate_one_folio()
  btrfs: page to folio conversion in put_file_data()

 fs/btrfs/relocation.c | 103 +++++++++++++++++++++---------------------
 fs/btrfs/send.c       |  44 +++++++++---------
 2 files changed, 75 insertions(+), 72 deletions(-)

-- 
2.43.0

[PATCH 1/3] btrfs: page to folio conversion: prealloc_file_extent_cluster()

[Goldwyn Rodrigues]

From: Goldwyn Rodrigues <rgoldwyn@suse.com>

Convert usage of page to folio in prealloc_file_extent_cluster()

Reviewed-by: Boris Burkov <boris@bur.io>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
---
 fs/btrfs/relocation.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index abe594f77f99..c365bfc60652 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -2858,7 +2858,7 @@ static noinline_for_stack int prealloc_file_extent_cluster(
 		struct address_space *mapping = inode->vfs_inode.i_mapping;
 		struct btrfs_fs_info *fs_info = inode->root->fs_info;
 		const u32 sectorsize = fs_info->sectorsize;
-		struct page *page;
+		struct folio *folio;
 
 		ASSERT(sectorsize < PAGE_SIZE);
 		ASSERT(IS_ALIGNED(i_size, sectorsize));
@@ -2889,16 +2889,16 @@ static noinline_for_stack int prealloc_file_extent_cluster(
 		clear_extent_bits(&inode->io_tree, i_size,
 				  round_up(i_size, PAGE_SIZE) - 1,
 				  EXTENT_UPTODATE);
-		page = find_lock_page(mapping, i_size >> PAGE_SHIFT);
+		folio = filemap_lock_folio(mapping, i_size >> PAGE_SHIFT);
 		/*
 		 * If page is freed we don't need to do anything then, as we
 		 * will re-read the whole page anyway.
 		 */
-		if (page) {
-			btrfs_subpage_clear_uptodate(fs_info, page_folio(page), i_size,
+		if (!IS_ERR(folio)) {
+			btrfs_subpage_clear_uptodate(fs_info, folio, i_size,
 					round_up(i_size, PAGE_SIZE) - i_size);
-			unlock_page(page);
-			put_page(page);
+			folio_unlock(folio);
+			folio_put(folio);
 		}
 	}
 
-- 
2.43.0

[PATCH 2/3] btrfs: convert relocate_one_page() to relocate_one_folio()

[Goldwyn Rodrigues]

From: Goldwyn Rodrigues <rgoldwyn@suse.com>

Convert page references to folios and call the respective folio
functions.

Since find_or_create_page() takes a mask argument, call
__filemap_get_folio() instead of filemap_grab_folio().

The patch assumes folio size is PAGE_SIZE, so added a
WARN_ON(folio_order(folio)) to warn future development
of using larger folio sizes.

Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
---
 fs/btrfs/relocation.c | 91 ++++++++++++++++++++++---------------------
 1 file changed, 46 insertions(+), 45 deletions(-)

diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index c365bfc60652..d9e70106c901 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -2849,7 +2849,7 @@ static noinline_for_stack int prealloc_file_extent_cluster(
 	 * btrfs_do_readpage() call of previously relocated file cluster.
 	 *
 	 * If the current cluster starts in the above range, btrfs_do_readpage()
-	 * will skip the read, and relocate_one_page() will later writeback
+	 * will skip the read, and relocate_one_folio() will later writeback
 	 * the padding zeros as new data, causing data corruption.
 	 *
 	 * Here we have to manually invalidate the range (i_size, PAGE_END + 1).
@@ -2983,68 +2983,71 @@ static u64 get_cluster_boundary_end(const struct file_extent_cluster *cluster,
 	return cluster->boundary[cluster_nr + 1] - 1;
 }
 
-static int relocate_one_page(struct inode *inode, struct file_ra_state *ra,
+static int relocate_one_folio(struct inode *inode, struct file_ra_state *ra,
 			     const struct file_extent_cluster *cluster,
-			     int *cluster_nr, unsigned long page_index)
+			     int *cluster_nr, unsigned long index)
 {
 	struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
 	u64 offset = BTRFS_I(inode)->index_cnt;
 	const unsigned long last_index = (cluster->end - offset) >> PAGE_SHIFT;
 	gfp_t mask = btrfs_alloc_write_mask(inode->i_mapping);
-	struct page *page;
-	u64 page_start;
-	u64 page_end;
+	struct folio *folio;
+	u64 folio_start;
+	u64 folio_end;
 	u64 cur;
 	int ret;
 
-	ASSERT(page_index <= last_index);
-	page = find_lock_page(inode->i_mapping, page_index);
-	if (!page) {
+	ASSERT(index <= last_index);
+	folio = filemap_lock_folio(inode->i_mapping, index);
+	if (IS_ERR(folio)) {
 		page_cache_sync_readahead(inode->i_mapping, ra, NULL,
-				page_index, last_index + 1 - page_index);
-		page = find_or_create_page(inode->i_mapping, page_index, mask);
-		if (!page)
-			return -ENOMEM;
+				index, last_index + 1 - index);
+		folio = __filemap_get_folio(inode->i_mapping, index,
+					    FGP_LOCK | FGP_ACCESSED | FGP_CREAT, mask);
+		if (IS_ERR(folio))
+			return PTR_ERR(folio);
 	}
 
-	if (PageReadahead(page))
+	WARN_ON(folio_order(folio));
+
+	if (folio_test_readahead(folio))
 		page_cache_async_readahead(inode->i_mapping, ra, NULL,
-				page_folio(page), page_index,
-				last_index + 1 - page_index);
+				folio, index,
+				last_index + 1 - index);
 
-	if (!PageUptodate(page)) {
-		btrfs_read_folio(NULL, page_folio(page));
-		lock_page(page);
-		if (!PageUptodate(page)) {
+	if (!folio_test_uptodate(folio)) {
+		btrfs_read_folio(NULL, folio);
+		folio_lock(folio);
+		if (!folio_test_uptodate(folio)) {
 			ret = -EIO;
-			goto release_page;
+			goto release_folio;
 		}
 	}
 
 	/*
-	 * We could have lost page private when we dropped the lock to read the
-	 * page above, make sure we set_page_extent_mapped here so we have any
+	 * We could have lost folio private when we dropped the lock to read the
+	 * folio above, make sure we set_page_extent_mapped here so we have any
 	 * of the subpage blocksize stuff we need in place.
 	 */
-	ret = set_page_extent_mapped(page);
+	ret = set_folio_extent_mapped(folio);
 	if (ret < 0)
-		goto release_page;
+		goto release_folio;
 
-	page_start = page_offset(page);
-	page_end = page_start + PAGE_SIZE - 1;
+	folio_start = folio_pos(folio);
+	folio_end = folio_start + PAGE_SIZE - 1;
 
 	/*
 	 * Start from the cluster, as for subpage case, the cluster can start
-	 * inside the page.
+	 * inside the folio.
 	 */
-	cur = max(page_start, cluster->boundary[*cluster_nr] - offset);
-	while (cur <= page_end) {
+	cur = max(folio_start, cluster->boundary[*cluster_nr] - offset);
+	while (cur <= folio_end) {
 		struct extent_state *cached_state = NULL;
 		u64 extent_start = cluster->boundary[*cluster_nr] - offset;
 		u64 extent_end = get_cluster_boundary_end(cluster,
 						*cluster_nr) - offset;
-		u64 clamped_start = max(page_start, extent_start);
-		u64 clamped_end = min(page_end, extent_end);
+		u64 clamped_start = max(folio_start, extent_start);
+		u64 clamped_end = min(folio_end, extent_end);
 		u32 clamped_len = clamped_end + 1 - clamped_start;
 
 		/* Reserve metadata for this range */
@@ -3052,7 +3055,7 @@ static int relocate_one_page(struct inode *inode, struct file_ra_state *ra,
 						      clamped_len, clamped_len,
 						      false);
 		if (ret)
-			goto release_page;
+			goto release_folio;
 
 		/* Mark the range delalloc and dirty for later writeback */
 		lock_extent(&BTRFS_I(inode)->io_tree, clamped_start, clamped_end,
@@ -3068,20 +3071,18 @@ static int relocate_one_page(struct inode *inode, struct file_ra_state *ra,
 							clamped_len, true);
 			btrfs_delalloc_release_extents(BTRFS_I(inode),
 						       clamped_len);
-			goto release_page;
+			goto release_folio;
 		}
-		btrfs_folio_set_dirty(fs_info, page_folio(page),
-				      clamped_start, clamped_len);
+		btrfs_folio_set_dirty(fs_info, folio, clamped_start, clamped_len);
 
 		/*
-		 * Set the boundary if it's inside the page.
+		 * Set the boundary if it's inside the folio.
 		 * Data relocation requires the destination extents to have the
 		 * same size as the source.
 		 * EXTENT_BOUNDARY bit prevents current extent from being merged
 		 * with previous extent.
 		 */
-		if (in_range(cluster->boundary[*cluster_nr] - offset,
-			     page_start, PAGE_SIZE)) {
+		if (in_range(cluster->boundary[*cluster_nr] - offset, folio_start, PAGE_SIZE)) {
 			u64 boundary_start = cluster->boundary[*cluster_nr] -
 						offset;
 			u64 boundary_end = boundary_start +
@@ -3104,8 +3105,8 @@ static int relocate_one_page(struct inode *inode, struct file_ra_state *ra,
 				break;
 		}
 	}
-	unlock_page(page);
-	put_page(page);
+	folio_unlock(folio);
+	folio_put(folio);
 
 	balance_dirty_pages_ratelimited(inode->i_mapping);
 	btrfs_throttle(fs_info);
@@ -3113,9 +3114,9 @@ static int relocate_one_page(struct inode *inode, struct file_ra_state *ra,
 		ret = -ECANCELED;
 	return ret;
 
-release_page:
-	unlock_page(page);
-	put_page(page);
+release_folio:
+	folio_unlock(folio);
+	folio_put(folio);
 	return ret;
 }
 
@@ -3150,7 +3151,7 @@ static int relocate_file_extent_cluster(struct inode *inode,
 	last_index = (cluster->end - offset) >> PAGE_SHIFT;
 	for (index = (cluster->start - offset) >> PAGE_SHIFT;
 	     index <= last_index && !ret; index++)
-		ret = relocate_one_page(inode, ra, cluster, &cluster_nr, index);
+		ret = relocate_one_folio(inode, ra, cluster, &cluster_nr, index);
 	if (ret == 0)
 		WARN_ON(cluster_nr != cluster->nr);
 out:
-- 
2.43.0

[PATCH 3/3] btrfs: page to folio conversion in put_file_data()

[Goldwyn Rodrigues]

From: Goldwyn Rodrigues <rgoldwyn@suse.com>

Use folio instead of page in put_file_data().
Add a WARN_ON(folio_order(folio)) to make sure we are dealing with
PAGE_SIZE folios.

Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
---
 fs/btrfs/send.c | 44 +++++++++++++++++++++++---------------------
 1 file changed, 23 insertions(+), 21 deletions(-)

diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index 141ab89fb63e..8885eadbb6a8 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -5257,10 +5257,11 @@ static int put_file_data(struct send_ctx *sctx, u64 offset, u32 len)
 {
 	struct btrfs_root *root = sctx->send_root;
 	struct btrfs_fs_info *fs_info = root->fs_info;
-	struct page *page;
+	struct folio *folio;
 	pgoff_t index = offset >> PAGE_SHIFT;
 	pgoff_t last_index;
 	unsigned pg_offset = offset_in_page(offset);
+	struct address_space *mapping = sctx->cur_inode->i_mapping;
 	int ret;
 
 	ret = put_data_header(sctx, len);
@@ -5273,44 +5274,45 @@ static int put_file_data(struct send_ctx *sctx, u64 offset, u32 len)
 		unsigned cur_len = min_t(unsigned, len,
 					 PAGE_SIZE - pg_offset);
 
-		page = find_lock_page(sctx->cur_inode->i_mapping, index);
-		if (!page) {
-			page_cache_sync_readahead(sctx->cur_inode->i_mapping,
+		folio = filemap_lock_folio(mapping, index);
+		if (IS_ERR(folio)) {
+			page_cache_sync_readahead(mapping,
 						  &sctx->ra, NULL, index,
 						  last_index + 1 - index);
 
-			page = find_or_create_page(sctx->cur_inode->i_mapping,
-						   index, GFP_KERNEL);
-			if (!page) {
-				ret = -ENOMEM;
+	                folio = filemap_grab_folio(mapping, index);
+			if (IS_ERR(folio)) {
+				ret = PTR_ERR(folio);
 				break;
 			}
 		}
 
-		if (PageReadahead(page))
-			page_cache_async_readahead(sctx->cur_inode->i_mapping,
-						   &sctx->ra, NULL, page_folio(page),
+		WARN_ON(folio_order(folio));
+
+		if (folio_test_readahead(folio))
+			page_cache_async_readahead(mapping,
+						   &sctx->ra, NULL, folio,
 						   index, last_index + 1 - index);
 
-		if (!PageUptodate(page)) {
-			btrfs_read_folio(NULL, page_folio(page));
-			lock_page(page);
-			if (!PageUptodate(page)) {
-				unlock_page(page);
+		if (!folio_test_uptodate(folio)) {
+			btrfs_read_folio(NULL, folio);
+			folio_lock(folio);
+			if (!folio_test_uptodate(folio)) {
+				folio_unlock(folio);
 				btrfs_err(fs_info,
 			"send: IO error at offset %llu for inode %llu root %llu",
-					page_offset(page), sctx->cur_ino,
+					folio_pos(folio), sctx->cur_ino,
 					sctx->send_root->root_key.objectid);
-				put_page(page);
+				folio_put(folio);
 				ret = -EIO;
 				break;
 			}
 		}
 
-		memcpy_from_page(sctx->send_buf + sctx->send_size, page,
+		memcpy_from_folio(sctx->send_buf + sctx->send_size, folio,
 				 pg_offset, cur_len);
-		unlock_page(page);
-		put_page(page);
+		folio_unlock(folio);
+		folio_put(folio);
 		index++;
 		pg_offset = 0;
 		len -= cur_len;
-- 
2.43.0

Re: [PATCH v2 0/3] page to folio conversion

[David Sterba]

On Tue, Jan 23, 2024 at 01:28:04PM -0600, Goldwyn Rodrigues wrote:
> From: Goldwyn Rodrigues <rgoldwyn@suse.com>
> 
> These patches transform some page usage to folio. All references and data
> of page/folio is within the scope of the function changed.
> 
> Changes since v1:
> Review comments - 
>   * Added WARN_ON(folio_order(folio)) to ensure future development knows
>     this code assumes folio_size(folio) == PAGE_SIZE
>   * namespace restoration: prefix variable names with folio_
>   * Line adjustments
> 
> Goldwyn Rodrigues (3):
>   btrfs: page to folio conversion: prealloc_file_extent_cluster()
>   btrfs: convert relocate_one_page() to relocate_one_folio()
>   btrfs: page to folio conversion in put_file_data()

The conversion looks straightforward like we've been doing elsewhere,
however the CI is still not in a shape to validate arm + subpage, I've
seen the hosts not pass with various sets of patches (removed potential
breakage and keeping potential fixes).

There are more folio conversions coming so I'd like to get them all in
so we can switch to the big folios eventually but without the CI
verification of subpage it's a bit risky.

Re: [PATCH v2 0/3] page to folio conversion

[Neal Gompa]

On Mon, Jan 29, 2024 at 8:05 AM David Sterba <dsterba@suse.cz> wrote:
>
> On Tue, Jan 23, 2024 at 01:28:04PM -0600, Goldwyn Rodrigues wrote:
> > From: Goldwyn Rodrigues <rgoldwyn@suse.com>
> >
> > These patches transform some page usage to folio. All references and data
> > of page/folio is within the scope of the function changed.
> >
> > Changes since v1:
> > Review comments -
> >   * Added WARN_ON(folio_order(folio)) to ensure future development knows
> >     this code assumes folio_size(folio) == PAGE_SIZE
> >   * namespace restoration: prefix variable names with folio_
> >   * Line adjustments
> >
> > Goldwyn Rodrigues (3):
> >   btrfs: page to folio conversion: prealloc_file_extent_cluster()
> >   btrfs: convert relocate_one_page() to relocate_one_folio()
> >   btrfs: page to folio conversion in put_file_data()
>
> The conversion looks straightforward like we've been doing elsewhere,
> however the CI is still not in a shape to validate arm + subpage, I've
> seen the hosts not pass with various sets of patches (removed potential
> breakage and keeping potential fixes).
>
> There are more folio conversions coming so I'd like to get them all in
> so we can switch to the big folios eventually but without the CI
> verification of subpage it's a bit risky.
>

Wait, we don't? I thought Josef had specifically added Fedora Asahi
runners specifically for subpage testing[1]?

[1]: https://josefbacik.github.io/kernel/2023/07/18/btrfs-github-ci.html



--
真実はいつも一つ！/ Always, there's only one truth!

Re: [PATCH v2 0/3] page to folio conversion

[David Sterba]

On Wed, Jan 31, 2024 at 04:29:36AM +0000, Neal Gompa wrote:
> On Mon, Jan 29, 2024 at 8:05 AM David Sterba <dsterba@suse.cz> wrote:
> >
> > On Tue, Jan 23, 2024 at 01:28:04PM -0600, Goldwyn Rodrigues wrote:
> > > From: Goldwyn Rodrigues <rgoldwyn@suse.com>
> > >
> > > These patches transform some page usage to folio. All references and data
> > > of page/folio is within the scope of the function changed.
> > >
> > > Changes since v1:
> > > Review comments -
> > >   * Added WARN_ON(folio_order(folio)) to ensure future development knows
> > >     this code assumes folio_size(folio) == PAGE_SIZE
> > >   * namespace restoration: prefix variable names with folio_
> > >   * Line adjustments
> > >
> > > Goldwyn Rodrigues (3):
> > >   btrfs: page to folio conversion: prealloc_file_extent_cluster()
> > >   btrfs: convert relocate_one_page() to relocate_one_folio()
> > >   btrfs: page to folio conversion in put_file_data()
> >
> > The conversion looks straightforward like we've been doing elsewhere,
> > however the CI is still not in a shape to validate arm + subpage, I've
> > seen the hosts not pass with various sets of patches (removed potential
> > breakage and keeping potential fixes).
> >
> > There are more folio conversions coming so I'd like to get them all in
> > so we can switch to the big folios eventually but without the CI
> > verification of subpage it's a bit risky.
> >
> 
> Wait, we don't? I thought Josef had specifically added Fedora Asahi
> runners specifically for subpage testing[1]?

Yes, but we don't have yet a stable testing base with all-pass results
on all configuration and with quirks disabling unreliable tests. There
are crashes reported with various sets of patches that are likely
caused by folio changes but it's hard to narrow down which ones.

Re: [PATCH v2 0/3] page to folio conversion

[David Sterba]

On Mon, Jan 29, 2024 at 09:04:42AM +0100, David Sterba wrote:
> On Tue, Jan 23, 2024 at 01:28:04PM -0600, Goldwyn Rodrigues wrote:
> > From: Goldwyn Rodrigues <rgoldwyn@suse.com>
> > 
> > These patches transform some page usage to folio. All references and data
> > of page/folio is within the scope of the function changed.
> > 
> > Changes since v1:
> > Review comments - 
> >   * Added WARN_ON(folio_order(folio)) to ensure future development knows
> >     this code assumes folio_size(folio) == PAGE_SIZE
> >   * namespace restoration: prefix variable names with folio_
> >   * Line adjustments
> > 
> > Goldwyn Rodrigues (3):
> >   btrfs: page to folio conversion: prealloc_file_extent_cluster()
> >   btrfs: convert relocate_one_page() to relocate_one_folio()
> >   btrfs: page to folio conversion in put_file_data()
> 
> The conversion looks straightforward like we've been doing elsewhere,
> however the CI is still not in a shape to validate arm + subpage, I've
> seen the hosts not pass with various sets of patches (removed potential
> breakage and keeping potential fixes).
> 
> There are more folio conversions coming so I'd like to get them all in
> so we can switch to the big folios eventually but without the CI
> verification of subpage it's a bit risky.

The patches have been in my misc-next (and in linux-next), no problems
reported so far. The extent buffer problems have been fixed so folios
can changes can be added again. As this patches is 2 months old I've
moved it to for-next with some code style fixups and changelog updates.