All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Jonathan Neuschäfer" <j.neuschaefer@gmx.net>
To: "Jason A. Donenfeld" <Jason@zx2c4.com>
Cc: "Andy Lutomirski" <luto@amacapital.net>,
	"Jonathan Neuschäfer" <j.neuschaefer@gmx.net>,
	"Theodore Ts'o" <tytso@mit.edu>,
	LKML <linux-kernel@vger.kernel.org>,
	"Sebastian Andrzej Siewior" <bigeasy@linutronix.de>,
	"Peter Zijlstra" <peterz@infradead.org>,
	"Ingo Molnar" <mingo@redhat.com>, "Will Deacon" <will@kernel.org>,
	"Waiman Long" <longman@redhat.com>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	sultan@kerneltoast.com, "Andy Lutomirski" <luto@kernel.org>
Subject: Re: [PATCH v2] random: remove batched entropy locking
Date: Sat, 29 Jan 2022 22:03:34 +0100	[thread overview]
Message-ID: <YfWrpoQIR9+NDGHl@latitude> (raw)
In-Reply-To: <20220128223548.97807-1-Jason@zx2c4.com>

[-- Attachment #1: Type: text/plain, Size: 4922 bytes --]

Hi Jason,

On Fri, Jan 28, 2022 at 11:35:48PM +0100, Jason A. Donenfeld wrote:
> Rather than use spinlocks to protect batched entropy, we can instead
> disable interrupts locally, since we're dealing with per-cpu data, and
> manage resets with a basic generation counter. This should fix up the
> below splat that Jonathan received with a PROVE_RAW_LOCK_NESTING=y
> kernel.
> 
> Note that Sebastian has pointed out a few other areas where
> using spinlock_t in an IRQ context is potentially problematic for
> PREEMPT_RT. This patch handles one of those cases, and we'll hopefully
> have additional patches for other cases.
> 
> [    2.500000] [ BUG: Invalid wait context ]
[...]
> 
> Reported-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
> Link: https://lore.kernel.org/lkml/YfMa0QgsjCVdRAvJ@latitude/
> Fixes: b7d5dc21072c ("random: add a spinlock_t to struct batched_entropy")
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
> Changes v1->v2:
> - We move from Andy's original patch, which was a bit racey, to using a
>   simple generation counter.
> 
>  drivers/char/random.c | 58 ++++++++++++++++++++-----------------------
>  1 file changed, 27 insertions(+), 31 deletions(-)

This patch doesn't seem to apply properly on 5.17-rc1:

$ git am rand2.mbox
Applying: random: remove batched entropy locking
error: patch failed: drivers/char/random.c:2057
error: drivers/char/random.c: patch does not apply
Patch failed at 0001 random: remove batched entropy locking
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

So I cherry-picked it from https://git.zx2c4.com/linux-rng jd/no-batch-lock.

The result looks alright (no splat during boot).

Tested-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
--


With both patches, I get the following about three minutes after boot, but
I guess it will be addressed by one of the other patchsets under discussion:

[  202.670000] =============================
[  202.670000] [ BUG: Invalid wait context ]
[  202.670000] 5.17.0-rc1-00001-g4e53c88bd88e #585 Not tainted
[  202.670000] -----------------------------
[  202.670000] swapper/0 is trying to lock:
[  202.670000] c0b0ff3c (random_write_wait.lock){....}-{3:3}, at: __wake_up_common_lock+0x54/0xb4
[  202.670000] other info that might help us debug this:
[  202.670000] context-{2:2}
[  202.670000] no locks held by swapper/0.
[  202.670000] stack backtrace:
[  202.670000] CPU: 0 PID: 0 Comm: swapper Not tainted 5.17.0-rc1-00001-g4e53c88bd88e #585
[  202.670000] Hardware name: WPCM450 chip
[  202.670000] [<c00100a8>] (unwind_backtrace) from [<c000db2c>] (show_stack+0x10/0x14)
[  202.670000] [<c000db2c>] (show_stack) from [<c0054eec>] (__lock_acquire+0x3f0/0x189c)
[  202.670000] [<c0054eec>] (__lock_acquire) from [<c0054478>] (lock_acquire+0x2b8/0x354)
[  202.670000] [<c0054478>] (lock_acquire) from [<c0568088>] (_raw_spin_lock_irqsave+0x60/0x74)
[  202.670000] [<c0568088>] (_raw_spin_lock_irqsave) from [<c004c34c>] (__wake_up_common_lock+0x54/0xb4)
[  202.670000] [<c004c34c>] (__wake_up_common_lock) from [<c004c3bc>] (__wake_up+0x10/0x18)
[  202.670000] [<c004c3bc>] (__wake_up) from [<c030d898>] (crng_reseed.constprop.0+0x240/0x338)
[  202.670000] [<c030d898>] (crng_reseed.constprop.0) from [<c0061b34>] (handle_irq_event_percpu+0x18/0x38)
[  202.670000] [<c0061b34>] (handle_irq_event_percpu) from [<c0061b8c>] (handle_irq_event+0x38/0x5c)
[  202.670000] [<c0061b8c>] (handle_irq_event) from [<c0065b28>] (handle_fasteoi_irq+0x9c/0x114)
[  202.670000] [<c0065b28>] (handle_fasteoi_irq) from [<c0061178>] (handle_irq_desc+0x24/0x34)
[  202.670000] [<c0061178>] (handle_irq_desc) from [<c05621ac>] (generic_handle_arch_irq+0x28/0x3c)
[  202.670000] [<c05621ac>] (generic_handle_arch_irq) from [<c0008eb4>] (__irq_svc+0x54/0x80)
[  202.670000] Exception stack(0xc0931f20 to 0xc0931f68)
[  202.670000] 1f20: 00000000 0005317f 0005217f 60000013 00000000 00000000 c0930000 ffffffff
[  202.670000] 1f40: c0937ac0 00000000 00053177 00000000 600000d3 c0931f70 c000b0e8 c000b0e0
[  202.670000] 1f60: 60000013 ffffffff
[  202.670000] [<c0008eb4>] (__irq_svc) from [<c000b0e0>] (arch_cpu_idle+0x24/0x34)
[  202.670000] [<c000b0e0>] (arch_cpu_idle) from [<c0567e24>] (default_idle_call+0x40/0x80)
[  202.670000] [<c0567e24>] (default_idle_call) from [<c0046fbc>] (do_idle+0xd4/0x254)
[  202.670000] [<c0046fbc>] (do_idle) from [<c00474e4>] (cpu_startup_entry+0xc/0x10)
[  202.670000] [<c00474e4>] (cpu_startup_entry) from [<c07bdedc>] (start_kernel+0x5cc/0x6c0)
[  202.670000] random: crng init done

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2022-01-29 21:04 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-27 22:21 "BUG: Invalid wait context" in invalidate_batched_entropy Jonathan Neuschäfer
2022-01-27 22:26 ` Jason A. Donenfeld
2022-01-28  8:34   ` Sebastian Andrzej Siewior
2022-01-28 16:04     ` Jason A. Donenfeld
2022-01-28 16:19       ` Sebastian Andrzej Siewior
2022-01-28 16:28         ` Jason A. Donenfeld
2022-01-28 17:02           ` Sebastian Andrzej Siewior
2022-01-28 15:33   ` [PATCH] random: remove batched entropy locking Jason A. Donenfeld
2022-01-28 15:44     ` Sebastian Andrzej Siewior
2022-01-28 15:54       ` Jason A. Donenfeld
2022-01-28 16:15         ` Sebastian Andrzej Siewior
2022-01-28 16:36           ` Jason A. Donenfeld
2022-01-28 15:48     ` Jason A. Donenfeld
2022-01-28 22:35       ` [PATCH v2] " Jason A. Donenfeld
2022-01-29 21:03         ` Jonathan Neuschäfer [this message]
2022-02-04  0:27         ` Jason A. Donenfeld
2022-02-04 11:10           ` Sebastian Andrzej Siewior
2022-02-04 13:42             ` Jason A. Donenfeld
2022-02-04 14:01               ` Sebastian Andrzej Siewior
2022-02-04 14:11                 ` Jason A. Donenfeld
2022-02-04 14:30                   ` Sebastian Andrzej Siewior
2022-02-04 15:39                     ` Jason A. Donenfeld
2022-02-04 15:51                       ` [PATCH v3] " Jason A. Donenfeld
2022-02-04 15:57                         ` Sebastian Andrzej Siewior
2022-02-04 16:12                           ` Jason A. Donenfeld
2022-02-16 20:01                           ` Jann Horn
2022-02-16 20:58                             ` Jason A. Donenfeld
2022-02-17 17:33                               ` Sebastian Andrzej Siewior
2022-01-28 18:05     ` [PATCH] " Jonathan Neuschäfer
2022-01-29 18:22       ` Jason A. Donenfeld
2022-01-29  7:10     ` [random] 1e1724f9dd: UBSAN:array-index-out-of-bounds_in_drivers/char/random.c kernel test robot
2022-01-29  7:10       ` kernel test robot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YfWrpoQIR9+NDGHl@latitude \
    --to=j.neuschaefer@gmx.net \
    --cc=Jason@zx2c4.com \
    --cc=bigeasy@linutronix.de \
    --cc=boqun.feng@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=longman@redhat.com \
    --cc=luto@amacapital.net \
    --cc=luto@kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=sultan@kerneltoast.com \
    --cc=tytso@mit.edu \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.