Messages in this thread | | | From | John Ogness <> | Subject | Re: [PATCH printk v1 11/13] printk: reimplement console_lock for proper kthread support | Date | Fri, 11 Mar 2022 14:34:40 +0106 |
| |
On 2022-03-11, Petr Mladek <pmladek@suse.com> wrote: >> console_unlock() >> { >> [...] >> if (may_schedule) >> retry = console_lock_reacquire(); >> else >> retry = console_trylock(); >> } >>
[...]
> OK, it means that the main problem here _is not_ the scheduling context, > console_lock() vs. console_trylock(). The main problem _is_ the direct > printing vs. the offload to kthreads. > > Of course, the context is important. It affects how we could re-take > the lock. But the main problem is the printing mode. We must make sure > that: > > 1. someone is printing pending messages when the direct mode is needed
console_trylock() causes difficulties here because it will fail if any kthread is active. It is an example of direct mode failure. But is that really any different than current mainline console_trylock() failing because a console_lock() context is active (and possibly not scheduled on a CPU)?
> 2. kthreads are woken and can enter the printing mode when the direct > mode is disabled.
This happens at the end of vprintk_emit() and within __console_unlock(), regardless if the printk() was running in direct mode or not.
> Will console_lock_reacquire() really help here? > > The API theoretically helps in direct mode when the lock was taken > via console_lock().
console_lock_reacquire() only exists for the console_lock() case.
> But it does not help when the lock was taken > via console_trylock() from printk(). It might mean that > the forward progress might not be guaranteed in the direct mode > (early boot, panic, ...).
How is the console_trylock() case different from current mainline now? As I mentioned above, the kthreads can block console_trylock(), but so can a console_lock() currently in mainline.
> Hmm, the forward progress seems to be guaranteed in the direct > mode most of the time. console_trylock() can take over > the atomic counter because console kthreads are not allowed > to enter the printing mode in this case. > > I used "most of the time" because there might be races when > the mode is switched. "printk_direct" is an atomic variable. > CON_DIRECT is set under con->mutex but console_trylock() > does not take the mutex...
You are mixing issues here. If CON_DIRECT is set, there is already a console_lock() in progress, so console_trylock() fails on @console_sem.
> There are also races when the offload to consoles kthreads > is allowed. For example, console_trylock() might block > console_kthread_printing_tryenter().
I do not see how that is a problem. If any context has the console lock (no matter how it got that lock) then the kthreads are blocked.
If direct printing is enabled (from @printk_direct or @oops_in_progress or @system_state != SYSTEM_RUNNING), the task with the console lock will print out *all* the remaining records.
If direct printing is not enabled, the kthreads are woken in __console_unlock().
> Sigh, I am afraid that we have non-trivial problems > to guarantee that all messages will be printed: > > + races when switching between direct mode > and offload to kthreads. It might cause > stall in both modes.
Scheduable contexts holding locks can cause stalls. We have that same problem with console_lock() in mainline now. The kernel provides mechanisms to avoid such stalls (niceness, priorities, policies, priority inheritance), but this is all problem-specific and must be fine-tuned by the user if they are running workloads that are causing problems. kthreads are not solving the reliability problem (and they never will).
> + console_trylock() races with > console_kthread_printing_tryenter(). > It might put kthread into a sleep even when > it is supposed to print the message.
kthread is never _supposed_ to print a message. It is there to offload direct printing. If console_trylock() (direct printing) wins, then that is the context that does the printing.
> IMHO, console_lock_reacquire() does not help much here. > We need to solve console_trylock() path anyway.
It preserves a consistent locking scenario for the console_lock() path. That is all it is intended to do.
> I think that the solution might be: > > + make sure that the state of "printk_direct" atomic variable > is enough to distinguish about the mode.
The printk subsystem does not have absolute control over which task/context is doing the actual printing. Currently in mainline there are printers that handoff to waiters and even some that do not print at all because there is already waiters. Adding kthreads introduces a new task that can print. But there still is no real control about who prints. The important thing is that there is some context or runnable task that is running the printing code (whether scheduled or not).
> + always wakeup() console kthreads after console_trylock() > to handle the possible race with > console_kthread_printing_tryenter()
I do not understand. If console_trylock() wins, it already wakes the kthreads on __console_unlock(). If console_trylock() loses, the kthreads are already running.
John
| |