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
