Re: [PATCH v4 2/2] lib/vsprintf: Initialize vsprintf's pointer hash once the random core is ready.

From: Sebastian Andrzej Siewior
Date: Wed Sep 28 2022 - 05:13:13 EST


On 2022-09-27 18:40:15 [+0200], Petr Mladek wrote:
> Another advantage is that it removes a nested lock from the printk()
> code path. A deadlock was partly prevented by the trylock. But there was
> still a risk of a deadlock when printk() was called under base_crng.lock.

Okay.

> > diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> > index bce63cbf23779..44b39ba56b796 100644
> > --- a/lib/vsprintf.c
> > +++ b/lib/vsprintf.c

> I have tried it and system_wq works well here. It actually
> even initializes the hash earlier here. But it is only by chance
> because it happens on the 2nd attempt instead of 3rd one.

Yeah. I added a reschedule of two seconds since it looked okay and I
didn't want to do very often. I have an old box where it takes ~12 secs
to setup and here it is the fifth attempt on average. (Before the rework
it needed way longer to initialize).

> > + return;
> > + }
> > +
> > + get_random_bytes(&ptr_key, sizeof(ptr_key));
> > +
> > + /* Pairs with smp_rmb() before reading ptr_key. */
> > + smp_wmb();
> > + WRITE_ONCE(filled_random_ptr_key, true);
> > +}
>
> With "system_wq":
>
> Reviewed-by: Petr Mladek <pmladek@xxxxxxxx>
>
> I could replace "system_unbound_wq" with "system_wq" when
> pushing. Is anybody against it, please?

so schedule_delayed_work() then?
I don't mind at all. I used that one just because serialisation is not
needed and neither is the CPU important.

If you are going to replace it, then I am not going to send an update
(unless I'm old otherwise).

> I am sorry that I have missed it when looking at the previous
> version.

No worries.

> Best Regards,
> Petr

Sebastian