On Wed 2022-11-16 17:16:38, Jason A. Donenfeld wrote:
> Rather than polling every second, use the new notifier to do this at
> exactly the right moment.
Great news!
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -752,26 +753,21 @@ early_param("debug_boot_weak_hash", debug_boot_weak_hash_enable);
>
> static bool filled_random_ptr_key __read_mostly;
> static siphash_key_t ptr_key __read_mostly;
> -static void fill_ptr_key_workfn(struct work_struct *work);
> -static DECLARE_DELAYED_WORK(fill_ptr_key_work, fill_ptr_key_workfn);
>
> -static void fill_ptr_key_workfn(struct work_struct *work)
> +static int fill_ptr_key(struct notifier_block *nb, unsigned long action, void *data)
> {
> - if (!rng_is_initialized()) {
> - queue_delayed_work(system_unbound_wq, &fill_ptr_key_work, HZ * 2);
> - 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);
> + return 0;
I believe that we should rather return NOTIFY_DONE here.
It is rather a formal change. The value is 0 as well.
That said, I have never really understood the difference between
NOTIFY_OK and NOTIFY_DONE.
> }
>
> static int __init vsprintf_init_hashval(void)
> {
> - fill_ptr_key_workfn(NULL);
> + static struct notifier_block fill_ptr_key_nb = { .notifier_call = fill_ptr_key };
> + notify_on_rng_initialized(&fill_ptr_key_nb);
> return 0;
> }
> subsys_initcall(vsprintf_init_hashval)
Anyway, the code looks good to me:
Reviewed-by: Petr Mladek <pmladek@xxxxxxxx>
Best Regards,
Petr
