diff mbox series

[1/7] iommu/iova: fix incorrect variable types

Message ID 1490164067-12552-2-git-send-email-thunder.leizhen@huawei.com
State Superseded
Headers show
Series iommu/iova: improve the allocation performance of dma64 | expand

Commit Message

Leizhen (ThunderTown) March 22, 2017, 6:27 a.m. UTC
Keep these four variables type consistent with the paramters of function
__alloc_and_insert_iova_range and the members of struct iova:

1. static int __alloc_and_insert_iova_range(struct iova_domain *iovad,
		unsigned long size, unsigned long limit_pfn,

2. struct iova {
	unsigned long	pfn_hi;
	unsigned long	pfn_lo;

In fact, limit_pfn is most likely larger than 32 bits on DMA64.

Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>

---
 drivers/iommu/iova.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

-- 
2.5.0

Comments

Robin Murphy March 23, 2017, 11:42 a.m. UTC | #1
On 22/03/17 06:27, Zhen Lei wrote:
> Keep these four variables type consistent with the paramters of function

> __alloc_and_insert_iova_range and the members of struct iova:

> 

> 1. static int __alloc_and_insert_iova_range(struct iova_domain *iovad,

> 		unsigned long size, unsigned long limit_pfn,

> 

> 2. struct iova {

> 	unsigned long	pfn_hi;

> 	unsigned long	pfn_lo;

> 

> In fact, limit_pfn is most likely larger than 32 bits on DMA64.


FWIW if pad_size manages to overflow an int something's probably gone
horribly wrong, but there's no harm in making it consistent with
everything else here. However, given that patch #6 makes this irrelevant
anyway, do we really need to bother?

Robin.

> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>

> ---

>  drivers/iommu/iova.c | 6 +++---

>  1 file changed, 3 insertions(+), 3 deletions(-)

> 

> diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c

> index b7268a1..8ba8b496 100644

> --- a/drivers/iommu/iova.c

> +++ b/drivers/iommu/iova.c

> @@ -104,8 +104,8 @@ __cached_rbnode_delete_update(struct iova_domain *iovad, struct iova *free)

>   * Computes the padding size required, to make the start address

>   * naturally aligned on the power-of-two order of its size

>   */

> -static unsigned int

> -iova_get_pad_size(unsigned int size, unsigned int limit_pfn)

> +static unsigned long

> +iova_get_pad_size(unsigned long size, unsigned long limit_pfn)

>  {

>  	return (limit_pfn + 1 - size) & (__roundup_pow_of_two(size) - 1);

>  }

> @@ -117,7 +117,7 @@ static int __alloc_and_insert_iova_range(struct iova_domain *iovad,

>  	struct rb_node *prev, *curr = NULL;

>  	unsigned long flags;

>  	unsigned long saved_pfn;

> -	unsigned int pad_size = 0;

> +	unsigned long pad_size = 0;

>  

>  	/* Walk the tree backwards */

>  	spin_lock_irqsave(&iovad->iova_rbtree_lock, flags);

>
Leizhen (ThunderTown) March 24, 2017, 2:27 a.m. UTC | #2
On 2017/3/23 19:42, Robin Murphy wrote:
> On 22/03/17 06:27, Zhen Lei wrote:

>> Keep these four variables type consistent with the paramters of function

>> __alloc_and_insert_iova_range and the members of struct iova:

>>

>> 1. static int __alloc_and_insert_iova_range(struct iova_domain *iovad,

>> 		unsigned long size, unsigned long limit_pfn,

>>

>> 2. struct iova {

>> 	unsigned long	pfn_hi;

>> 	unsigned long	pfn_lo;

>>

>> In fact, limit_pfn is most likely larger than 32 bits on DMA64.

> 

> FWIW if pad_size manages to overflow an int something's probably gone

> horribly wrong, but there's no harm in making it consistent with

> everything else here. However, given that patch #6 makes this irrelevant

> anyway, do we really need to bother?


Because I'm not sure whether patch #6 can be applied or not.

> 

> Robin.

> 

>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>

>> ---

>>  drivers/iommu/iova.c | 6 +++---

>>  1 file changed, 3 insertions(+), 3 deletions(-)

>>

>> diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c

>> index b7268a1..8ba8b496 100644

>> --- a/drivers/iommu/iova.c

>> +++ b/drivers/iommu/iova.c

>> @@ -104,8 +104,8 @@ __cached_rbnode_delete_update(struct iova_domain *iovad, struct iova *free)

>>   * Computes the padding size required, to make the start address

>>   * naturally aligned on the power-of-two order of its size

>>   */

>> -static unsigned int

>> -iova_get_pad_size(unsigned int size, unsigned int limit_pfn)

>> +static unsigned long

>> +iova_get_pad_size(unsigned long size, unsigned long limit_pfn)

>>  {

>>  	return (limit_pfn + 1 - size) & (__roundup_pow_of_two(size) - 1);

>>  }

>> @@ -117,7 +117,7 @@ static int __alloc_and_insert_iova_range(struct iova_domain *iovad,

>>  	struct rb_node *prev, *curr = NULL;

>>  	unsigned long flags;

>>  	unsigned long saved_pfn;

>> -	unsigned int pad_size = 0;

>> +	unsigned long pad_size = 0;

>>  

>>  	/* Walk the tree backwards */

>>  	spin_lock_irqsave(&iovad->iova_rbtree_lock, flags);

>>

> 

> 

> .

> 


-- 
Thanks!
BestRegards
Leizhen (ThunderTown) March 31, 2017, 3:30 a.m. UTC | #3
On 2017/3/24 10:27, Leizhen (ThunderTown) wrote:
> 

> 

> On 2017/3/23 19:42, Robin Murphy wrote:

>> On 22/03/17 06:27, Zhen Lei wrote:

>>> Keep these four variables type consistent with the paramters of function

>>> __alloc_and_insert_iova_range and the members of struct iova:

>>>

>>> 1. static int __alloc_and_insert_iova_range(struct iova_domain *iovad,

>>> 		unsigned long size, unsigned long limit_pfn,

>>>

>>> 2. struct iova {

>>> 	unsigned long	pfn_hi;

>>> 	unsigned long	pfn_lo;

>>>

>>> In fact, limit_pfn is most likely larger than 32 bits on DMA64.

>>

>> FWIW if pad_size manages to overflow an int something's probably gone

>> horribly wrong, but there's no harm in making it consistent with

>> everything else here. However, given that patch #6 makes this irrelevant

>> anyway, do we really need to bother?

> 

> Because I'm not sure whether patch #6 can be applied or not.

So if Patch #6 can be applied, I can merge this patch and patch #6 into one.

> 

>>

>> Robin.

>>

>>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>

>>> ---

>>>  drivers/iommu/iova.c | 6 +++---

>>>  1 file changed, 3 insertions(+), 3 deletions(-)

>>>

>>> diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c

>>> index b7268a1..8ba8b496 100644

>>> --- a/drivers/iommu/iova.c

>>> +++ b/drivers/iommu/iova.c

>>> @@ -104,8 +104,8 @@ __cached_rbnode_delete_update(struct iova_domain *iovad, struct iova *free)

>>>   * Computes the padding size required, to make the start address

>>>   * naturally aligned on the power-of-two order of its size

>>>   */

>>> -static unsigned int

>>> -iova_get_pad_size(unsigned int size, unsigned int limit_pfn)

>>> +static unsigned long

>>> +iova_get_pad_size(unsigned long size, unsigned long limit_pfn)

>>>  {

>>>  	return (limit_pfn + 1 - size) & (__roundup_pow_of_two(size) - 1);

>>>  }

>>> @@ -117,7 +117,7 @@ static int __alloc_and_insert_iova_range(struct iova_domain *iovad,

>>>  	struct rb_node *prev, *curr = NULL;

>>>  	unsigned long flags;

>>>  	unsigned long saved_pfn;

>>> -	unsigned int pad_size = 0;

>>> +	unsigned long pad_size = 0;

>>>  

>>>  	/* Walk the tree backwards */

>>>  	spin_lock_irqsave(&iovad->iova_rbtree_lock, flags);

>>>

>>

>>

>> .

>>

> 


-- 
Thanks!
BestRegards
diff mbox series

Patch

diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
index b7268a1..8ba8b496 100644
--- a/drivers/iommu/iova.c
+++ b/drivers/iommu/iova.c
@@ -104,8 +104,8 @@  __cached_rbnode_delete_update(struct iova_domain *iovad, struct iova *free)
  * Computes the padding size required, to make the start address
  * naturally aligned on the power-of-two order of its size
  */
-static unsigned int
-iova_get_pad_size(unsigned int size, unsigned int limit_pfn)
+static unsigned long
+iova_get_pad_size(unsigned long size, unsigned long limit_pfn)
 {
 	return (limit_pfn + 1 - size) & (__roundup_pow_of_two(size) - 1);
 }
@@ -117,7 +117,7 @@  static int __alloc_and_insert_iova_range(struct iova_domain *iovad,
 	struct rb_node *prev, *curr = NULL;
 	unsigned long flags;
 	unsigned long saved_pfn;
-	unsigned int pad_size = 0;
+	unsigned long pad_size = 0;
 
 	/* Walk the tree backwards */
 	spin_lock_irqsave(&iovad->iova_rbtree_lock, flags);