[PATCH] nilfs2: simplify nilfs_mdt_writeback()

[PATCH] nilfs2: simplify nilfs_mdt_writeback()

[Ryusuke Konishi]

The writepages() callback for metadata files, nilfs_mdt_writeback(),
calls nilfs_mdt_write_folio() for each dirty folio via writeback_iter(),
where read-only superblock checks, folio dirty state clearing on error,
and triggering of the log writer via nilfs_construct_segment() are
performed.  This helper function is used nowhere else.

Executing these operations per folio is inefficient:

- Checking if the filesystem has fallen back to read-only mode is
  repeated redundantly.

- Calling nilfs_construct_segment() inside the per-folio loop causes
  redundant log writer invocations for a single sync request, where only
  the first invocation performs actual log writing.

Unfold nilfs_mdt_write_folio() into nilfs_mdt_writeback() and consolidate
these operations at the function level.  The read-only check is moved to
the top of nilfs_mdt_writeback(), replacing per-folio
nilfs_clear_folio_dirty() calls with a single nilfs_clear_dirty_pages()
call to discard dirty pages across the mapping at once.  Additionally,
nilfs_construct_segment() is invoked once when 'sync_mode' is WB_SYNC_ALL
and the mapping is tagged dirty.

With these operations consolidated outside the loop, the remaining
per-folio actions (calling folio_redirty_for_writepage() on already
dirty folios and unlocking them) serve no purpose, so the folio iteration
itself is removed.

Signed-off-by: Ryusuke Konishi <konishi.ryusuke@gmail.com>
---
Viacheslav,

Please add this to the queue for the next cycle.

This refactors the metadata file writeback callback function,
restructuring an inefficient legacy function organization rooted in
the era of aops->writepage() to align with aops->writepages().

Thanks,
Ryusuke Konishi

 fs/nilfs2/mdt.c | 40 +++++++++++-----------------------------
 1 file changed, 11 insertions(+), 29 deletions(-)

diff --git a/fs/nilfs2/mdt.c b/fs/nilfs2/mdt.c
index b50c88b65183..77dc2be2ce65 100644
--- a/fs/nilfs2/mdt.c
+++ b/fs/nilfs2/mdt.c
@@ -391,51 +391,33 @@ int nilfs_mdt_fetch_dirty(struct inode *inode)
 	return test_bit(NILFS_I_DIRTY, &ii->i_state);
 }
 
-static int nilfs_mdt_write_folio(struct folio *folio,
+static int nilfs_mdt_writeback(struct address_space *mapping,
 		struct writeback_control *wbc)
 {
-	struct inode *inode = folio->mapping->host;
-	struct super_block *sb;
+	struct inode *inode = mapping->host;
 	int err = 0;
 
-	if (inode && sb_rdonly(inode->i_sb)) {
+	if (!inode)
+		return 0;
+
+	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 folios that try to be flushed in background.
-		 * So, here we simply discard this dirty folio.
+		 * So, here we simply discard these dirty folios.
 		 */
-		nilfs_clear_folio_dirty(folio, false);
-		folio_unlock(folio);
+		nilfs_clear_dirty_pages(mapping, false);
 		return -EROFS;
 	}
 
-	folio_redirty_for_writepage(wbc, folio);
-	folio_unlock(folio);
-
-	if (!inode)
-		return 0;
-
-	sb = inode->i_sb;
-
-	if (wbc->sync_mode == WB_SYNC_ALL)
-		err = nilfs_construct_segment(sb);
+	if (wbc->sync_mode == WB_SYNC_ALL &&
+		mapping_tagged(mapping, PAGECACHE_TAG_DIRTY))
+		err = nilfs_construct_segment(inode->i_sb);
 
 	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)))
-		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,
-- 
2.53.0

Re: [PATCH] nilfs2: simplify nilfs_mdt_writeback()

[Viacheslav Dubeyko]

On Mon, 2026-10-05 at 08:31 +0900, Ryusuke Konishi wrote:
> The writepages() callback for metadata files, nilfs_mdt_writeback(),
> calls nilfs_mdt_write_folio() for each dirty folio via
> writeback_iter(),
> where read-only superblock checks, folio dirty state clearing on
> error,
> and triggering of the log writer via nilfs_construct_segment() are
> performed.  This helper function is used nowhere else.
> 
> Executing these operations per folio is inefficient:
> 
> - Checking if the filesystem has fallen back to read-only mode is
>   repeated redundantly.
> 
> - Calling nilfs_construct_segment() inside the per-folio loop causes
>   redundant log writer invocations for a single sync request, where
> only
>   the first invocation performs actual log writing.
> 
> Unfold nilfs_mdt_write_folio() into nilfs_mdt_writeback() and
> consolidate
> these operations at the function level.  The read-only check is moved
> to
> the top of nilfs_mdt_writeback(), replacing per-folio
> nilfs_clear_folio_dirty() calls with a single
> nilfs_clear_dirty_pages()
> call to discard dirty pages across the mapping at once. 
> Additionally,
> nilfs_construct_segment() is invoked once when 'sync_mode' is
> WB_SYNC_ALL
> and the mapping is tagged dirty.
> 
> With these operations consolidated outside the loop, the remaining
> per-folio actions (calling folio_redirty_for_writepage() on already
> dirty folios and unlocking them) serve no purpose, so the folio
> iteration
> itself is removed.
> 
> Signed-off-by: Ryusuke Konishi <konishi.ryusuke@gmail.com>
> ---
> Viacheslav,
> 
> Please add this to the queue for the next cycle.
> 
> This refactors the metadata file writeback callback function,
> restructuring an inefficient legacy function organization rooted in
> the era of aops->writepage() to align with aops->writepages().
> 
> Thanks,
> Ryusuke Konishi
> 
>  fs/nilfs2/mdt.c | 40 +++++++++++-----------------------------
>  1 file changed, 11 insertions(+), 29 deletions(-)
> 
> diff --git a/fs/nilfs2/mdt.c b/fs/nilfs2/mdt.c
> index b50c88b65183..77dc2be2ce65 100644
> --- a/fs/nilfs2/mdt.c
> +++ b/fs/nilfs2/mdt.c
> @@ -391,51 +391,33 @@ int nilfs_mdt_fetch_dirty(struct inode *inode)
>  	return test_bit(NILFS_I_DIRTY, &ii->i_state);
>  }
>  
> -static int nilfs_mdt_write_folio(struct folio *folio,
> +static int nilfs_mdt_writeback(struct address_space *mapping,
>  		struct writeback_control *wbc)
>  {
> -	struct inode *inode = folio->mapping->host;
> -	struct super_block *sb;
> +	struct inode *inode = mapping->host;
>  	int err = 0;
>  
> -	if (inode && sb_rdonly(inode->i_sb)) {
> +	if (!inode)
> +		return 0;
> +
> +	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 folios that try to be flushed in
> background.
> -		 * So, here we simply discard this dirty folio.
> +		 * So, here we simply discard these dirty folios.
>  		 */
> -		nilfs_clear_folio_dirty(folio, false);
> -		folio_unlock(folio);
> +		nilfs_clear_dirty_pages(mapping, false);
>  		return -EROFS;
>  	}
>  
> -	folio_redirty_for_writepage(wbc, folio);
> -	folio_unlock(folio);
> -
> -	if (!inode)
> -		return 0;
> -
> -	sb = inode->i_sb;
> -
> -	if (wbc->sync_mode == WB_SYNC_ALL)
> -		err = nilfs_construct_segment(sb);
> +	if (wbc->sync_mode == WB_SYNC_ALL &&
> +		mapping_tagged(mapping, PAGECACHE_TAG_DIRTY))
> +		err = nilfs_construct_segment(inode->i_sb);
>  
>  	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)))
> -		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,

Applied.

Thanks,
Slava.