| Message ID | 20240802115120.362902-3-yi.zhang@huaweicloud.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | ext4: simplify the counting and management of delalloc reserved blocks | expand |
On Fri 02-08-24 19:51:12, Zhang Yi wrote: > From: Zhang Yi <yi.zhang@huawei.com> > > 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@huawei.com> Ah, nice idea. The patch looks good. Feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> 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 >
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)