All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
To: "Jason A. Donenfeld" <Jason@zx2c4.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	"Peter Zijlstra" <peterz@infradead.org>,
	"Theodore Ts'o" <tytso@mit.edu>,
	"Sultan Alsawaf" <sultan@kerneltoast.com>,
	"Jonathan Neuschäfer" <j.neuschaefer@gmx.net>
Subject: Re: [PATCH RFC v1] random: do not take spinlocks in irq handler
Date: Fri, 4 Feb 2022 21:53:10 +0100	[thread overview]
Message-ID: <Yf2SNpi5847c7ceK@linutronix.de> (raw)
In-Reply-To: <CAHmME9pucLWXDofvOgHEau3y-7RmdtU91_jQHSt7psuR22eXBg@mail.gmail.com>

On 2022-02-04 16:58:58 [+0100], Jason A. Donenfeld wrote:
> FWIW, the biggest issue with this
> 
> On Fri, Feb 4, 2022 at 4:32 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> > +static void mix_interrupt_randomness(struct work_struct *work)
> > +{
> [...]
> > +       if (unlikely(crng_init == 0)) {
> > +               if (crng_fast_load((u8 *)&fast_pool->pool, sizeof(fast_pool->pool)) > 0)
> > +                       atomic_set(&fast_pool->count, 0);
> > +               else
> > +                       atomic_and(~FAST_POOL_MIX_INFLIGHT, &fast_pool->count);
> > +               return;
> > +       }
> [...]
> >  void add_interrupt_randomness(int irq)
> > -       if (unlikely(crng_init == 0)) {
> > -               if ((fast_pool->count >= 64) &&
> > -                   crng_fast_load((u8 *)fast_pool->pool, sizeof(fast_pool->pool)) > 0) {
> > -                       fast_pool->count = 0;
> > -                       fast_pool->last = now;
> > -               }
> > -               return;
> 
> The point of crng_fast_load is to shuffle bytes into the crng as fast
> as possible for very early boot usage. Deferring that to a workqueue
> seems problematic. So I think at the very least _that_ part will have
> to stay in the IRQ handler. That means we've still got a spinlock. But
> at least it's a less problematic one than the input pool spinlock, and
> perhaps we can deal with that some other way than this patch's
> approach.

RT wise we _could_ acquire that spinlock_t in IRQ context early during
boot as long as system_state < SYSTEM_SCHEDULING. After that, we could
dead lock.

> In other words, this approach for the calls to mix_pool_bytes, and a
> different approach for that call to crng_fast_load.
> 
> Jason

Sebastian

  reply	other threads:[~2022-02-04 20:53 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-07 12:17 [PATCH 0/5] random: Add PREEMPT_RT support Sebastian Andrzej Siewior
2021-12-07 12:17 ` [PATCH 1/5] random: Remove unused irq_flags argument from add_interrupt_randomness() Sebastian Andrzej Siewior
2021-12-07 12:44   ` Wei Liu
2021-12-07 17:35   ` Jason A. Donenfeld
2021-12-07 12:17 ` [PATCH 2/5] irq: Remove unsued flags argument from __handle_irq_event_percpu() Sebastian Andrzej Siewior
2021-12-07 17:41   ` Jason A. Donenfeld
2021-12-11 22:39     ` Thomas Gleixner
2021-12-12 21:13       ` Jason A. Donenfeld
2021-12-07 12:17 ` [PATCH 3/5] random: Split add_interrupt_randomness() Sebastian Andrzej Siewior
2021-12-07 12:17 ` [PATCH 4/5] random: Move the fast_pool reset into the caller Sebastian Andrzej Siewior
2021-12-07 12:17 ` [PATCH 5/5] random: Defer processing of randomness on PREEMPT_RT Sebastian Andrzej Siewior
2021-12-07 18:14   ` Jason A. Donenfeld
2021-12-07 20:10     ` Sebastian Andrzej Siewior
2021-12-13 15:16       ` Sebastian Andrzej Siewior
2021-12-17  2:23       ` Herbert Xu
2021-12-17  9:00         ` Sebastian Andrzej Siewior
2021-12-18  3:24           ` Herbert Xu
2021-12-19  9:48             ` Sebastian Andrzej Siewior
2021-12-20  2:56               ` Herbert Xu
2021-12-20 14:38       ` Jason A. Donenfeld
2022-01-12 17:49         ` Sebastian Andrzej Siewior
2022-01-30 22:55           ` Jason A. Donenfeld
2022-01-31 16:33             ` Sebastian Andrzej Siewior
2022-02-04 15:31               ` [PATCH RFC v1] random: do not take spinlocks in irq handler Jason A. Donenfeld
2022-02-04 15:58                 ` Jason A. Donenfeld
2022-02-04 20:53                   ` Sebastian Andrzej Siewior [this message]
2022-02-04 20:47                 ` Sebastian Andrzej Siewior
2022-02-04 22:08                   ` Jason A. Donenfeld
2022-02-04 23:09                     ` [PATCH v2] random: defer fast pool mixing to worker Jason A. Donenfeld
2022-02-05  4:02                   ` [PATCH RFC v1] random: do not take spinlocks in irq handler Sultan Alsawaf
2022-02-05 12:50                     ` Jason A. Donenfeld

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=Yf2SNpi5847c7ceK@linutronix.de \
    --to=bigeasy@linutronix.de \
    --cc=Jason@zx2c4.com \
    --cc=j.neuschaefer@gmx.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=sultan@kerneltoast.com \
    --cc=tglx@linutronix.de \
    --cc=tytso@mit.edu \
    /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.