| Message ID | 426fd12a24d7876e445aea3f14a6e09c2eba8fe3.1643642105.git.riteshh@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ext4: fast_commit fixes and some minor cleanups | expand |
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@linux.ibm.com> Yeah, the function was rather long. Feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> 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 >
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
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@linux.ibm.com> --- fs/ext4/mballoc.c | 180 ++++++++++++++++++++++++++-------------------- 1 file changed, 102 insertions(+), 78 deletions(-)