[v1] dma: dmatest: Use div64_s64

[PATCH] dma: dmatest: Use div64_s64

Posted by coolrrsh@gmail.com 2 years, 5 months ago

From: Rajeshwar R Shinde <coolrrsh@gmail.com>

In the function do_div, the dividend is evaluated multiple times
so it can cause side effects. Therefore replace it with div64_s64.

This fixes warning such as:
drivers/dma/dmatest.c:496:1-7:
WARNING: do_div() does a 64-by-32 division,
please consider using div64_s64 instead.

Signed-off-by: Rajeshwar R Shinde <coolrrsh@gmail.com>
---
 drivers/dma/dmatest.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
index ffe621695e47..07042f239db8 100644
--- a/drivers/dma/dmatest.c
+++ b/drivers/dma/dmatest.c
@@ -9,6 +9,7 @@
 
 #include <linux/err.h>
 #include <linux/delay.h>
+#include <linux/math64.h>
 #include <linux/dma-mapping.h>
 #include <linux/dmaengine.h>
 #include <linux/freezer.h>
@@ -493,7 +494,7 @@ static unsigned long long dmatest_persec(s64 runtime, unsigned int val)
 
 	per_sec *= val;
 	per_sec = INT_TO_FIXPT(per_sec);
-	do_div(per_sec, runtime);
+	per_sec=div64_s64(per_sec, runtime);
 
 	return per_sec;
 }
-- 
2.25.1

RE: [PATCH] dma: dmatest: Use div64_s64

Posted by David Laight 2 years, 5 months ago

From: coolrrsh@gmail.com
> Sent: Wednesday, August 16, 2023 7:04 AM
> 
> From: Rajeshwar R Shinde <coolrrsh@gmail.com>
> 
> In the function do_div, the dividend is evaluated multiple times
> so it can cause side effects. Therefore replace it with div64_s64.

Nope, and even if it did it wouldn't matter here.

> This fixes warning such as:
> drivers/dma/dmatest.c:496:1-7:
> WARNING: do_div() does a 64-by-32 division,
> please consider using div64_s64 instead.

What you should really do is look carefully at the domain
of the input values to see if the division can actually
overflow.

That message is basically incorrect.

The full 64x64 divide is horribly slow on all 32bit archs.
It is even about twice as slow as the 64x32 divide on Intel
x86 cpu in 64bit mode (regardless of the values).

	David

> 
> Signed-off-by: Rajeshwar R Shinde <coolrrsh@gmail.com>
> ---
>  drivers/dma/dmatest.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
> index ffe621695e47..07042f239db8 100644
> --- a/drivers/dma/dmatest.c
> +++ b/drivers/dma/dmatest.c
> @@ -9,6 +9,7 @@
> 
>  #include <linux/err.h>
>  #include <linux/delay.h>
> +#include <linux/math64.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/dmaengine.h>
>  #include <linux/freezer.h>
> @@ -493,7 +494,7 @@ static unsigned long long dmatest_persec(s64 runtime, unsigned int val)
> 
>  	per_sec *= val;
>  	per_sec = INT_TO_FIXPT(per_sec);
> -	do_div(per_sec, runtime);
> +	per_sec=div64_s64(per_sec, runtime);
> 
>  	return per_sec;
>  }
> --
> 2.25.1

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH] dma: dmatest: Use div64_s64

Posted by Greg KH 2 years, 5 months ago

On Wed, Aug 16, 2023 at 11:34:00AM +0530, coolrrsh@gmail.com wrote:
> From: Rajeshwar R Shinde <coolrrsh@gmail.com>
> 
> In the function do_div, the dividend is evaluated multiple times
> so it can cause side effects. Therefore replace it with div64_s64.
> 
> This fixes warning such as:
> drivers/dma/dmatest.c:496:1-7:
> WARNING: do_div() does a 64-by-32 division,
> please consider using div64_s64 instead.
> 
> Signed-off-by: Rajeshwar R Shinde <coolrrsh@gmail.com>
> ---
>  drivers/dma/dmatest.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
> index ffe621695e47..07042f239db8 100644
> --- a/drivers/dma/dmatest.c
> +++ b/drivers/dma/dmatest.c
> @@ -9,6 +9,7 @@
>  
>  #include <linux/err.h>
>  #include <linux/delay.h>
> +#include <linux/math64.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/dmaengine.h>
>  #include <linux/freezer.h>
> @@ -493,7 +494,7 @@ static unsigned long long dmatest_persec(s64 runtime, unsigned int val)
>  
>  	per_sec *= val;
>  	per_sec = INT_TO_FIXPT(per_sec);
> -	do_div(per_sec, runtime);
> +	per_sec=div64_s64(per_sec, runtime);

Please always run checkpatch.pl on your changes before submitting them
for others to review.

thanks,

greg k-h

Re: [PATCH] dma: dmatest: Use div64_s64

Posted by Randy Dunlap 2 years, 5 months ago


On 8/16/23 08:16, Greg KH wrote:
> On Wed, Aug 16, 2023 at 11:34:00AM +0530, coolrrsh@gmail.com wrote:
>> From: Rajeshwar R Shinde <coolrrsh@gmail.com>
>>
>> In the function do_div, the dividend is evaluated multiple times
>> so it can cause side effects. Therefore replace it with div64_s64.
>>
>> This fixes warning such as:
>> drivers/dma/dmatest.c:496:1-7:
>> WARNING: do_div() does a 64-by-32 division,
>> please consider using div64_s64 instead.
>>
>> Signed-off-by: Rajeshwar R Shinde <coolrrsh@gmail.com>
>> ---
>>  drivers/dma/dmatest.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
>> index ffe621695e47..07042f239db8 100644
>> --- a/drivers/dma/dmatest.c
>> +++ b/drivers/dma/dmatest.c
>> @@ -9,6 +9,7 @@
>>  
>>  #include <linux/err.h>
>>  #include <linux/delay.h>
>> +#include <linux/math64.h>
>>  #include <linux/dma-mapping.h>
>>  #include <linux/dmaengine.h>
>>  #include <linux/freezer.h>
>> @@ -493,7 +494,7 @@ static unsigned long long dmatest_persec(s64 runtime, unsigned int val)
>>  
>>  	per_sec *= val;
>>  	per_sec = INT_TO_FIXPT(per_sec);
>> -	do_div(per_sec, runtime);
>> +	per_sec=div64_s64(per_sec, runtime);
> 
> Please always run checkpatch.pl on your changes before submitting them
> for others to review.

Also please tell us what tool produced that warning message.
Thanks.

-- 
~Randy