On Mon 31-01-22 20:46:54, Ritesh Harjani wrote: > ext4_free_blocks() function became too long and confusing, this patch > just pulls out the ext4_mb_clear_bb() function logic from it > which clears the block bitmap and frees it. > > No functionality change in this patch > > Signed-off-by: Ritesh Harjani <riteshh@xxxxxxxxxxxxx> Yeah, the function was rather long. Feel free to add: Reviewed-by: Jan Kara <jack@xxxxxxx> Honza > --- > fs/ext4/mballoc.c | 180 ++++++++++++++++++++++++++-------------------- > 1 file changed, 102 insertions(+), 78 deletions(-) > > diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c > index 2f931575e6c2..5f20e355d08c 100644 > --- a/fs/ext4/mballoc.c > +++ b/fs/ext4/mballoc.c > @@ -5870,7 +5870,8 @@ static void ext4_free_blocks_simple(struct inode *inode, ext4_fsblk_t block, > } > > /** > - * ext4_free_blocks() -- Free given blocks and update quota > + * ext4_mb_clear_bb() -- helper function for freeing blocks. > + * Used by ext4_free_blocks() > * @handle: handle for this transaction > * @inode: inode > * @bh: optional buffer of the block to be freed > @@ -5878,9 +5879,9 @@ static void ext4_free_blocks_simple(struct inode *inode, ext4_fsblk_t block, > * @count: number of blocks to be freed > * @flags: flags used by ext4_free_blocks > */ > -void ext4_free_blocks(handle_t *handle, struct inode *inode, > - struct buffer_head *bh, ext4_fsblk_t block, > - unsigned long count, int flags) > +static void ext4_mb_clear_bb(handle_t *handle, struct inode *inode, > + ext4_fsblk_t block, unsigned long count, > + int flags) > { > struct buffer_head *bitmap_bh = NULL; > struct super_block *sb = inode->i_sb; > @@ -5897,80 +5898,6 @@ void ext4_free_blocks(handle_t *handle, struct inode *inode, > > sbi = EXT4_SB(sb); > > - if (sbi->s_mount_state & EXT4_FC_REPLAY) { > - ext4_free_blocks_simple(inode, block, count); > - return; > - } > - > - might_sleep(); > - if (bh) { > - if (block) > - BUG_ON(block != bh->b_blocknr); > - else > - block = bh->b_blocknr; > - } > - > - if (!(flags & EXT4_FREE_BLOCKS_VALIDATED) && > - !ext4_inode_block_valid(inode, block, count)) { > - ext4_error(sb, "Freeing blocks not in datazone - " > - "block = %llu, count = %lu", block, count); > - goto error_return; > - } > - > - ext4_debug("freeing block %llu\n", block); > - trace_ext4_free_blocks(inode, block, count, flags); > - > - if (bh && (flags & EXT4_FREE_BLOCKS_FORGET)) { > - BUG_ON(count > 1); > - > - ext4_forget(handle, flags & EXT4_FREE_BLOCKS_METADATA, > - inode, bh, block); > - } > - > - /* > - * If the extent to be freed does not begin on a cluster > - * boundary, we need to deal with partial clusters at the > - * beginning and end of the extent. Normally we will free > - * blocks at the beginning or the end unless we are explicitly > - * requested to avoid doing so. > - */ > - overflow = EXT4_PBLK_COFF(sbi, block); > - if (overflow) { > - if (flags & EXT4_FREE_BLOCKS_NOFREE_FIRST_CLUSTER) { > - overflow = sbi->s_cluster_ratio - overflow; > - block += overflow; > - if (count > overflow) > - count -= overflow; > - else > - return; > - } else { > - block -= overflow; > - count += overflow; > - } > - } > - overflow = EXT4_LBLK_COFF(sbi, count); > - if (overflow) { > - if (flags & EXT4_FREE_BLOCKS_NOFREE_LAST_CLUSTER) { > - if (count > overflow) > - count -= overflow; > - else > - return; > - } else > - count += sbi->s_cluster_ratio - overflow; > - } > - > - if (!bh && (flags & EXT4_FREE_BLOCKS_FORGET)) { > - int i; > - int is_metadata = flags & EXT4_FREE_BLOCKS_METADATA; > - > - for (i = 0; i < count; i++) { > - cond_resched(); > - if (is_metadata) > - bh = sb_find_get_block(inode->i_sb, block + i); > - ext4_forget(handle, is_metadata, inode, bh, block + i); > - } > - } > - > do_more: > overflow = 0; > ext4_get_group_no_and_offset(sb, block, &block_group, &bit); > @@ -6132,6 +6059,103 @@ void ext4_free_blocks(handle_t *handle, struct inode *inode, > return; > } > > +/** > + * ext4_free_blocks() -- Free given blocks and update quota > + * @handle: handle for this transaction > + * @inode: inode > + * @bh: optional buffer of the block to be freed > + * @block: starting physical block to be freed > + * @count: number of blocks to be freed > + * @flags: flags used by ext4_free_blocks > + */ > +void ext4_free_blocks(handle_t *handle, struct inode *inode, > + struct buffer_head *bh, ext4_fsblk_t block, > + unsigned long count, int flags) > +{ > + struct super_block *sb = inode->i_sb; > + unsigned int overflow; > + struct ext4_sb_info *sbi; > + > + sbi = EXT4_SB(sb); > + > + if (sbi->s_mount_state & EXT4_FC_REPLAY) { > + ext4_free_blocks_simple(inode, block, count); > + return; > + } > + > + might_sleep(); > + if (bh) { > + if (block) > + BUG_ON(block != bh->b_blocknr); > + else > + block = bh->b_blocknr; > + } > + > + if (!(flags & EXT4_FREE_BLOCKS_VALIDATED) && > + !ext4_inode_block_valid(inode, block, count)) { > + ext4_error(sb, "Freeing blocks not in datazone - " > + "block = %llu, count = %lu", block, count); > + return; > + } > + > + ext4_debug("freeing block %llu\n", block); > + trace_ext4_free_blocks(inode, block, count, flags); > + > + if (bh && (flags & EXT4_FREE_BLOCKS_FORGET)) { > + BUG_ON(count > 1); > + > + ext4_forget(handle, flags & EXT4_FREE_BLOCKS_METADATA, > + inode, bh, block); > + } > + > + /* > + * If the extent to be freed does not begin on a cluster > + * boundary, we need to deal with partial clusters at the > + * beginning and end of the extent. Normally we will free > + * blocks at the beginning or the end unless we are explicitly > + * requested to avoid doing so. > + */ > + overflow = EXT4_PBLK_COFF(sbi, block); > + if (overflow) { > + if (flags & EXT4_FREE_BLOCKS_NOFREE_FIRST_CLUSTER) { > + overflow = sbi->s_cluster_ratio - overflow; > + block += overflow; > + if (count > overflow) > + count -= overflow; > + else > + return; > + } else { > + block -= overflow; > + count += overflow; > + } > + } > + overflow = EXT4_LBLK_COFF(sbi, count); > + if (overflow) { > + if (flags & EXT4_FREE_BLOCKS_NOFREE_LAST_CLUSTER) { > + if (count > overflow) > + count -= overflow; > + else > + return; > + } else > + count += sbi->s_cluster_ratio - overflow; > + } > + > + if (!bh && (flags & EXT4_FREE_BLOCKS_FORGET)) { > + int i; > + int is_metadata = flags & EXT4_FREE_BLOCKS_METADATA; > + > + for (i = 0; i < count; i++) { > + cond_resched(); > + if (is_metadata) > + bh = sb_find_get_block(inode->i_sb, block + i); > + ext4_forget(handle, is_metadata, inode, bh, block + i); > + } > + } > + > + ext4_mb_clear_bb(handle, inode, block, count, flags); > + return; > +} > + > /** > * ext4_group_add_blocks() -- Add given blocks to an existing group > * @handle: handle to this transaction > -- > 2.31.1 > -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR