linux-ext4 - Re: [PATCH v3 20/20] ext4: simplify calculation of blkoff in ext4_mb_new_blocks

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 16 Mar 2023 18:19:40 +0800
From:   Kemeng Shi <shikemeng@...weicloud.com>
To:     Theodore Ts'o <tytso@....edu>
Cc:     adilger.kernel@...ger.ca, ojaswin@...ux.ibm.com,
        ritesh.list@...il.com,
        Harshad Shirwadkar <harshadshirwadkar@...il.com>,
        linux-ext4@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 20/20] ext4: simplify calculation of blkoff in
 ext4_mb_new_blocks_simple



on 3/16/2023 1:07 PM, Theodore Ts'o wrote:
> On Sat, Mar 04, 2023 at 01:21:20AM +0800, Kemeng Shi wrote:
>> We try to allocate a block from goal in ext4_mb_new_blocks_simple. We
>> only need get blkoff in first group with goal and set blkoff to 0 for
>> the rest groups.
>>
>> Signed-off-by: Kemeng Shi <shikemeng@...weicloud.com>
> 
> While this patch is OK as a simplification, there's a bigger problem
> with ext4_mb_new_blocks_simple(), and that is that we start looking
> for free blocks starting at the goal block, and then if we can't any
> free blocks by the time we get to the last block group.... we stop,
> and return ENOSPC.
> 
> This function is only used in the fast commit replay path, but for a
> very full file system, this could cause a fast commit replay to fail
> unnecesarily.  What we need to do is to try wrapping back to the
> beginning of the file system, and stop when we hit the original group
> (of the goal block) minus one.
> 
> We can fix this up in a separate patch, since this doesn't make things
> any worse, but essentially we need to do something like this:
Hi Theodore, thanks for feedback. I will submit another patchset for
mballoc and I would like to include this fix if no one else does. As
new patches may be conflicted with old ones I submited, I would submit
the new patchset after the old ones are fully reviewed and applied
if this fix is not in rush. Thanks!
> 
>     	maxgroups = ext4_get_groups_count(sb);
> 	for (g = 0; g < maxgroups ; g++) {
> 	
> 		...
> 		group++;
> 		if (group > maxgroups)
> 			group = 0;
> 	}
> 
> 					- Ted
> 

-- 
Best wishes
Kemeng Shi