Commit 8c9dab2c authored by John Ogness's avatar John Ogness Committed by Petr Mladek

printk: nbcon: Clarify rules of the owner/waiter matching

The functions nbcon_owner_matches() and nbcon_waiter_matches()
use a minimal set of data to determine if a context matches.
The existing kerneldoc and comments were not clear enough and
caused the printk folks to re-prove that the functions are
indeed reliable in all cases.

Update and expand the explanations so that it is clear that the
implementations are sufficient for all cases.
Signed-off-by: default avatarJohn Ogness <john.ogness@linutronix.de>
Reviewed-by: default avatarPetr Mladek <pmladek@suse.com>
Link: https://lore.kernel.org/r/20240820063001.36405-6-john.ogness@linutronix.deSigned-off-by: default avatarPetr Mladek <pmladek@suse.com>
parent 0e1d5731
...@@ -228,6 +228,13 @@ static int nbcon_context_try_acquire_direct(struct nbcon_context *ctxt, ...@@ -228,6 +228,13 @@ static int nbcon_context_try_acquire_direct(struct nbcon_context *ctxt,
struct nbcon_state new; struct nbcon_state new;
do { do {
/*
* Panic does not imply that the console is owned. However, it
* is critical that non-panic CPUs during panic are unable to
* acquire ownership in order to satisfy the assumptions of
* nbcon_waiter_matches(). In particular, the assumption that
* lower priorities are ignored during panic.
*/
if (other_cpu_in_panic()) if (other_cpu_in_panic())
return -EPERM; return -EPERM;
...@@ -259,18 +266,29 @@ static bool nbcon_waiter_matches(struct nbcon_state *cur, int expected_prio) ...@@ -259,18 +266,29 @@ static bool nbcon_waiter_matches(struct nbcon_state *cur, int expected_prio)
/* /*
* The request context is well defined by the @req_prio because: * The request context is well defined by the @req_prio because:
* *
* - Only a context with a higher priority can take over the request. * - Only a context with a priority higher than the owner can become
* a waiter.
* - Only a context with a priority higher than the waiter can
* directly take over the request.
* - There are only three priorities. * - There are only three priorities.
* - Only one CPU is allowed to request PANIC priority. * - Only one CPU is allowed to request PANIC priority.
* - Lower priorities are ignored during panic() until reboot. * - Lower priorities are ignored during panic() until reboot.
* *
* As a result, the following scenario is *not* possible: * As a result, the following scenario is *not* possible:
* *
* 1. Another context with a higher priority directly takes ownership. * 1. This context is currently a waiter.
* 2. The higher priority context releases the ownership. * 2. Another context with a higher priority than this context
* 3. A lower priority context takes the ownership. * directly takes ownership.
* 4. Another context with the same priority as this context * 3. The higher priority context releases the ownership.
* 4. Another lower priority context takes the ownership.
* 5. Another context with the same priority as this context
* creates a request and starts waiting. * creates a request and starts waiting.
*
* Event #1 implies this context is EMERGENCY.
* Event #2 implies the new context is PANIC.
* Event #3 occurs when panic() has flushed the console.
* Events #4 and #5 are not possible due to the other_cpu_in_panic()
* check in nbcon_context_try_acquire_direct().
*/ */
return (cur->req_prio == expected_prio); return (cur->req_prio == expected_prio);
...@@ -578,11 +596,29 @@ static bool nbcon_owner_matches(struct nbcon_state *cur, int expected_cpu, ...@@ -578,11 +596,29 @@ static bool nbcon_owner_matches(struct nbcon_state *cur, int expected_cpu,
int expected_prio) int expected_prio)
{ {
/* /*
* Since consoles can only be acquired by higher priorities, * A similar function, nbcon_waiter_matches(), only deals with
* owning contexts are uniquely identified by @prio. However, * EMERGENCY and PANIC priorities. However, this function must also
* since contexts can unexpectedly lose ownership, it is * deal with the NORMAL priority, which requires additional checks
* possible that later another owner appears with the same * and constraints.
* priority. For this reason @cpu is also needed. *
* For the case where preemption and interrupts are disabled, it is
* enough to also verify that the owning CPU has not changed.
*
* For the case where preemption or interrupts are enabled, an
* external synchronization method *must* be used. In particular,
* the driver-specific locking mechanism used in device_lock()
* (including disabling migration) should be used. It prevents
* scenarios such as:
*
* 1. [Task A] owns a context with NBCON_PRIO_NORMAL on [CPU X] and
* is scheduled out.
* 2. Another context takes over the lock with NBCON_PRIO_EMERGENCY
* and releases it.
* 3. [Task B] acquires a context with NBCON_PRIO_NORMAL on [CPU X]
* and is scheduled out.
* 4. [Task A] gets running on [CPU X] and sees that the console is
* still owned by a task on [CPU X] with NBON_PRIO_NORMAL. Thus
* [Task A] thinks it is the owner when it is not.
*/ */
if (cur->prio != expected_prio) if (cur->prio != expected_prio)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment