Re: [PATCH v2 2/4] printk: deprecate boot_delay in favour of printk_delay

[Andrew Murray]

Hi Petr,

On Wed, 8 Jul 2026 at 16:01, Petr Mladek <pmladek@xxxxxxxx> wrote:
>
> On Tue 2026-06-30 17:35:58, Andrew Murray wrote:
> > The boot_delay (BOOT_PRINTK_DELAY) kernel parameter and printk_delay sysctl
> > are two distinct mechanisms for providing similar functionality which add a
> > delay prior to each printed printk message.
> >
> > boot_delay provides a kernel parameter for delaying printk output from
> > kernel start through to boot (SYSTEM_RUNNING), whereas printk_delay is
> > configurable only via sysctl and thus is only used post boot.
> >
> > Let's deprecate the boot_delay feature in favour of printk_delay. In order
> > to preserve functionality, we'll also extend printk_delay such that it can
> > additionally configured via an early kernel parameter.
> >
> > Behavior change:
> >
> > The delay enabled by both "boot_delay" and "printk_delay" continues
> > working even in SYSTEM_RUNNING state. It must be explicitly stopped
> > by setting printk_delay=0 via sysctl.
> >
> > The delay is skipped when the message is suppressed in all system
> > states. It used to skipped only for the boot_delay.
> >
> > Signed-off-by: Andrew Murray <amurray@xxxxxxxxxxxxxxxxxxxx>
>
> Looks good to me:
>
> Reviewed-by: Petr Mladek <pmladek@xxxxxxxx>
>
> Note that Sashiko AI warns about possible problems with negative
> printk_delay values, see
> https://sashiko.dev/#/patchset/20260630-deprecate_boot_delay-v2-0-f9883d36aa4b%40thegoodpenguin.co.uk
>
> But they should be handled in both the early parameter
> and sysctl interface by the "10 * 1000" and "&ten_thousand"
> limits.

Yes, that was my also view.


>
> The only potential problem might be a warning about possible
> "sign" mismatch from the compiler. But I do not see any
> even with make W=2.

I suspect this will come up everytime a change is made in this area,
or perhaps some W=2 errors will pop up via kernelci or similar. I
propose updating the patch as follows:

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 5278d9cb19e4..fbb67f10c21e 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -1338,10 +1338,13 @@ static inline void early_boot_delay_msec(void)

 static int __init printk_delay_setup(char *str)
 {
-       get_option(&str, &printk_delay_msec);
-       if (printk_delay_msec > 10 * 1000)
-               printk_delay_msec = 0;
+       int printk_delay_val = 0;

+       get_option(&str, &printk_delay_val);
+       if (printk_delay_val < 0 || printk_delay_val > 10 * 1000)
+               return 0;
+
+       printk_delay_msec = (unsigned int)printk_delay_val;
        printk_delay_calculate();

        return 0;
diff --git a/kernel/printk/sysctl.c b/kernel/printk/sysctl.c
index f15732e93c2e..f256fc05faaf 100644
--- a/kernel/printk/sysctl.c
+++ b/kernel/printk/sysctl.c
@@ -46,7 +46,7 @@ static const struct ctl_table printk_sysctls[] = {
                .data           = &printk_delay_msec,
                .maxlen         = sizeof(int),
                .mode           = 0644,
-               .proc_handler   = proc_dointvec_minmax,
+               .proc_handler   = proc_douintvec_minmax,
                .extra1         = SYSCTL_ZERO,
                .extra2         = (void *)&ten_thousand,
        },

This makes the cast explict, verifies the range before casting (I
guess a range of negative values could result in a postive value
within the 10,000 range) and also uses the correct proc_handler.

This has the side effect of not showing the following pr_debug for the
lpj calculation when a delay is not set. Though loops_per_msec is only
used during boot, so I don't think there is any loss here.

        pr_debug("printk_delay: %u, preset_lpj: %ld, lpj: %lu, "
                "HZ: %d, loops_per_msec: %llu\n",
                printk_delay_msec, preset_lpj, lpj, HZ, loops_per_msec);

Can I keep your Reviewed-By with these changes?

Thanks,

Andrew Murray

>
> Best Regards,
> Petr