Re: [PATCH v2 6/8] fs/ext4: use sleeping version of __find_get_block()

Jan Kara
2025-04-10T13:36:32Z

On Wed 09-04-25 18:49:43, Luis Chamberlain wrote:
> From: Davidlohr Bueso <dave@stgolabs.net>
>
> Trivially introduce the wrapper and enable ext4_free_blocks() to use
> it, which has a cond_resched to begin with. Convert to the new nonatomic
> flavor to benefit from potential performance benefits and adapt in the
> future vs migration such that semantics are kept.
>
> Suggested-by: Jan Kara <jack@suse.cz>
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> ---
> fs/ext4/inode.c | 2 ++
> fs/ext4/mballoc.c | 3 ++-
> include/linux/buffer_head.h | 6 ++++++
> 3 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 1dc09ed5d403..b7acb5d3adcb 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -860,6 +860,8 @@ struct buffer_head *ext4_getblk(handle_t *handle, struct inode *inode,
> return sb_find_get_block(inode->i_sb, map.m_pblk);
>
> /*
> + * Potential TODO: use sb_find_get_block_nonatomic() instead.
> + *

Yes, please. Since we are behind nowait check, we are fine with blocking...

Honza

> * Since bh could introduce extra ref count such as referred by
> * journal_head etc. Try to avoid using __GFP_MOVABLE here
> * as it may fail the migration when journal_head remains.
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 0d523e9fb3d5..6f4265b21e19 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -6644,7 +6644,8 @@ void ext4_free_blocks(handle_t *handle, struct inode *inode,
> for (i = 0; i < count; i++) {
> cond_resched();
> if (is_metadata)
> - bh = sb_find_get_block(inode->i_sb, block + i);
> + bh = sb_find_get_block_nonatomic(inode->i_sb,
> + block + i);
> ext4_forget(handle, is_metadata, inode, bh, block + i);
> }
> }
> diff --git a/include/linux/buffer_head.h b/include/linux/buffer_head.h
> index 2b5458517def..8db10ca288fc 100644
> --- a/include/linux/buffer_head.h
> +++ b/include/linux/buffer_head.h
> @@ -399,6 +399,12 @@ sb_find_get_block(struct super_block *sb, sector_t block)
> return __find_get_block(sb->s_bdev, block, sb->s_blocksize);
> }
>
> +static inline struct buffer_head *
> +sb_find_get_block_nonatomic(struct super_block *sb, sector_t block)
> +{
> + return __find_get_block_nonatomic(sb->s_bdev, block, sb->s_blocksize);
> +}
> +

This hunk probably belongs to some introductory patch implementing
nonatomic helpers.

Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR