| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20220201114059.chy3rauebk3ytszx@quack3.lan>
Date: Tue, 1 Feb 2022 12:40:59 +0100
From: Jan Kara <jack@...e.cz>
To: Ritesh Harjani <riteshh@...ux.ibm.com>
Cc: linux-ext4@...r.kernel.org, linux-fsdevel@...r.kernel.org,
Theodore Ts'o <tytso@....edu>, Jan Kara <jack@...e.cz>,
Harshad Shirwadkar <harshadshirwadkar@...il.com>
Subject: Re: [RFC 5/6] ext4: Refactor ext4_free_blocks() to pull out
ext4_mb_clear_bb()
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@...ux.ibm.com>
Yeah, the function was rather long. Feel free to add:
Reviewed-by: Jan Kara <jack@...e.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
>
--
Jan Kara <jack@...e.com>
SUSE Labs, CR
Powered by blists - more mailing lists