[PATCH 0/4] nilfs2: Finish folio conversion

[PATCH 0/4] nilfs2: Finish folio conversion

[Matthew Wilcox (Oracle)]

After "nilfs2: Convert nilfs_copy_buffer() to use folios", there are
only a few remaining users of struct page in all of nilfs2, and they're
straightforward to remove.  Build tested only.

Matthew Wilcox (Oracle) (4):
  nilfs2: Remove nilfs_writepage
  nilfs2: Convert nilfs_page_count_clean_buffers() to take a folio
  nilfs2: Convert nilfs_recovery_copy_block() to take a folio
  nilfs2: Convert metadata aops from writepage to writepages

 fs/nilfs2/dir.c      |  2 +-
 fs/nilfs2/inode.c    | 35 ++---------------------------------
 fs/nilfs2/mdt.c      | 19 +++++++++++++++----
 fs/nilfs2/page.c     |  4 ++--
 fs/nilfs2/page.h     |  4 ++--
 fs/nilfs2/recovery.c | 11 ++++-------
 6 files changed, 26 insertions(+), 49 deletions(-)

-- 
2.43.0

[PATCH 1/4] nilfs2: Remove nilfs_writepage

[Matthew Wilcox (Oracle)]

Since nilfs2 has a ->writepages operation already, ->writepage is only
called by the migration code.  If we add a ->migrate_folio operation,
it won't even be used for that and so it can be deleted.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/nilfs2/inode.c | 33 +--------------------------------
 1 file changed, 1 insertion(+), 32 deletions(-)

diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
index be6acf6e2bfc..f1b47b655672 100644
--- a/fs/nilfs2/inode.c
+++ b/fs/nilfs2/inode.c
@@ -170,37 +170,6 @@ static int nilfs_writepages(struct address_space *mapping,
 	return err;
 }
 
