Re: [PATCH 1/1] Revert "iommu/iova: Retry from last rb tree node if iova search fails"

[Robin Murphy <robin.murphy@arm.com>]

On 2021-01-29 09:48, Leizhen (ThunderTown) wrote:
> 
> Currently, we are thinking about the solution to the problem. However, because the end time of v5.11 is approaching, this patch is sent first.

However, that commit was made for a reason - how do we justify that one 
thing being slow is more important than another thing being completely 
broken? It's not practical to just keep doing the patch hokey-cokey 
based on whoever shouts loudest :(

> On 2021/1/29 17:21, Zhen Lei wrote:
>> This reverts commit 4e89dce725213d3d0b0475211b500eda4ef4bf2f.
>>
>> We find that this patch has a great impact on performance. According to
>> our test: the iops decreases from 1655.6K to 893.5K, about half.
>>
>> Hardware: 1 SAS expander with 12 SAS SSD
>> Command:  Only the main parameters are listed.
>>            fio bs=4k rw=read iodepth=128 cpus_allowed=0-127

FWIW, I'm 99% sure that what you really want is [1], but then you get to 
battle against an unknown quantity of dodgy firmware instead.

Robin.

[1] 
https://lore.kernel.org/linux-iommu/d412c292d222eb36469effd338e985f9d9e24cd6.1594207679.git.robin.murphy@arm.com/

>> Fixes: 4e89dce72521 ("iommu/iova: Retry from last rb tree node if iova search fails")
>> Tested-by: Xiang Chen <chenxiang66@hisilicon.com>
>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>> ---
>>   drivers/iommu/iova.c | 23 ++++++-----------------
>>   1 file changed, 6 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
>> index d20b8b333d30d17..f840c7207efbced 100644
>> --- a/drivers/iommu/iova.c
>> +++ b/drivers/iommu/iova.c
>> @@ -185,9 +185,8 @@ static int __alloc_and_insert_iova_range(struct iova_domain *iovad,
>>   	struct rb_node *curr, *prev;
>>   	struct iova *curr_iova;
>>   	unsigned long flags;
>> -	unsigned long new_pfn, retry_pfn;
>> +	unsigned long new_pfn;
>>   	unsigned long align_mask = ~0UL;
>> -	unsigned long high_pfn = limit_pfn, low_pfn = iovad->start_pfn;
>>   
>>   	if (size_aligned)
>>   		align_mask <<= fls_long(size - 1);
>> @@ -200,25 +199,15 @@ static int __alloc_and_insert_iova_range(struct iova_domain *iovad,
>>   
>>   	curr = __get_cached_rbnode(iovad, limit_pfn);
>>   	curr_iova = rb_entry(curr, struct iova, node);
>> -	retry_pfn = curr_iova->pfn_hi + 1;
>> -
>> -retry:
>>   	do {
>> -		high_pfn = min(high_pfn, curr_iova->pfn_lo);
>> -		new_pfn = (high_pfn - size) & align_mask;
>> +		limit_pfn = min(limit_pfn, curr_iova->pfn_lo);
>> +		new_pfn = (limit_pfn - size) & align_mask;
>>   		prev = curr;
>>   		curr = rb_prev(curr);
>>   		curr_iova = rb_entry(curr, struct iova, node);
>> -	} while (curr && new_pfn <= curr_iova->pfn_hi && new_pfn >= low_pfn);
>> -
>> -	if (high_pfn < size || new_pfn < low_pfn) {
>> -		if (low_pfn == iovad->start_pfn && retry_pfn < limit_pfn) {
>> -			high_pfn = limit_pfn;
>> -			low_pfn = retry_pfn;
>> -			curr = &iovad->anchor.node;
>> -			curr_iova = rb_entry(curr, struct iova, node);
>> -			goto retry;
>> -		}
>> +	} while (curr && new_pfn <= curr_iova->pfn_hi);
>> +
>> +	if (limit_pfn < size || new_pfn < iovad->start_pfn) {
>>   		iovad->max32_alloc_size = size;
>>   		goto iova32_full;
>>   	}
>>
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu