[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <875y6eo0ad.fsf@doe.com>
Date: Fri, 21 Jul 2023 00:52:50 +0530
From: Ritesh Harjani (IBM) <ritesh.list@...il.com>
To: Baokun Li <libaokun1@...wei.com>, linux-ext4@...r.kernel.org
Cc: tytso@....edu, adilger.kernel@...ger.ca, jack@...e.cz,
ojaswin@...ux.ibm.com, linux-kernel@...r.kernel.org,
yi.zhang@...wei.com, yangerkun@...wei.com, yukuai3@...wei.com,
libaokun1@...wei.com
Subject: Re: [PATCH 4/4] ext4: avoid prealloc space being skipped due to overflow
Baokun Li <libaokun1@...wei.com> writes:
> If there is a pa in the i_prealloc_list of an inode with a tmp_pa_end
> of 4294967296(0x100000000), since tmp_pa_end is of type ext4_lblk_t,
> tmp_pa_end will be recognized as 0 due to overflow, which causes
> (ac->ac_o_ ex.fe_logical >= tmp_pa_end) always holds, so that pa will
> always be skipped. This then triggers the regular allocation process,
> and if the excess tail of the free extent from that allocation is put
> into the i_prealloc_list again, it will again not be used. This ends up
> leaving us with a lot of free physical blocks in the i_prealloc_list.
>
> We avoid this problem by using pa_end() to compute tmp_pa_end and
> declaring tmp_pa_end to be of type loff_t.
>
> Signed-off-by: Baokun Li <libaokun1@...wei.com>
Looks good to me with fex_end() and pa_end() dropped.
-ritesh
> ---
> fs/ext4/mballoc.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 77d47af525d9..06db40fb29d6 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -4765,7 +4765,8 @@ ext4_mb_use_preallocated(struct ext4_allocation_context *ac)
> struct ext4_inode_info *ei = EXT4_I(ac->ac_inode);
> struct ext4_locality_group *lg;
> struct ext4_prealloc_space *tmp_pa, *cpa = NULL;
> - ext4_lblk_t tmp_pa_start, tmp_pa_end;
> + ext4_lblk_t tmp_pa_start;
> + loff_t tmp_pa_end;
> struct rb_node *iter;
> ext4_fsblk_t goal_block;
>
> @@ -4784,7 +4785,7 @@ ext4_mb_use_preallocated(struct ext4_allocation_context *ac)
> /* all fields in this condition don't change,
> * so we can skip locking for them */
> tmp_pa_start = tmp_pa->pa_lstart;
> - tmp_pa_end = tmp_pa->pa_lstart + EXT4_C2B(sbi, tmp_pa->pa_len);
> + tmp_pa_end = pa_end(sbi, tmp_pa);
>
> /* original request start doesn't lie in this PA */
> if (ac->ac_o_ex.fe_logical < tmp_pa_start ||
> --
> 2.31.1
Powered by blists - more mailing lists