Re: [PATCH v3 20/20] ext4: simplify calculation of blkoff in ext4_mb_new_blocks_simple

[Kemeng Shi]

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@xxxxxxxxxxxxxxx>
> 
> 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