Re: [PATCH v2] crypto: Fix divide error in do_xor_speed()

2021-01-07 Thread Herbert Xu
On Tue, Jan 05, 2021 at 01:24:18PM -0800, Doug Anderson wrote:
>
> ...so while I think your fix will avoid the crash and could land as a
> stopgap, it's a sign that we need to run more repetitions on your
> particular setup to get accurate timings.  Your patch will probably
> cause it to just randomly pick one of the implementations.

We can always do a follow-up patch.

Thanks,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt


Re: [PATCH v2] crypto: Fix divide error in do_xor_speed()

2021-01-07 Thread Herbert Xu
On Thu, Dec 31, 2020 at 12:33:18AM +0300, Kirill Tkhai wrote:
> crypto: Fix divide error in do_xor_speed()
> 
> From: Kirill Tkhai 
> 
> Latest (but not only latest) linux-next panics with divide
> error on my QEMU setup.
> 
> The patch at the bottom of this message fixes the problem.
> 
> xor: measuring software checksum speed
> divide error:  [#1] PREEMPT SMP KASAN
> PREEMPT SMP KASAN
> CPU: 3 PID: 1 Comm: swapper/0 Not tainted 5.10.0-next-20201223+ #2177
> RIP: 0010:do_xor_speed+0xbb/0xf3
> Code: 41 ff cc 75 b5 bf 01 00 00 00 e8 3d 23 8b fe 65 8b 05 f6 49 83 7d 85 c0 
> 75 05 e8
>  84 70 81 fe b8 00 00 50 c3 31 d2 48 8d 7b 10  f5 41 89 c4 e8 58 07 a2 fe 
> 44 89 63 10 48 8d 7b 08
>  e8 cb 07 a2
> RSP: :888100137dc8 EFLAGS: 00010246
> RAX: c350 RBX: 823f0160 RCX: 
> RDX:  RSI: 0808 RDI: 823f0170
> RBP:  R08: 8109c50f R09: 824bb6f7
> R10: fbfff04976de R11: 0001 R12: 
> R13: 888101997000 R14: 888101994000 R15: 823f0178
> FS:  () GS:8881f778() knlGS:
> CS:  0010 DS:  ES:  CR0: 80050033
> CR2:  CR3: 0220e000 CR4: 06a0
> Call Trace:
>  calibrate_xor_blocks+0x13c/0x1c4
>  ? do_xor_speed+0xf3/0xf3
>  do_one_initcall+0xc1/0x1b7
>  ? start_kernel+0x373/0x373
>  ? unpoison_range+0x3a/0x60
>  kernel_init_freeable+0x1dd/0x238
>  ? rest_init+0xc6/0xc6
>  kernel_init+0x8/0x10a
>  ret_from_fork+0x1f/0x30
> ---[ end trace 5bd3c1d0b2da ]---
> 
> Fixes: c055e3eae0f1 ("crypto: xor - use ktime for template benchmarking")
> Signed-off-by: Kirill Tkhai 
> Acked-by: Ard Biesheuvel 
> ---
> 
> v2: New Year resend :) Added fixes tag.
>  crypto/xor.c |2 ++
>  1 file changed, 2 insertions(+)

Patch applied.  Thanks.
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt


Re: [PATCH v2] crypto: Fix divide error in do_xor_speed()

2021-01-05 Thread Doug Anderson
Hi,

On Wed, Dec 30, 2020 at 1:34 PM Kirill Tkhai  wrote:
>
> crypto: Fix divide error in do_xor_speed()
>
> From: Kirill Tkhai 
>
> Latest (but not only latest) linux-next panics with divide
> error on my QEMU setup.
>
> The patch at the bottom of this message fixes the problem.
>
> xor: measuring software checksum speed
> divide error:  [#1] PREEMPT SMP KASAN
> PREEMPT SMP KASAN
> CPU: 3 PID: 1 Comm: swapper/0 Not tainted 5.10.0-next-20201223+ #2177
> RIP: 0010:do_xor_speed+0xbb/0xf3
> Code: 41 ff cc 75 b5 bf 01 00 00 00 e8 3d 23 8b fe 65 8b 05 f6 49 83 7d 85 c0 
> 75 05 e8
>  84 70 81 fe b8 00 00 50 c3 31 d2 48 8d 7b 10  f5 41 89 c4 e8 58 07 a2 fe 
> 44 89 63 10 48 8d 7b 08
>  e8 cb 07 a2
> RSP: :888100137dc8 EFLAGS: 00010246
> RAX: c350 RBX: 823f0160 RCX: 
> RDX:  RSI: 0808 RDI: 823f0170
> RBP:  R08: 8109c50f R09: 824bb6f7
> R10: fbfff04976de R11: 0001 R12: 
> R13: 888101997000 R14: 888101994000 R15: 823f0178
> FS:  () GS:8881f778() knlGS:
> CS:  0010 DS:  ES:  CR0: 80050033
> CR2:  CR3: 0220e000 CR4: 06a0
> Call Trace:
>  calibrate_xor_blocks+0x13c/0x1c4
>  ? do_xor_speed+0xf3/0xf3
>  do_one_initcall+0xc1/0x1b7
>  ? start_kernel+0x373/0x373
>  ? unpoison_range+0x3a/0x60
>  kernel_init_freeable+0x1dd/0x238
>  ? rest_init+0xc6/0xc6
>  kernel_init+0x8/0x10a
>  ret_from_fork+0x1f/0x30
> ---[ end trace 5bd3c1d0b2da ]---
>
> Fixes: c055e3eae0f1 ("crypto: xor - use ktime for template benchmarking")
> Signed-off-by: Kirill Tkhai 
> Acked-by: Ard Biesheuvel 
> ---
>
> v2: New Year resend :) Added fixes tag.
>  crypto/xor.c |2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/crypto/xor.c b/crypto/xor.c
> index eacbf4f93990..8f899f898ec9 100644
> --- a/crypto/xor.c
> +++ b/crypto/xor.c
> @@ -107,6 +107,8 @@ do_xor_speed(struct xor_block_template *tmpl, void *b1, 
> void *b2)
> preempt_enable();
>
> // bytes/ns == GB/s, multiply by 1000 to get MB/s [not MiB/s]
> +   if (!min)
> +   min = 1;

