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@xxxxxxxxx>
> ---
> 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.