Re: [PATCH 11/27] ext4: use reserved metadata blocks when splitting extent on endio

Jan Kara
2024-12-04T12:16:48Z

On Tue 22-10-24 19:10:42, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
>
> When performing buffered writes, we may need to split and convert an
> unwritten extent into a written one during the end I/O process. However,
> we do not reserve space specifically for these metadata changes, we only
> reserve 2% of space or 4096 blocks. To address this, we use
> EXT4_GET_BLOCKS_PRE_IO to potentially split extents in advance and
> EXT4_GET_BLOCKS_METADATA_NOFAIL to utilize reserved space if necessary.
>
> These two approaches can reduce the likelihood of running out of space
> and losing data. However, these methods are merely best efforts, we
> could still run out of space, and there is not much difference between
> converting an extent during the writeback process and the end I/O
> process, it won't increase the rick of losing data if we postpone the
> conversion.
>
> Therefore, also use EXT4_GET_BLOCKS_METADATA_NOFAIL in
> ext4_convert_unwritten_extents_endio() to prepare for the buffered I/O
> iomap conversion, which may perform extent conversion during the end I/O
> process.
>
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>

Yeah, I agree. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

Honza

> ---
> fs/ext4/extents.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index d5067d5aa449..33bc2cc5aff4 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -3767,6 +3767,8 @@ ext4_convert_unwritten_extents_endio(handle_t *handle, struct inode *inode,
> * illegal.
> */
> if (ee_block != map->m_lblk || ee_len > map->m_len) {
> + int flags = EXT4_GET_BLOCKS_CONVERT |
> + EXT4_GET_BLOCKS_METADATA_NOFAIL;
> #ifdef CONFIG_EXT4_DEBUG
> ext4_warning(inode->i_sb, "Inode (%ld) finished: extent logical block %llu,"
> " len %u; IO logical block %llu, len %u",
> @@ -3774,7 +3776,7 @@ ext4_convert_unwritten_extents_endio(handle_t *handle, struct inode *inode,
> (unsigned long long)map->m_lblk, map->m_len);
> #endif
> path = ext4_split_convert_extents(handle, inode, map, path,
> - EXT4_GET_BLOCKS_CONVERT, NULL);
> + flags, NULL);
> if (IS_ERR(path))
> return path;
>
> --
> 2.46.1
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR