On Fri 02-08-24 19:51:12, Zhang Yi wrote: > From: Zhang Yi <yi.zhang@xxxxxxxxxx> > > When doing block allocation, magic EXT4_GET_BLOCKS_DELALLOC_RESERVE > means the allocating range covers a range of delayed allocated clusters, > the blocks and quotas have already been reserved in ext4_da_map_blocks(), > we should update the reserved space and don't need to claim them again. > > At the moment, we only set this magic in mpage_map_one_extent() when > allocating a range of delayed allocated clusters in the write back path, > it makes things complicated since we have to notice and deal with the > case of allocating non-delayed allocated clusters separately in > ext4_ext_map_blocks(). For example, it we fallocate some blocks that > have been delayed allocated, free space would be claimed again in > ext4_mb_new_blocks() (this is wrong exactily), and we can't claim quota > space again, we have to release the quota reservations made for that > previously delayed allocated clusters. > > Move the position thats set the EXT4_GET_BLOCKS_DELALLOC_RESERVE to > where we actually do block allocation, it could simplify above handling > a lot, it means that we always set this magic once the allocation range > covers delalloc blocks, no need to take care of the allocation path. > > Signed-off-by: Zhang Yi <yi.zhang@xxxxxxxxxx> Ah, nice idea. The patch looks good. Feel free to add: Reviewed-by: Jan Kara <jack@xxxxxxx> Honza > --- > fs/ext4/inode.c | 15 ++++++++------- > 1 file changed, 8 insertions(+), 7 deletions(-) > > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c > index 112aec171ee9..91b2610a6dc5 100644 > --- a/fs/ext4/inode.c > +++ b/fs/ext4/inode.c > @@ -489,6 +489,14 @@ static int ext4_map_create_blocks(handle_t *handle, struct inode *inode, > unsigned int status; > int err, retval = 0; > > + /* > + * We pass in the magic EXT4_GET_BLOCKS_DELALLOC_RESERVE > + * indicates that the blocks and quotas has already been > + * checked when the data was copied into the page cache. > + */ > + if (map->m_flags & EXT4_MAP_DELAYED) > + flags |= EXT4_GET_BLOCKS_DELALLOC_RESERVE; > + > /* > * Here we clear m_flags because after allocating an new extent, > * it will be set again. > @@ -2224,11 +2232,6 @@ static int mpage_map_one_extent(handle_t *handle, struct mpage_da_data *mpd) > * writeback and there is nothing we can do about it so it might result > * in data loss. So use reserved blocks to allocate metadata if > * possible. > - * > - * We pass in the magic EXT4_GET_BLOCKS_DELALLOC_RESERVE if > - * the blocks in question are delalloc blocks. This indicates > - * that the blocks and quotas has already been checked when > - * the data was copied into the page cache. > */ > get_blocks_flags = EXT4_GET_BLOCKS_CREATE | > EXT4_GET_BLOCKS_METADATA_NOFAIL | > @@ -2236,8 +2239,6 @@ static int mpage_map_one_extent(handle_t *handle, struct mpage_da_data *mpd) > dioread_nolock = ext4_should_dioread_nolock(inode); > if (dioread_nolock) > get_blocks_flags |= EXT4_GET_BLOCKS_IO_CREATE_EXT; > - if (map->m_flags & BIT(BH_Delay)) > - get_blocks_flags |= EXT4_GET_BLOCKS_DELALLOC_RESERVE; > > err = ext4_map_blocks(handle, inode, map, get_blocks_flags); > if (err < 0) > -- > 2.39.2 > -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
