Re: [PATCH 2/7] iommu/iova: cut down judgement times

2017-03-30 Thread Leizhen (ThunderTown)


On 2017/3/23 20:11, Robin Murphy wrote:
> On 22/03/17 06:27, Zhen Lei wrote:
>> Below judgement can only be satisfied at the last time, which produced 2N
>> judgements(suppose N times failed, 0 or 1 time successed) in vain.
>>
>> if ((pfn >= iova->pfn_lo) && (pfn <= iova->pfn_hi)) {
>>  return iova;
>> }
> 
> For me, GCC (6.2.1 AArch64) seems to do a pretty good job of this
> function already, so this change only saves two instructions in total
> (pfn is compared against pfn_lo only once instead of twice), which I
> wouldn't expect to see a noticeable performance effect from.
OK, thanks for your careful analysis.

Although only two instructions saved in each loop iteration, but it's also an 
improvment and no harm.

> 
> Given the improvement in readability, though, I don't even care about
> any codegen differences :)
> 
> Reviewed-by: Robin Murphy 
> 
>> Signed-off-by: Zhen Lei 
>> ---
>>  drivers/iommu/iova.c | 9 +++--
>>  1 file changed, 3 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
>> index 8ba8b496..1c49969 100644
>> --- a/drivers/iommu/iova.c
>> +++ b/drivers/iommu/iova.c
>> @@ -312,15 +312,12 @@ private_find_iova(struct iova_domain *iovad, unsigned 
>> long pfn)
>>  while (node) {
>>  struct iova *iova = rb_entry(node, struct iova, node);
>>  
>> -/* If pfn falls within iova's range, return iova */
>> -if ((pfn >= iova->pfn_lo) && (pfn <= iova->pfn_hi)) {
>> -return iova;
>> -}
>> -
>>  if (pfn < iova->pfn_lo)
>>  node = node->rb_left;
>> -else if (pfn > iova->pfn_lo)
>> +else if (pfn > iova->pfn_hi)
>>  node = node->rb_right;
>> +else
>> +return iova;/* pfn falls within iova's range */
>>  }
>>  
>>  return NULL;
>>
> 
> 
> .
> 

-- 
Thanks!
BestRegards

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 2/7] iommu/iova: cut down judgement times

2017-03-23 Thread Robin Murphy
On 22/03/17 06:27, Zhen Lei wrote:
> Below judgement can only be satisfied at the last time, which produced 2N
> judgements(suppose N times failed, 0 or 1 time successed) in vain.
> 
> if ((pfn >= iova->pfn_lo) && (pfn <= iova->pfn_hi)) {
>   return iova;
> }

For me, GCC (6.2.1 AArch64) seems to do a pretty good job of this
function already, so this change only saves two instructions in total
(pfn is compared against pfn_lo only once instead of twice), which I
wouldn't expect to see a noticeable performance effect from.

Given the improvement in readability, though, I don't even care about
any codegen differences :)

Reviewed-by: Robin Murphy 

> Signed-off-by: Zhen Lei 
> ---
>  drivers/iommu/iova.c | 9 +++--
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
> index 8ba8b496..1c49969 100644
> --- a/drivers/iommu/iova.c
> +++ b/drivers/iommu/iova.c
> @@ -312,15 +312,12 @@ private_find_iova(struct iova_domain *iovad, unsigned 
> long pfn)
>   while (node) {
>   struct iova *iova = rb_entry(node, struct iova, node);
>  
> - /* If pfn falls within iova's range, return iova */
> - if ((pfn >= iova->pfn_lo) && (pfn <= iova->pfn_hi)) {
> - return iova;
> - }
> -
>   if (pfn < iova->pfn_lo)
>   node = node->rb_left;
> - else if (pfn > iova->pfn_lo)
> + else if (pfn > iova->pfn_hi)
>   node = node->rb_right;
> + else
> + return iova;/* pfn falls within iova's range */
>   }
>  
>   return NULL;
> 

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu