Re: [PATCH v2] crypto: Fix divide error in do_xor_speed()
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()
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()
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()
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;