[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 2/3] btrfs: convert relocate_one_page() to relocate_one_folio()

[kernel test robot]

Hi Goldwyn,

kernel test robot noticed the following build errors:

[auto build test ERROR on kdave/for-next]
[also build test ERROR on linus/master v6.8-rc1 next-20240125]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Goldwyn-Rodrigues/btrfs-page-to-folio-conversion-prealloc_file_extent_cluster/20240124-032942
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
patch link:    https://lore.kernel.org/r/0ad1a3b7098c852f6e0d41770fae5daea70ea8d1.1706037337.git.rgoldwyn%40suse.com
patch subject: [PATCH 2/3] btrfs: convert relocate_one_page() to relocate_one_folio()
config: i386-randconfig-011-20240127 (https://download.01.org/0day-ci/archive/20240127/202401270800.NyZlHGxV-lkp@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240127/202401270800.NyZlHGxV-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202401270800.NyZlHGxV-lkp@intel.com/

All errors (new ones prefixed by >>):

>> fs/btrfs/relocation.c:3032:8: error: call to undeclared function 'set_folio_extent_mapped'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    3032 |         ret = set_folio_extent_mapped(folio);
         |               ^
   fs/btrfs/relocation.c:3032:8: note: did you mean 'set_page_extent_mapped'?
   fs/btrfs/extent_io.h:234:5: note: 'set_page_extent_mapped' declared here
     234 | int set_page_extent_mapped(struct page *page);
         |     ^
   1 error generated.


vim +/set_folio_extent_mapped +3032 fs/btrfs/relocation.c

  2985	
  2986	static int relocate_one_folio(struct inode *inode, struct file_ra_state *ra,
  2987				     const struct file_extent_cluster *cluster,
  2988				     int *cluster_nr, unsigned long index)
  2989	{
  2990		struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
  2991		u64 offset = BTRFS_I(inode)->index_cnt;
  2992		const unsigned long last_index = (cluster->end - offset) >> PAGE_SHIFT;
  2993		gfp_t mask = btrfs_alloc_write_mask(inode->i_mapping);
  2994		struct folio *folio;
  2995		u64 folio_start;
  2996		u64 folio_end;
  2997		u64 cur;
  2998		int ret;
  2999	
  3000		ASSERT(index <= last_index);
  3001		folio = filemap_lock_folio(inode->i_mapping, index);
  3002		if (IS_ERR(folio)) {
  3003			page_cache_sync_readahead(inode->i_mapping, ra, NULL,
  3004					index, last_index + 1 - index);
  3005			folio = __filemap_get_folio(inode->i_mapping, index,
  3006						    FGP_LOCK | FGP_ACCESSED | FGP_CREAT, mask);
  3007			if (IS_ERR(folio))
  3008				return PTR_ERR(folio);
  3009		}
  3010	
  3011		WARN_ON(folio_order(folio));
  3012	
  3013		if (folio_test_readahead(folio))
  3014			page_cache_async_readahead(inode->i_mapping, ra, NULL,
  3015					folio, index,
  3016					last_index + 1 - index);
  3017	
  3018		if (!folio_test_uptodate(folio)) {
  3019			btrfs_read_folio(NULL, folio);
  3020			folio_lock(folio);
  3021			if (!folio_test_uptodate(folio)) {
  3022				ret = -EIO;
  3023				goto release_folio;
  3024			}
  3025		}
  3026	
  3027		/*
  3028		 * We could have lost folio private when we dropped the lock to read the
  3029		 * folio above, make sure we set_page_extent_mapped here so we have any
  3030		 * of the subpage blocksize stuff we need in place.
  3031		 */
> 3032		ret = set_folio_extent_mapped(folio);
  3033		if (ret < 0)
  3034			goto release_folio;
  3035	
  3036		folio_start = folio_pos(folio);
  3037		folio_end = folio_start + PAGE_SIZE - 1;
  3038	
  3039		/*
  3040		 * Start from the cluster, as for subpage case, the cluster can start
  3041		 * inside the folio.
  3042		 */
  3043		cur = max(folio_start, cluster->boundary[*cluster_nr] - offset);
  3044		while (cur <= folio_end) {
  3045			struct extent_state *cached_state = NULL;
  3046			u64 extent_start = cluster->boundary[*cluster_nr] - offset;
  3047			u64 extent_end = get_cluster_boundary_end(cluster,
  3048							*cluster_nr) - offset;
  3049			u64 clamped_start = max(folio_start, extent_start);
  3050			u64 clamped_end = min(folio_end, extent_end);
  3051			u32 clamped_len = clamped_end + 1 - clamped_start;
  3052	
  3053			/* Reserve metadata for this range */
  3054			ret = btrfs_delalloc_reserve_metadata(BTRFS_I(inode),
  3055							      clamped_len, clamped_len,
  3056							      false);
  3057			if (ret)
  3058				goto release_folio;
  3059	
  3060			/* Mark the range delalloc and dirty for later writeback */
  3061			lock_extent(&BTRFS_I(inode)->io_tree, clamped_start, clamped_end,
  3062				    &cached_state);
  3063			ret = btrfs_set_extent_delalloc(BTRFS_I(inode), clamped_start,
  3064							clamped_end, 0, &cached_state);
  3065			if (ret) {
  3066				clear_extent_bit(&BTRFS_I(inode)->io_tree,
  3067						 clamped_start, clamped_end,
  3068						 EXTENT_LOCKED | EXTENT_BOUNDARY,
  3069						 &cached_state);
  3070				btrfs_delalloc_release_metadata(BTRFS_I(inode),
  3071								clamped_len, true);
  3072				btrfs_delalloc_release_extents(BTRFS_I(inode),
  3073							       clamped_len);
  3074				goto release_folio;
  3075			}
  3076			btrfs_folio_set_dirty(fs_info, folio, clamped_start, clamped_len);
  3077	
  3078			/*
  3079			 * Set the boundary if it's inside the folio.
  3080			 * Data relocation requires the destination extents to have the
  3081			 * same size as the source.
  3082			 * EXTENT_BOUNDARY bit prevents current extent from being merged
  3083			 * with previous extent.
  3084			 */
  3085			if (in_range(cluster->boundary[*cluster_nr] - offset, folio_start, PAGE_SIZE)) {
  3086				u64 boundary_start = cluster->boundary[*cluster_nr] -
  3087							offset;
  3088				u64 boundary_end = boundary_start +
  3089						   fs_info->sectorsize - 1;
  3090	
  3091				set_extent_bit(&BTRFS_I(inode)->io_tree,
  3092					       boundary_start, boundary_end,
  3093					       EXTENT_BOUNDARY, NULL);
  3094			}
  3095			unlock_extent(&BTRFS_I(inode)->io_tree, clamped_start, clamped_end,
  3096				      &cached_state);
  3097			btrfs_delalloc_release_extents(BTRFS_I(inode), clamped_len);
  3098			cur += clamped_len;
  3099	
  3100			/* Crossed extent end, go to next extent */
  3101			if (cur >= extent_end) {
  3102				(*cluster_nr)++;
  3103				/* Just finished the last extent of the cluster, exit. */
  3104				if (*cluster_nr >= cluster->nr)
  3105					break;
  3106			}
  3107		}
  3108		folio_unlock(folio);
  3109		folio_put(folio);
  3110	
  3111		balance_dirty_pages_ratelimited(inode->i_mapping);
  3112		btrfs_throttle(fs_info);
  3113		if (btrfs_should_cancel_balance(fs_info))
  3114			ret = -ECANCELED;
  3115		return ret;
  3116	
  3117	release_folio:
  3118		folio_unlock(folio);
  3119		folio_put(folio);
  3120		return ret;
  3121	}
  3122	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

[kernel test robot]

Hi Goldwyn,

kernel test robot noticed the following build errors:

[auto build test ERROR on kdave/for-next]
[also build test ERROR on linus/master v6.8-rc1 next-20240125]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Goldwyn-Rodrigues/btrfs-page-to-folio-conversion-prealloc_file_extent_cluster/20240124-032942
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
patch link:    https://lore.kernel.org/r/0ad1a3b7098c852f6e0d41770fae5daea70ea8d1.1706037337.git.rgoldwyn%40suse.com
patch subject: [PATCH 2/3] btrfs: convert relocate_one_page() to relocate_one_folio()
config: i386-buildonly-randconfig-002-20240127 (https://download.01.org/0day-ci/archive/20240127/202401270939.F2MFTxSK-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240127/202401270939.F2MFTxSK-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202401270939.F2MFTxSK-lkp@intel.com/

All errors (new ones prefixed by >>):

   fs/btrfs/relocation.c: In function 'relocate_one_folio':
>> fs/btrfs/relocation.c:3032:15: error: implicit declaration of function 'set_folio_extent_mapped'; did you mean 'set_page_extent_mapped'? [-Werror=implicit-function-declaration]
    3032 |         ret = set_folio_extent_mapped(folio);
         |               ^~~~~~~~~~~~~~~~~~~~~~~
         |               set_page_extent_mapped
   cc1: some warnings being treated as errors


vim +3032 fs/btrfs/relocation.c

  2985	
  2986	static int relocate_one_folio(struct inode *inode, struct file_ra_state *ra,
  2987				     const struct file_extent_cluster *cluster,
  2988				     int *cluster_nr, unsigned long index)
  2989	{
  2990		struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
  2991		u64 offset = BTRFS_I(inode)->index_cnt;
  2992		const unsigned long last_index = (cluster->end - offset) >> PAGE_SHIFT;
  2993		gfp_t mask = btrfs_alloc_write_mask(inode->i_mapping);
  2994		struct folio *folio;
  2995		u64 folio_start;
  2996		u64 folio_end;
  2997		u64 cur;
  2998		int ret;
  2999	
  3000		ASSERT(index <= last_index);
  3001		folio = filemap_lock_folio(inode->i_mapping, index);
  3002		if (IS_ERR(folio)) {
  3003			page_cache_sync_readahead(inode->i_mapping, ra, NULL,
  3004					index, last_index + 1 - index);
  3005			folio = __filemap_get_folio(inode->i_mapping, index,
  3006						    FGP_LOCK | FGP_ACCESSED | FGP_CREAT, mask);
  3007			if (IS_ERR(folio))
  3008				return PTR_ERR(folio);
  3009		}
  3010	
  3011		WARN_ON(folio_order(folio));
  3012	
  3013		if (folio_test_readahead(folio))
  3014			page_cache_async_readahead(inode->i_mapping, ra, NULL,
  3015					folio, index,
  3016					last_index + 1 - index);
  3017	
  3018		if (!folio_test_uptodate(folio)) {
  3019			btrfs_read_folio(NULL, folio);
  3020			folio_lock(folio);
  3021			if (!folio_test_uptodate(folio)) {
  3022				ret = -EIO;
  3023				goto release_folio;
  3024			}
  3025		}
  3026	
  3027		/*
  3028		 * We could have lost folio private when we dropped the lock to read the
  3029		 * folio above, make sure we set_page_extent_mapped here so we have any
  3030		 * of the subpage blocksize stuff we need in place.
  3031		 */
> 3032		ret = set_folio_extent_mapped(folio);
  3033		if (ret < 0)
  3034			goto release_folio;
  3035	
  3036		folio_start = folio_pos(folio);
  3037		folio_end = folio_start + PAGE_SIZE - 1;
  3038	
  3039		/*
  3040		 * Start from the cluster, as for subpage case, the cluster can start
  3041		 * inside the folio.
  3042		 */
  3043		cur = max(folio_start, cluster->boundary[*cluster_nr] - offset);
  3044		while (cur <= folio_end) {
  3045			struct extent_state *cached_state = NULL;
  3046			u64 extent_start = cluster->boundary[*cluster_nr] - offset;
  3047			u64 extent_end = get_cluster_boundary_end(cluster,
  3048							*cluster_nr) - offset;
  3049			u64 clamped_start = max(folio_start, extent_start);
  3050			u64 clamped_end = min(folio_end, extent_end);
  3051			u32 clamped_len = clamped_end + 1 - clamped_start;
  3052	
  3053			/* Reserve metadata for this range */
  3054			ret = btrfs_delalloc_reserve_metadata(BTRFS_I(inode),
  3055							      clamped_len, clamped_len,
  3056							      false);
  3057			if (ret)
  3058				goto release_folio;
  3059	
  3060			/* Mark the range delalloc and dirty for later writeback */
  3061			lock_extent(&BTRFS_I(inode)->io_tree, clamped_start, clamped_end,
  3062				    &cached_state);
  3063			ret = btrfs_set_extent_delalloc(BTRFS_I(inode), clamped_start,
  3064							clamped_end, 0, &cached_state);
  3065			if (ret) {
  3066				clear_extent_bit(&BTRFS_I(inode)->io_tree,
  3067						 clamped_start, clamped_end,
  3068						 EXTENT_LOCKED | EXTENT_BOUNDARY,
  3069						 &cached_state);
  3070				btrfs_delalloc_release_metadata(BTRFS_I(inode),
  3071								clamped_len, true);
  3072				btrfs_delalloc_release_extents(BTRFS_I(inode),
  3073							       clamped_len);
  3074				goto release_folio;
  3075			}
  3076			btrfs_folio_set_dirty(fs_info, folio, clamped_start, clamped_len);
  3077	
  3078			/*
  3079			 * Set the boundary if it's inside the folio.
  3080			 * Data relocation requires the destination extents to have the
  3081			 * same size as the source.
  3082			 * EXTENT_BOUNDARY bit prevents current extent from being merged
  3083			 * with previous extent.
  3084			 */
  3085			if (in_range(cluster->boundary[*cluster_nr] - offset, folio_start, PAGE_SIZE)) {
  3086				u64 boundary_start = cluster->boundary[*cluster_nr] -
  3087							offset;
  3088				u64 boundary_end = boundary_start +
  3089						   fs_info->sectorsize - 1;
  3090	
  3091				set_extent_bit(&BTRFS_I(inode)->io_tree,
  3092					       boundary_start, boundary_end,
  3093					       EXTENT_BOUNDARY, NULL);
  3094			}
  3095			unlock_extent(&BTRFS_I(inode)->io_tree, clamped_start, clamped_end,
  3096				      &cached_state);
  3097			btrfs_delalloc_release_extents(BTRFS_I(inode), clamped_len);
  3098			cur += clamped_len;
  3099	
  3100			/* Crossed extent end, go to next extent */
  3101			if (cur >= extent_end) {
  3102				(*cluster_nr)++;
  3103				/* Just finished the last extent of the cluster, exit. */
  3104				if (*cluster_nr >= cluster->nr)
  3105					break;
  3106			}
  3107		}
  3108		folio_unlock(folio);
  3109		folio_put(folio);
  3110	
  3111		balance_dirty_pages_ratelimited(inode->i_mapping);
  3112		btrfs_throttle(fs_info);
  3113		if (btrfs_should_cancel_balance(fs_info))
  3114			ret = -ECANCELED;
  3115		return ret;
  3116	
  3117	release_folio:
  3118		folio_unlock(folio);
  3119		folio_put(folio);
  3120		return ret;
  3121	}
  3122	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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.