I guess this is if you just have a ktime backend that is not granular
enough for this measurement?  So if ktime is backed by a 32kHz clock
then ktime might increment in ~30us increments and maybe we ran in
less time than that?

...so while I think your fix will avoid the crash and could land as a
stopgap, it's a sign that we need to run more repetitions on your
particular setup to get accurate timings.  Your patch will probably
cause it to just randomly pick one of the implementations.

Presumably the right thing to do would be to look at
ktime_get_resolution_ns().  If "diff" is ever less than
"ktime_get_resolution_ns() * 10" then we should ramp up the number of
repetitions and try again.  The extra "* 10" is to make sure that we'd
be able to tell the difference between faster and slower algorithms.
Perhaps it should actually be more like * 50 or * 100.

-Doug


[PATCH v2] crypto: Fix divide error in do_xor_speed()

2020-12-30 Thread Kirill Tkhai
crypto: Fix divide error in do_xor_speed()

From: Kirill Tkhai 

Latest (but not only latest) linux-next panics with divide
error on my QEMU setup.

The patch at the bottom of this message fixes the problem.

xor: measuring software checksum speed
divide error:  [#1] PREEMPT SMP KASAN
PREEMPT SMP KASAN
CPU: 3 PID: 1 Comm: swapper/0 Not tainted 5.10.0-next-20201223+ #2177
RIP: 0010:do_xor_speed+0xbb/0xf3
Code: 41 ff cc 75 b5 bf 01 00 00 00 e8 3d 23 8b fe 65 8b 05 f6 49 83 7d 85 c0 
75 05 e8
 84 70 81 fe b8 00 00 50 c3 31 d2 48 8d 7b 10  f5 41 89 c4 e8 58 07 a2 fe 
44 89 63 10 48 8d 7b 08
 e8 cb 07 a2
RSP: :888100137dc8 EFLAGS: 00010246
RAX: c350 RBX: 823f0160 RCX: 
RDX:  RSI: 0808 RDI: 823f0170
RBP:  R08: 8109c50f R09: 824bb6f7
R10: fbfff04976de R11: 0001 R12: 
R13: 888101997000 R14: 888101994000 R15: 823f0178
FS:  () GS:8881f778() knlGS:
CS:  0010 DS:  ES:  CR0: 80050033
CR2:  CR3: 0220e000 CR4: 06a0
Call Trace:
 calibrate_xor_blocks+0x13c/0x1c4
 ? do_xor_speed+0xf3/0xf3
 do_one_initcall+0xc1/0x1b7
 ? start_kernel+0x373/0x373
 ? unpoison_range+0x3a/0x60
 kernel_init_freeable+0x1dd/0x238
 ? rest_init+0xc6/0xc6
 kernel_init+0x8/0x10a
 ret_from_fork+0x1f/0x30
---[ end trace 5bd3c1d0b2da ]---

Fixes: c055e3eae0f1 ("crypto: xor - use ktime for template benchmarking")
Signed-off-by: Kirill Tkhai 
Acked-by: Ard Biesheuvel 
---

v2: New Year resend :) Added fixes tag.
 crypto/xor.c |2 ++
 1 file changed, 2 insertions(+)

diff --git a/crypto/xor.c b/crypto/xor.c
index eacbf4f93990..8f899f898ec9 100644
--- a/crypto/xor.c
+++ b/crypto/xor.c
@@ -107,6 +107,8 @@ do_xor_speed(struct xor_block_template *tmpl, void *b1, 
void *b2)
preempt_enable();
 
// bytes/ns == GB/s, multiply by 1000 to get MB/s [not MiB/s]
+   if (!min)
+   min = 1;
speed = (1000 * REPS * BENCH_SIZE) / (unsigned int)ktime_to_ns(min);
tmpl->speed = speed;