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

[Jan Kara]

On Tue 22-10-24 19:10:42, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@xxxxxxxxxx>
> 
> 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@xxxxxxxxxx>

Yeah, I agree. Feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>

								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@xxxxxxxx>
SUSE Labs, CR