Re: [PATCH 03/21] ext4: avoid to use preallocated blocks if EXT4_MB_HINT_GOAL_ONLY is set

[Kemeng Shi <shikemeng@huaweicloud.com>]

on 2/13/2023 4:38 PM, Ojaswin Mujoo wrote:
> On Fri, Feb 10, 2023 at 03:48:07AM +0800, Kemeng Shi wrote:
>> ext4_mb_use_preallocated will ignore the demand to alloc at goal block
>> only. Return false if EXT4_MB_HINT_GOAL_ONLY is set before use
>> preallocated blocks in ext4_mb_use_preallocated.
>>
>> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
>> ---
>>  fs/ext4/mballoc.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
>> index 375d9655b525..352ac9139fee 100644
>> --- a/fs/ext4/mballoc.c
>> +++ b/fs/ext4/mballoc.c
>> @@ -4368,6 +4368,9 @@ ext4_mb_use_preallocated(struct ext4_allocation_context *ac)
>>  	if (!(ac->ac_flags & EXT4_MB_HINT_DATA))
>>  		return false;
>>  
>> +	if (unlikely(ac->ac_flags & EXT4_MB_HINT_GOAL_ONLY))
>> +		return false;
>> +
>>  	/* first, try per-file preallocation */
>>  	rcu_read_lock();
>>  	list_for_each_entry_rcu(pa, &ei->i_prealloc_list, pa_inode_list) {
>> -- 
>> 2.30.0
>>
> So with the flag, even when we request for a logical/goal block
> combination that can be satisfied by one of the inode PAs in the list,
> we would still exit early and the allocation would eventually fail since
> the goal block would be marked "in-use" in the buddy. This doesn't seem
> to be desirable.
> 
> Maybe instead of exiting early we should try to find a PA that satisfies
> the logical block we are asking for and then incase of
> EXT4_MB_HINT_GOAL_ONLY, we can add a check to see if the physical block
> of the PA corresponds to the goal block. Would like to hear your
> thoughts on this.
Yes, this is a more thoughtful, I will improve this in next version.
Besides, maybe we should also test length if EXT4_MB_HINT_MERGE is set
as ext4_mb_find_by_goal do.

> Regards,
> ojaswin
> 

-- 
Best wishes
Kemeng Shi