Subject: Re: [PATCH 03/21] ext4: avoid to use preallocated blocks if
 EXT4_MB_HINT_GOAL_ONLY is set
To:     Ojaswin Mujoo <ojaswin@linux.ibm.com>
Cc:     tytso@mit.edu, adilger.kernel@dilger.ca, jack@suse.cz,
        linux-ext4@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20230209194825.511043-1-shikemeng@huaweicloud.com>
 <20230209194825.511043-4-shikemeng@huaweicloud.com>
 <Y+n2+7pjkeb3vWQ7@li-bb2b2a4c-3307-11b2-a85c-8fa5c3a69313.ibm.com>
From:   Kemeng Shi <shikemeng@huaweicloud.com>
Message-ID: <f967d63b-907e-a495-3ec5-9043c35b9938@huaweicloud.com>
Date:   Mon, 13 Feb 2023 21:27:03 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101
 Thunderbird/60.5.0
MIME-Version: 1.0
In-Reply-To: <Y+n2+7pjkeb3vWQ7@li-bb2b2a4c-3307-11b2-a85c-8fa5c3a69313.ibm.com>
Content-Type: text/plain; charset=gbk
Content-Transfer-Encoding: 7bit
Precedence: bulk


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