Re: [PATCH printk v2] printk: Postpone kthread startup

[Derek Barbosa <debarbos@redhat.com>]

On Fri, Oct 16, 2026 at 04:43:47PM +0200, Petr Mladek wrote:
> On Fri 2026-10-16 15:00:54, John Ogness wrote:
> > On 2026-10-16, sashiko.dev wrote:
> > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> > index 0ed86b5e8b160..0a87f49b3908a 100644
> > --- a/kernel/printk/printk.c
> > +++ b/kernel/printk/printk.c
> > @@ -4322,9 +4322,6 @@ static int unregister_console_locked(struct console *console)
> >  	if (console->exit)
> >  		res = console->exit(console);
> >  
> > -	/* Changed console list, may require printer threads to start/stop. */
> > -	printk_kthreads_check_locked();
> > -
> >  	return res;
> >  }
> >  
> > @@ -4333,7 +4330,12 @@ int unregister_console(struct console *console)
> >  	int res;
> >  
> >  	console_list_lock();
> > +
> >  	res = unregister_console_locked(console);
> > +
> > +	/* Changed console list, may require printer threads to start/stop. */
> 
> It might make sense to warn about the potential recursion. Otherwise,
> someone might try to optimize it back. And the problem is far from
> obvious. We should not rely on Sashiko...
> 
> Something like:
> 
> 	/*
> 	 * Changed console list, may require printer threads to	start/stop.
> 	 * Do this outside of unregister_console_locked() to avoid regression.
> 	 */

Sorry for chiming in here. It might be an obvious (and therefore useless)
comment, but could we expand this warning to say:

    /*
     * Changed console list, may require printer threads to start/stop.
     * Perform this check outside of unregister_console_locked() call sites
     * to avoid use-after-free scenarios.
     /

to elaborate on what "regression" could mean?

> Or we could add a comment above printk_kthreads_check_locked(). But it
> would be easier to overlook.
> 

I could be wrong, but iss there any harm in doing both (outside of being
redundant)? Explaining the "design pattern" above the function could be helpful

    /*
     * If appropriate, start nbcon kthreads and set @printk_kthreads_running.
     * If any kthreads fail to start, unregister those consoles.
     *
     * Refrain from calling this function within routines that may iterate over
     * the active console list to perform any (un)registration.
     *
     * Must be called under console_list_lock().
     */

IIUC it just makes it a little more clear that the old behavior, where the inner
loop was mutating the outer loop's iterator, should be avoided.


> > +	printk_kthreads_check_locked();
> > +
> >  	console_list_unlock();
> >  	return res;
> >  }
> > @@ -4471,6 +4473,9 @@ static int __init printk_late_init(void)
> >  		}
> >  
> >  		unregister_console_locked(con);
> > +
> > +		/* Changed console list, may require printer threads to start/stop. */
> 
> Same here.
> 
> > +		printk_kthreads_check_locked();
> >  	}
> >  	console_list_unlock();
> > 
> > I can submit an official patch after the 7.4 merge window.
> 
> That would be great. Thanks in advance.
> 
> Best Regards,
> Petr
> 

-- 
Derek <debarbos@redhat.com>