[PATCH] tracing: Explicitly cast divisor to fix Coccinelle warning

[PATCH] tracing: Explicitly cast divisor to fix Coccinelle warning

[Thorsten Blum]

Explicitly cast the divisor to u32 to fix a Coccinelle/coccicheck warning
reported by do_div.cocci.

Signed-off-by: Thorsten Blum <thorsten.blum@toblux.com>
---
 kernel/trace/trace_benchmark.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/trace_benchmark.c b/kernel/trace/trace_benchmark.c
index 54d5fa35c90a..218b40050629 100644
--- a/kernel/trace/trace_benchmark.c
+++ b/kernel/trace/trace_benchmark.c
@@ -105,7 +105,7 @@ static void trace_do_benchmark(void)
 		stddev = 0;
 
 	delta = bm_total;
-	do_div(delta, bm_cnt);
+	do_div(delta, (u32)bm_cnt);
 	avg = delta;
 
 	if (stddev > 0) {
-- 
2.44.0

Re: [PATCH] tracing: Explicitly cast divisor to fix Coccinelle warning

[Masami Hiramatsu]

On Mon, 18 Mar 2024 11:52:44 +0100
Thorsten Blum <thorsten.blum@toblux.com> wrote:

> Explicitly cast the divisor to u32 to fix a Coccinelle/coccicheck warning
> reported by do_div.cocci.
> 

Hmm, strange, trace_do_benchmark() has another do_div(u64, u64). 

                do {
                        last_seed = seed;
                        seed = stddev;
                        if (!last_seed)
                                break;
                        do_div(seed, last_seed);
                        seed += last_seed;
                        do_div(seed, 2);
                } while (i++ < 10 && last_seed != seed);

Didn't Coccinelle find that?

Thank you,

> Signed-off-by: Thorsten Blum <thorsten.blum@toblux.com>
> ---
>  kernel/trace/trace_benchmark.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/trace/trace_benchmark.c b/kernel/trace/trace_benchmark.c
> index 54d5fa35c90a..218b40050629 100644
> --- a/kernel/trace/trace_benchmark.c
> +++ b/kernel/trace/trace_benchmark.c
> @@ -105,7 +105,7 @@ static void trace_do_benchmark(void)
>  		stddev = 0;
>  
>  	delta = bm_total;
> -	do_div(delta, bm_cnt);
> +	do_div(delta, (u32)bm_cnt);
>  	avg = delta;
>  
>  	if (stddev > 0) {
> -- 
> 2.44.0
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH] tracing: Explicitly cast divisor to fix Coccinelle warning

[Thorsten Blum]

On 20. Mar 2024, at 11:27, Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> 
> Hmm, strange, trace_do_benchmark() has another do_div(u64, u64). 
> 
>                do {
>                        last_seed = seed;
>                        seed = stddev;
>                        if (!last_seed)
>                                break;
>                        do_div(seed, last_seed);
>                        seed += last_seed;
>                        do_div(seed, 2);
>                } while (i++ < 10 && last_seed != seed);
> 
> Didn't Coccinelle find that?

Coccinelle also finds this one, but please ignore this patch as I just realized
this was already fixed in another patch of mine from February.

Sorry for the inconvenience.

Link: https://lore.kernel.org/linux-kernel/20240225164507.232942-2-thorsten.blum@toblux.com/

Re: [PATCH] tracing: Explicitly cast divisor to fix Coccinelle warning

[Thorsten Blum]

On 20. Mar 2024, at 16:01, Thorsten Blum <thorsten.blum@toblux.com> wrote:
> 
> Coccinelle also finds this one, but please ignore this patch as I just realized
> this was already fixed in another patch of mine from February.
> 
> Sorry for the inconvenience.
> 
> Link: https://lore.kernel.org/linux-kernel/20240225164507.232942-2-thorsten.blum@toblux.com/

Actually, I will submit a new patch to revert

	delta = div64_u64(delta, bm_cnt);

back to

	do_div(delta, bm_cnt);

but this time include an explicit cast to u32

	do_div(delta, (u32)bm_cnt);

to remove the Coccinelle warning reported by do_div.cocci and to improve
performance.

The do_div() macro does a 64-by-32 division which is faster than the 64-by-64 
division done by div64_u64(). Casting the divisor bm_cnt to u32 is safe since 
we return early from trace_do_benchmark() if bm_cnt > UINT_MAX (something I 
missed in d6cb38e10810).

This approach is already used when calculating the standard deviation:

	do_div(stddev, (u32)bm_cnt);
	do_div(stddev, (u32)bm_cnt - 1);

Thanks,
Thorsten

[PATCH v2] tracing: Improve performance by using do_div()

[Thorsten Blum]

Partially revert commit d6cb38e10810 ("tracing: Use div64_u64() instead
of do_div()") and use do_div() again to utilize its faster 64-by-32
division compared to the 64-by-64 division done by div64_u64().

Explicitly cast the divisor bm_cnt to u32 to prevent a Coccinelle
warning reported by do_div.cocci. The warning was removed with commit
d6cb38e10810 ("tracing: Use div64_u64() instead of do_div()").

Using the faster 64-by-32 division and casting bm_cnt to u32 is safe
because we return early from trace_do_benchmark() if bm_cnt > UINT_MAX.
This approach is already used twice in trace_do_benchmark() when
calculating the standard deviation:

	do_div(stddev, (u32)bm_cnt);
	do_div(stddev, (u32)bm_cnt - 1);

Signed-off-by: Thorsten Blum <thorsten.blum@toblux.com>
---
Changes in v2:
- Update patch with latest changes from master
- Update patch title and description
---
 kernel/trace/trace_benchmark.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/trace_benchmark.c b/kernel/trace/trace_benchmark.c
index 811b08439406..e19c32f2a938 100644
--- a/kernel/trace/trace_benchmark.c
+++ b/kernel/trace/trace_benchmark.c
@@ -104,7 +104,7 @@ static void trace_do_benchmark(void)
 		stddev = 0;
 
 	delta = bm_total;
-	delta = div64_u64(delta, bm_cnt);
+	do_div(delta, (u32)bm_cnt);
 	avg = delta;
 
 	if (stddev > 0) {
-- 
2.44.0

[RESEND PATCH v2] tracing: Improve performance by using do_div()

[Thorsten Blum]

Partially revert commit d6cb38e10810 ("tracing: Use div64_u64() instead
of do_div()") and use do_div() again to utilize its faster 64-by-32
division compared to the 64-by-64 division done by div64_u64().

Explicitly cast the divisor bm_cnt to u32 to prevent a Coccinelle
warning reported by do_div.cocci. The warning was removed with commit
d6cb38e10810 ("tracing: Use div64_u64() instead of do_div()").

Using the faster 64-by-32 division and casting bm_cnt to u32 is safe
because we return early from trace_do_benchmark() if bm_cnt > UINT_MAX.
This approach is already used twice in trace_do_benchmark() when
calculating the standard deviation:

	do_div(stddev, (u32)bm_cnt);
	do_div(stddev, (u32)bm_cnt - 1);

Signed-off-by: Thorsten Blum <thorsten.blum@toblux.com>
---
Changes in v2:
- Update patch with latest changes from master
- Update patch title and description
---
 kernel/trace/trace_benchmark.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/trace_benchmark.c b/kernel/trace/trace_benchmark.c
index 811b08439406..e19c32f2a938 100644
--- a/kernel/trace/trace_benchmark.c
+++ b/kernel/trace/trace_benchmark.c
@@ -104,7 +104,7 @@ static void trace_do_benchmark(void)
 		stddev = 0;
 
 	delta = bm_total;
-	delta = div64_u64(delta, bm_cnt);
+	do_div(delta, (u32)bm_cnt);
 	avg = delta;
 
 	if (stddev > 0) {
-- 
2.44.0