-static int nilfs_writepage(struct page *page, struct writeback_control *wbc)
-{
-	struct folio *folio = page_folio(page);
-	struct inode *inode = folio->mapping->host;
-	int err;
-
-	if (sb_rdonly(inode->i_sb)) {
-		/*
-		 * It means that filesystem was remounted in read-only
-		 * mode because of error or metadata corruption. But we
-		 * have dirty pages that try to be flushed in background.
-		 * So, here we simply discard this dirty page.
-		 */
-		nilfs_clear_folio_dirty(folio);
-		folio_unlock(folio);
-		return -EROFS;
-	}
-
-	folio_redirty_for_writepage(wbc, folio);
-	folio_unlock(folio);
-
-	if (wbc->sync_mode == WB_SYNC_ALL) {
-		err = nilfs_construct_segment(inode->i_sb);
-		if (unlikely(err))
-			return err;
-	} else if (wbc->for_reclaim)
-		nilfs_flush_segment(inode->i_sb, inode->i_ino);
-
-	return 0;
-}
-
 static bool nilfs_dirty_folio(struct address_space *mapping,
 		struct folio *folio)
 {
@@ -295,7 +264,6 @@ nilfs_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
 }
 
 const struct address_space_operations nilfs_aops = {
-	.writepage		= nilfs_writepage,
 	.read_folio		= nilfs_read_folio,
 	.writepages		= nilfs_writepages,
 	.dirty_folio		= nilfs_dirty_folio,
@@ -304,6 +272,7 @@ const struct address_space_operations nilfs_aops = {
 	.write_end		= nilfs_write_end,
 	.invalidate_folio	= block_invalidate_folio,
 	.direct_IO		= nilfs_direct_IO,
+	.migrate_folio		= buffer_migrate_folio,
 	.is_partially_uptodate  = block_is_partially_uptodate,
 };
 
-- 
2.43.0

[PATCH 2/4] nilfs2: Convert nilfs_page_count_clean_buffers() to take a folio

[Matthew Wilcox (Oracle)]

Both callers have a folio, so pass it in and use it directly.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/nilfs2/dir.c   | 2 +-
 fs/nilfs2/inode.c | 2 +-
 fs/nilfs2/page.c  | 4 ++--
 fs/nilfs2/page.h  | 4 ++--
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/nilfs2/dir.c b/fs/nilfs2/dir.c
index fe5b1a30c509..b1ad4062bbab 100644
--- a/fs/nilfs2/dir.c
+++ b/fs/nilfs2/dir.c
@@ -95,7 +95,7 @@ static void nilfs_commit_chunk(struct folio *folio,
 	unsigned int nr_dirty;
 	int err;
 
-	nr_dirty = nilfs_page_count_clean_buffers(&folio->page, from, to);
+	nr_dirty = nilfs_page_count_clean_buffers(folio, from, to);
 	copied = block_write_end(NULL, mapping, pos, len, len, folio, NULL);
 	if (pos + copied > dir->i_size)
 		i_size_write(dir, pos + copied);
diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
index f1b47b655672..005dfd1f8fec 100644
--- a/fs/nilfs2/inode.c
+++ b/fs/nilfs2/inode.c
@@ -242,7 +242,7 @@ static int nilfs_write_end(struct file *file, struct address_space *mapping,
 	unsigned int nr_dirty;
 	int err;
 
-	nr_dirty = nilfs_page_count_clean_buffers(&folio->page, start,
+	nr_dirty = nilfs_page_count_clean_buffers(folio, start,
 						  start + copied);
 	copied = generic_write_end(file, mapping, pos, len, copied, folio,
 				   fsdata);
diff --git a/fs/nilfs2/page.c b/fs/nilfs2/page.c
index 16bb82cdbc07..ebd395dd131b 100644
--- a/fs/nilfs2/page.c
+++ b/fs/nilfs2/page.c
@@ -419,14 +419,14 @@ void nilfs_clear_folio_dirty(struct folio *folio)
 	__nilfs_clear_folio_dirty(folio);
 }
 
-unsigned int nilfs_page_count_clean_buffers(struct page *page,
+unsigned int nilfs_page_count_clean_buffers(struct folio *folio,
 					    unsigned int from, unsigned int to)
 {
 	unsigned int block_start, block_end;
 	struct buffer_head *bh, *head;
 	unsigned int nc = 0;
 
-	for (bh = head = page_buffers(page), block_start = 0;
+	for (bh = head = folio_buffers(folio), block_start = 0;
 	     bh != head || !block_start;
 	     block_start = block_end, bh = bh->b_this_page) {
 		block_end = block_start + bh->b_size;
diff --git a/fs/nilfs2/page.h b/fs/nilfs2/page.h
index 64521a03a19e..b6d9301f16ae 100644
--- a/fs/nilfs2/page.h
+++ b/fs/nilfs2/page.h
@@ -43,8 +43,8 @@ int nilfs_copy_dirty_pages(struct address_space *, struct address_space *);
 void nilfs_copy_back_pages(struct address_space *, struct address_space *);
 void nilfs_clear_folio_dirty(struct folio *folio);
 void nilfs_clear_dirty_pages(struct address_space *mapping);
-unsigned int nilfs_page_count_clean_buffers(struct page *, unsigned int,
-					    unsigned int);
+unsigned int nilfs_page_count_clean_buffers(struct folio *,
+		unsigned int from, unsigned int to);
 unsigned long nilfs_find_uncommitted_extent(struct inode *inode,
 					    sector_t start_blk,
 					    sector_t *blkoff);
-- 
2.43.0

[PATCH 3/4] nilfs2: Convert nilfs_recovery_copy_block() to take a folio

[Matthew Wilcox (Oracle)]

Use memcpy_to_folio() instead of open-coding it, and use offset_in_folio()
in case anybody wants to use nilfs2 on a device with large blocks.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/nilfs2/recovery.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/fs/nilfs2/recovery.c b/fs/nilfs2/recovery.c
index 21d81097a89f..1c665a32f002 100644
--- a/fs/nilfs2/recovery.c
+++ b/fs/nilfs2/recovery.c
@@ -481,19 +481,16 @@ static int nilfs_prepare_segment_for_recovery(struct the_nilfs *nilfs,
 
 static int nilfs_recovery_copy_block(struct the_nilfs *nilfs,
 				     struct nilfs_recovery_block *rb,
-				     loff_t pos, struct page *page)
+				     loff_t pos, struct folio *folio)
 {
 	struct buffer_head *bh_org;
-	size_t from = pos & ~PAGE_MASK;
-	void *kaddr;
+	size_t from = offset_in_folio(folio, pos);
 
 	bh_org = __bread(nilfs->ns_bdev, rb->blocknr, nilfs->ns_blocksize);
 	if (unlikely(!bh_org))
 		return -EIO;
 
-	kaddr = kmap_local_page(page);
-	memcpy(kaddr + from, bh_org->b_data, bh_org->b_size);
-	kunmap_local(kaddr);
+	memcpy_to_folio(folio, from, bh_org->b_data, bh_org->b_size);
 	brelse(bh_org);
 	return 0;
 }
@@ -531,7 +528,7 @@ static int nilfs_recover_dsync_blocks(struct the_nilfs *nilfs,
 			goto failed_inode;
 		}
 
-		err = nilfs_recovery_copy_block(nilfs, rb, pos, &folio->page);
+		err = nilfs_recovery_copy_block(nilfs, rb, pos, folio);
 		if (unlikely(err))
 			goto failed_page;
 
-- 
2.43.0

[PATCH 4/4] nilfs2: Convert metadata aops from writepage to writepages

[Matthew Wilcox (Oracle)]

By implementing ->writepages instead of ->writepage, we remove a
layer of indirect function calls from the writeback path and the
last use of struct page in nilfs2.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/nilfs2/mdt.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/fs/nilfs2/mdt.c b/fs/nilfs2/mdt.c
index ceb7dc0b5bad..4f4a935fcdc5 100644
--- a/fs/nilfs2/mdt.c
+++ b/fs/nilfs2/mdt.c
@@ -396,10 +396,9 @@ int nilfs_mdt_fetch_dirty(struct inode *inode)
 	return test_bit(NILFS_I_DIRTY, &ii->i_state);
 }
 
-static int
-nilfs_mdt_write_page(struct page *page, struct writeback_control *wbc)
+static int nilfs_mdt_write_folio(struct folio *folio,
+		struct writeback_control *wbc)
 {
-	struct folio *folio = page_folio(page);
 	struct inode *inode = folio->mapping->host;
 	struct super_block *sb;
 	int err = 0;
@@ -432,11 +431,23 @@ nilfs_mdt_write_page(struct page *page, struct writeback_control *wbc)
 	return err;
 }
 
+static int nilfs_mdt_writeback(struct address_space *mapping,
+		struct writeback_control *wbc)
+{
+	struct folio *folio = NULL;
+	int error;
+
+	while ((folio = writeback_iter(mapping, wbc, folio, &error)))
+		nilfs_mdt_write_folio(folio, wbc);
+
+	return error;
+}
 
 static const struct address_space_operations def_mdt_aops = {
 	.dirty_folio		= block_dirty_folio,
 	.invalidate_folio	= block_invalidate_folio,
-	.writepage		= nilfs_mdt_write_page,
+	.writepages		= nilfs_mdt_writeback,
+	.migrate_folio		= buffer_migrate_folio,
 };
 
 static const struct inode_operations def_mdt_iops;
-- 
2.43.0

Re: [PATCH 0/4] nilfs2: Finish folio conversion

[Ryusuke Konishi]

On Thu, Oct 3, 2024 at 12:00 AM Matthew Wilcox (Oracle) wrote:
>
> After "nilfs2: Convert nilfs_copy_buffer() to use folios", there are
> only a few remaining users of struct page in all of nilfs2, and they're
> straightforward to remove.  Build tested only.

Thank you for your ongoing work on converting to folio-based.

Page structure references still remain in other files, but I'm
preparing a patch set to convert them to be folio-based, so together
with that, I think we'll be able to remove most of the page references
in nilfs2 in the next cycle.

I'll check out this patch set.

Thanks,
Ryusuke Konishi

>
> Matthew Wilcox (Oracle) (4):
>   nilfs2: Remove nilfs_writepage
>   nilfs2: Convert nilfs_page_count_clean_buffers() to take a folio
>   nilfs2: Convert nilfs_recovery_copy_block() to take a folio
>   nilfs2: Convert metadata aops from writepage to writepages
>
>  fs/nilfs2/dir.c      |  2 +-
>  fs/nilfs2/inode.c    | 35 ++---------------------------------
>  fs/nilfs2/mdt.c      | 19 +++++++++++++++----
>  fs/nilfs2/page.c     |  4 ++--
>  fs/nilfs2/page.h     |  4 ++--
>  fs/nilfs2/recovery.c | 11 ++++-------
>  6 files changed, 26 insertions(+), 49 deletions(-)
>
> --
> 2.43.0
>

Re: [PATCH 1/4] nilfs2: Remove nilfs_writepage

[Ryusuke Konishi]

On Thu, Oct 3, 2024 at 12:00 AM Matthew Wilcox (Oracle) wrote:
>
> Since nilfs2 has a ->writepages operation already, ->writepage is only
> called by the migration code.  If we add a ->migrate_folio operation,
> it won't even be used for that and so it can be deleted.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  fs/nilfs2/inode.c | 33 +--------------------------------
>  1 file changed, 1 insertion(+), 32 deletions(-)
>
> diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
> index be6acf6e2bfc..f1b47b655672 100644
> --- a/fs/nilfs2/inode.c
> +++ b/fs/nilfs2/inode.c
> @@ -170,37 +170,6 @@ static int nilfs_writepages(struct address_space *mapping,
>         return err;
>  }
>
> -static int nilfs_writepage(struct page *page, struct writeback_control *wbc)
> -{
> -       struct folio *folio = page_folio(page);
> -       struct inode *inode = folio->mapping->host;
> -       int err;
> -
> -       if (sb_rdonly(inode->i_sb)) {
> -               /*
> -                * It means that filesystem was remounted in read-only
> -                * mode because of error or metadata corruption. But we
> -                * have dirty pages that try to be flushed in background.
> -                * So, here we simply discard this dirty page.
> -                */
> -               nilfs_clear_folio_dirty(folio);
> -               folio_unlock(folio);
> -               return -EROFS;
> -       }
> -
> -       folio_redirty_for_writepage(wbc, folio);
> -       folio_unlock(folio);
> -
> -       if (wbc->sync_mode == WB_SYNC_ALL) {
> -               err = nilfs_construct_segment(inode->i_sb);
> -               if (unlikely(err))
> -                       return err;
> -       } else if (wbc->for_reclaim)
> -               nilfs_flush_segment(inode->i_sb, inode->i_ino);
> -
> -       return 0;
> -}
> -
>  static bool nilfs_dirty_folio(struct address_space *mapping,
>                 struct folio *folio)
>  {
> @@ -295,7 +264,6 @@ nilfs_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
>  }
>
>  const struct address_space_operations nilfs_aops = {
> -       .writepage              = nilfs_writepage,
>         .read_folio             = nilfs_read_folio,
>         .writepages             = nilfs_writepages,
>         .dirty_folio            = nilfs_dirty_folio,
> @@ -304,6 +272,7 @@ const struct address_space_operations nilfs_aops = {
>         .write_end              = nilfs_write_end,
>         .invalidate_folio       = block_invalidate_folio,
>         .direct_IO              = nilfs_direct_IO,
> +       .migrate_folio          = buffer_migrate_folio,
>         .is_partially_uptodate  = block_is_partially_uptodate,
>  };
>

After applying this patch, fsstress started causing kernel panics.

Looking at the patch, I realized that migrate_folio needs to use
buffer_migrate_folio_norefs, which checks for buffer head references.

I was able to eliminate the kernel panic by setting migrate_folio as follows:

+ .migrate_folio = buffer_migrate_folio_norefs,

I would like to continue load testing to avoid side effects of reclaim
by completely eliminating nilfs_writepage (calling
nilfs_flush_segment). So far, no problems have occurred even in tests
with different block sizes or architectures, as long as I make the
above changes.

Thanks,
Ryusuke Konishi

Re: [PATCH 4/4] nilfs2: Convert metadata aops from writepage to writepages

[Ryusuke Konishi]

On Thu, Oct 3, 2024 at 12:00 AM Matthew Wilcox (Oracle) wrote:
>
> By implementing ->writepages instead of ->writepage, we remove a
> layer of indirect function calls from the writeback path and the
> last use of struct page in nilfs2.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  fs/nilfs2/mdt.c | 19 +++++++++++++++----
>  1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/fs/nilfs2/mdt.c b/fs/nilfs2/mdt.c
> index ceb7dc0b5bad..4f4a935fcdc5 100644
> --- a/fs/nilfs2/mdt.c
> +++ b/fs/nilfs2/mdt.c
> @@ -396,10 +396,9 @@ int nilfs_mdt_fetch_dirty(struct inode *inode)
>         return test_bit(NILFS_I_DIRTY, &ii->i_state);
>  }
>
> -static int
> -nilfs_mdt_write_page(struct page *page, struct writeback_control *wbc)
> +static int nilfs_mdt_write_folio(struct folio *folio,
> +               struct writeback_control *wbc)
>  {
> -       struct folio *folio = page_folio(page);
>         struct inode *inode = folio->mapping->host;
>         struct super_block *sb;
>         int err = 0;
> @@ -432,11 +431,23 @@ nilfs_mdt_write_page(struct page *page, struct writeback_control *wbc)
>         return err;
>  }
>
> +static int nilfs_mdt_writeback(struct address_space *mapping,
> +               struct writeback_control *wbc)
> +{
> +       struct folio *folio = NULL;
> +       int error;
> +

> +       while ((folio = writeback_iter(mapping, wbc, folio, &error)))
> +               nilfs_mdt_write_folio(folio, wbc);

In order to catch and return the error returned by
nilfs_mdt_write_folio, I think it's necessary to assign the return
value to the variable "error" in the loop as follows:

+       while ((folio = writeback_iter(mapping, wbc, folio, &error)))
+               error = nilfs_mdt_write_folio(folio, wbc);

> +
> +       return error;
> +}
>
>  static const struct address_space_operations def_mdt_aops = {
>         .dirty_folio            = block_dirty_folio,
>         .invalidate_folio       = block_invalidate_folio,
> -       .writepage              = nilfs_mdt_write_page,
> +       .writepages             = nilfs_mdt_writeback,

> +       .migrate_folio          = buffer_migrate_folio,
>  };

And, this also caused kernel panics with the fsstress command.  As
with patch 1/4, it was necessary to use buffer_migrate_folio_norefs
for migrate_folio:

+ .migrate_folio = buffer_migrate_folio_norefs,


Thanks,
Ryusuke Konishi


>
>  static const struct inode_operations def_mdt_iops;
> --
> 2.43.0
>

Re: [PATCH 2/4] nilfs2: Convert nilfs_page_count_clean_buffers() to take a folio

[Ryusuke Konishi]

On Thu, Oct 3, 2024 at 12:00 AM Matthew Wilcox (Oracle) wrote:
>
> Both callers have a folio, so pass it in and use it directly.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  fs/nilfs2/dir.c   | 2 +-
>  fs/nilfs2/inode.c | 2 +-
>  fs/nilfs2/page.c  | 4 ++--
>  fs/nilfs2/page.h  | 4 ++--
>  4 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/fs/nilfs2/dir.c b/fs/nilfs2/dir.c
> index fe5b1a30c509..b1ad4062bbab 100644
> --- a/fs/nilfs2/dir.c
> +++ b/fs/nilfs2/dir.c
> @@ -95,7 +95,7 @@ static void nilfs_commit_chunk(struct folio *folio,
>         unsigned int nr_dirty;
>         int err;
>
> -       nr_dirty = nilfs_page_count_clean_buffers(&folio->page, from, to);
> +       nr_dirty = nilfs_page_count_clean_buffers(folio, from, to);
>         copied = block_write_end(NULL, mapping, pos, len, len, folio, NULL);
>         if (pos + copied > dir->i_size)
>                 i_size_write(dir, pos + copied);
> diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
> index f1b47b655672..005dfd1f8fec 100644
> --- a/fs/nilfs2/inode.c
> +++ b/fs/nilfs2/inode.c
> @@ -242,7 +242,7 @@ static int nilfs_write_end(struct file *file, struct address_space *mapping,
>         unsigned int nr_dirty;
>         int err;
>
> -       nr_dirty = nilfs_page_count_clean_buffers(&folio->page, start,
> +       nr_dirty = nilfs_page_count_clean_buffers(folio, start,
>                                                   start + copied);
>         copied = generic_write_end(file, mapping, pos, len, copied, folio,
>                                    fsdata);
> diff --git a/fs/nilfs2/page.c b/fs/nilfs2/page.c
> index 16bb82cdbc07..ebd395dd131b 100644
> --- a/fs/nilfs2/page.c
> +++ b/fs/nilfs2/page.c
> @@ -419,14 +419,14 @@ void nilfs_clear_folio_dirty(struct folio *folio)
>         __nilfs_clear_folio_dirty(folio);
>  }
>
> -unsigned int nilfs_page_count_clean_buffers(struct page *page,
> +unsigned int nilfs_page_count_clean_buffers(struct folio *folio,
>                                             unsigned int from, unsigned int to)
>  {
>         unsigned int block_start, block_end;
>         struct buffer_head *bh, *head;
>         unsigned int nc = 0;
>
> -       for (bh = head = page_buffers(page), block_start = 0;
> +       for (bh = head = folio_buffers(folio), block_start = 0;
>              bh != head || !block_start;
>              block_start = block_end, bh = bh->b_this_page) {
>                 block_end = block_start + bh->b_size;
> diff --git a/fs/nilfs2/page.h b/fs/nilfs2/page.h
> index 64521a03a19e..b6d9301f16ae 100644
> --- a/fs/nilfs2/page.h
> +++ b/fs/nilfs2/page.h
> @@ -43,8 +43,8 @@ int nilfs_copy_dirty_pages(struct address_space *, struct address_space *);
>  void nilfs_copy_back_pages(struct address_space *, struct address_space *);
>  void nilfs_clear_folio_dirty(struct folio *folio);
>  void nilfs_clear_dirty_pages(struct address_space *mapping);
> -unsigned int nilfs_page_count_clean_buffers(struct page *, unsigned int,
> -                                           unsigned int);
> +unsigned int nilfs_page_count_clean_buffers(struct folio *,
> +               unsigned int from, unsigned int to);

This gives the following checkpatch warning:

WARNING: function definition argument 'struct folio *' should also
have an identifier name
#75: FILE: fs/nilfs2/page.h:46:
+unsigned int nilfs_page_count_clean_buffers(struct folio *,

It would be appreciated if you could include the argument name in the
declaration of nilfs_page_count_clean_buffer after replacement.

Everything else seems fine.

Thanks,
Ryusuke Konishi

>  unsigned long nilfs_find_uncommitted_extent(struct inode *inode,
>                                             sector_t start_blk,
>                                             sector_t *blkoff);
> --
> 2.43.0
>

Re: [PATCH 3/4] nilfs2: Convert nilfs_recovery_copy_block() to take a folio

[Ryusuke Konishi]

On Thu, Oct 3, 2024 at 12:00 AM Matthew Wilcox (Oracle) wrote:
>
> Use memcpy_to_folio() instead of open-coding it, and use offset_in_folio()
> in case anybody wants to use nilfs2 on a device with large blocks.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  fs/nilfs2/recovery.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/fs/nilfs2/recovery.c b/fs/nilfs2/recovery.c
> index 21d81097a89f..1c665a32f002 100644
> --- a/fs/nilfs2/recovery.c
> +++ b/fs/nilfs2/recovery.c
> @@ -481,19 +481,16 @@ static int nilfs_prepare_segment_for_recovery(struct the_nilfs *nilfs,
>
>  static int nilfs_recovery_copy_block(struct the_nilfs *nilfs,
>                                      struct nilfs_recovery_block *rb,
> -                                    loff_t pos, struct page *page)
> +                                    loff_t pos, struct folio *folio)
>  {
>         struct buffer_head *bh_org;
> -       size_t from = pos & ~PAGE_MASK;
> -       void *kaddr;
> +       size_t from = offset_in_folio(folio, pos);
>
>         bh_org = __bread(nilfs->ns_bdev, rb->blocknr, nilfs->ns_blocksize);
>         if (unlikely(!bh_org))
>                 return -EIO;
>
> -       kaddr = kmap_local_page(page);
> -       memcpy(kaddr + from, bh_org->b_data, bh_org->b_size);
> -       kunmap_local(kaddr);
> +       memcpy_to_folio(folio, from, bh_org->b_data, bh_org->b_size);
>         brelse(bh_org);
>         return 0;
>  }
> @@ -531,7 +528,7 @@ static int nilfs_recover_dsync_blocks(struct the_nilfs *nilfs,
>                         goto failed_inode;
>                 }
>
> -               err = nilfs_recovery_copy_block(nilfs, rb, pos, &folio->page);
> +               err = nilfs_recovery_copy_block(nilfs, rb, pos, folio);
>                 if (unlikely(err))
>                         goto failed_page;
>
> --
> 2.43.0
>

This patch looks good to me.

One small thing: with this conversion, there is no reference to the
page structure in nilfs_recover_dsync_blocks, so how about changing
the jump label "failed_page" to "failed_folio"?
This will reduce noise when searching for "page" with grep.

Thanks,
Ryusuke Konishi

Re: [PATCH 0/4] nilfs2: Finish folio conversion

[Ryusuke Konishi]

On Thu, Oct 3, 2024 at 12:40 AM Ryusuke Konishi wrote:
>
> On Thu, Oct 3, 2024 at 12:00 AM Matthew Wilcox (Oracle) wrote:
> >
> > After "nilfs2: Convert nilfs_copy_buffer() to use folios", there are
> > only a few remaining users of struct page in all of nilfs2, and they're
> > straightforward to remove.  Build tested only.
>
> Thank you for your ongoing work on converting to folio-based.
>
> Page structure references still remain in other files, but I'm
> preparing a patch set to convert them to be folio-based, so together
> with that, I think we'll be able to remove most of the page references
> in nilfs2 in the next cycle.
>
> I'll check out this patch set.
>
> Thanks,
> Ryusuke Konishi

I've added comments to each patch based on my review and testing.

The biggest comment is about the kernel panic caused by patches 1/4
and 4/4.  As I wrote in my reply to each of them, this can be fixed by
replacing "buffer_migrate_folio" with "buffer_migrate_folio_norefs".

If you are busy and don't mind, I can fix the points I commented on.
If so, please let me know.
Or if you send me the v2 patchset, I'll check it again and add it to
the patches I'll send upstream for the next cycle.

Thanks,
Ryusuke Konishi