Re: [PATCH printk v2 12/18] printk: Add kthread for all legacy consoles

[Petr Mladek <pmladek@suse.com>]

On Fri 2024-06-28 14:28:47, John Ogness wrote:
> On 2024-06-28, Petr Mladek <pmladek@suse.com> wrote:
> >> @@ -1494,7 +1508,9 @@ void nbcon_cpu_emergency_exit(void)
> >>  		 * to handle it.
> >>  		 */
> >>  		do_trigger_flush = true;
> >> -		if (printing_via_unlock && !is_printk_deferred()) {
> >> +		if (!force_printkthreads() &&
> >
> > Is this correct? We still need to flush the messages directly
> > when the legacy kthread is not ready yet.
> 
> No! If force_printkthreads() is set, messages will never flush directly
> except for console_flush_on_panic().

But this is an _emergency_ context! This would be inconsistent with
the nbcon consoles where the messages are flushed directly.

Is RT sheduling quaranteed when the system reached emergency state?

In fact, we probably should not check force_printkthreads() here at
all. It would be only for NBCON_PRIO_NORMAL context.

Or the option should force the kthreads even for nbcon consoles.


> I see that I implemented get_init_console_seq() to flush since it is
> known to be in task state. But that is wrong. Also there it should not
> flush directly. It is not about whether or not printing is safe. It is
> about deferring to the thread 100% of the time except for panic.
> 
> >> +void nbcon_legacy_kthread_create(void)
> >> +{
> >> +	struct task_struct *kt;
> >> +
> >> +	lockdep_assert_held(&console_mutex);
> >> +
> >> +	if (!force_printkthreads())
> >> +		return;
> >> +
> >> +	if (!printk_threads_enabled || nbcon_legacy_kthread)
> >> +		return;
> >> +
> >> +	kt = kthread_run(nbcon_legacy_kthread_func, NULL, "pr/legacy");
> >> +	if (IS_ERR(kt)) {
> >> +		pr_err("unable to start legacy printing thread\n");
> >> +		return;
> >
> > Is this acceptable for RT, please?
> 
> It is not acceptable for mainline. For the next version, any failed
> thread creation leads to unregistering the console. In the case of the
> legacy thread, it will mean unregistering all legacy consoles on
> failure.

It means that the system would become silent. Is this a good idea?

IMHO, we have a fundamental problem here.
Are RT guarantees more important than printk() or vice versa?

What is happening when the RT guarantees are violated?
For example, when the scheduler detects a problem?
Does it panic() or tries to continue?

From my POV, silent system is the worst solution. The user
might think: "no messages, no problem".

Best Regards,
Petr