diff mbox series

gpu: drm: msm: use div64_u64() instead of do_div()

Message ID 1644395837-3845-1-git-send-email-wangqing@vivo.com (mailing list archive)
State Not Applicable
Headers show
Series gpu: drm: msm: use div64_u64() instead of do_div() | expand

Commit Message

王擎 Feb. 9, 2022, 8:37 a.m. UTC
From: Wang Qing <wangqing@vivo.com>

do_div() does a 64-by-32 division.
When the divisor is u64, do_div() truncates it to 32 bits, this means it
can test non-zero and be truncated to zero for division.

fix do_div.cocci warning:
do_div() does a 64-by-32 division, please consider using div64_u64 instead.

Signed-off-by: Wang Qing <wangqing@vivo.com>
---
 drivers/gpu/drm/msm/msm_gpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Dmitry Baryshkov Feb. 9, 2022, 10:17 p.m. UTC | #1
On 09/02/2022 11:37, Qing Wang wrote:
> From: Wang Qing <wangqing@vivo.com>
> 
> do_div() does a 64-by-32 division.
> When the divisor is u64, do_div() truncates it to 32 bits, this means it
> can test non-zero and be truncated to zero for division.
> 
> fix do_div.cocci warning:
> do_div() does a 64-by-32 division, please consider using div64_u64 instead.
> 
> Signed-off-by: Wang Qing <wangqing@vivo.com>

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

> ---
>   drivers/gpu/drm/msm/msm_gpu.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
> index 2c1049c..aa4617b
> --- a/drivers/gpu/drm/msm/msm_gpu.c
> +++ b/drivers/gpu/drm/msm/msm_gpu.c
> @@ -648,7 +648,7 @@ static void retire_submit(struct msm_gpu *gpu, struct msm_ringbuffer *ring,
>   	/* Calculate the clock frequency from the number of CP cycles */
>   	if (elapsed) {
>   		clock = (stats->cpcycles_end - stats->cpcycles_start) * 1000;
> -		do_div(clock, elapsed);
> +		div64_u64(clock, elapsed);
>   	}
>   
>   	trace_msm_gpu_submit_retired(submit, elapsed, clock,
Dmitry Baryshkov Feb. 10, 2022, 9:50 a.m. UTC | #2
On 10/02/2022 01:17, Dmitry Baryshkov wrote:
> On 09/02/2022 11:37, Qing Wang wrote:
>> From: Wang Qing <wangqing@vivo.com>
>>
>> do_div() does a 64-by-32 division.
>> When the divisor is u64, do_div() truncates it to 32 bits, this means it
>> can test non-zero and be truncated to zero for division.
>>
>> fix do_div.cocci warning:
>> do_div() does a 64-by-32 division, please consider using div64_u64 
>> instead.
>>
>> Signed-off-by: Wang Qing <wangqing@vivo.com>
> 
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

After rechecking, I'd like to withdraw my R-B tag (Minecrell, thanks for 
pointing this out!)

The div64_u64 is not equivalent to do_div. It returns the quotient 
rather than modifying the first arg. Moreover it is unoptimal on 32-bit 
arches.

I'd suggest changing the math to remove multiplications by 1000 and 
10000 before division. Or just ignoring this at all, judging from the 
fact that these values are used only for tracing rather than actual 
calculations.

> 
>> ---
>>   drivers/gpu/drm/msm/msm_gpu.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/msm/msm_gpu.c 
>> b/drivers/gpu/drm/msm/msm_gpu.c
>> index 2c1049c..aa4617b
>> --- a/drivers/gpu/drm/msm/msm_gpu.c
>> +++ b/drivers/gpu/drm/msm/msm_gpu.c
>> @@ -648,7 +648,7 @@ static void retire_submit(struct msm_gpu *gpu, 
>> struct msm_ringbuffer *ring,
>>       /* Calculate the clock frequency from the number of CP cycles */
>>       if (elapsed) {
>>           clock = (stats->cpcycles_end - stats->cpcycles_start) * 1000;
>> -        do_div(clock, elapsed);
>> +        div64_u64(clock, elapsed);
>>       }
>>       trace_msm_gpu_submit_retired(submit, elapsed, clock,
> 
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index 2c1049c..aa4617b
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -648,7 +648,7 @@  static void retire_submit(struct msm_gpu *gpu, struct msm_ringbuffer *ring,
 	/* Calculate the clock frequency from the number of CP cycles */
 	if (elapsed) {
 		clock = (stats->cpcycles_end - stats->cpcycles_start) * 1000;
-		do_div(clock, elapsed);
+		div64_u64(clock, elapsed);
 	}
 
 	trace_msm_gpu_submit_retired(submit, elapsed, clock,