2024-02-18 18:59:46

by John Ogness

[permalink] [raw]
Subject: [PATCH printk v2 17/26] printk: nbcon: Assign priority based on CPU state

Use the current state of the CPU to determine which priority to
assign to the printing context.

Note: The uart_port wrapper, which is responsible for non-console-
printing activities, will always use NORMAL priority.

Signed-off-by: John Ogness <[email protected]>
---
kernel/printk/internal.h | 2 ++
kernel/printk/nbcon.c | 30 ++++++++++++++++++++++++++++--
2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/kernel/printk/internal.h b/kernel/printk/internal.h
index b34847ec6b0d..70e9a1ea75be 100644
--- a/kernel/printk/internal.h
+++ b/kernel/printk/internal.h
@@ -79,6 +79,7 @@ void nbcon_seq_force(struct console *con, u64 seq);
bool nbcon_alloc(struct console *con);
void nbcon_init(struct console *con);
void nbcon_free(struct console *con);
+enum nbcon_prio nbcon_get_default_prio(void);
void nbcon_atomic_flush_all(void);
bool nbcon_legacy_emit_next_record(struct console *con, bool *handover,
int cookie);
@@ -136,6 +137,7 @@ static inline void nbcon_seq_force(struct console *con, u64 seq) { }
static inline bool nbcon_alloc(struct console *con) { return false; }
static inline void nbcon_init(struct console *con) { }
static inline void nbcon_free(struct console *con) { }
+static inline enum nbcon_prio nbcon_get_default_prio(void) { return NBCON_PRIO_NONE; }
static inline void nbcon_atomic_flush_all(void) { }
static inline bool nbcon_legacy_emit_next_record(struct console *con, bool *handover,
int cookie) { return false; }
diff --git a/kernel/printk/nbcon.c b/kernel/printk/nbcon.c
index 747f5cbfe5ee..1a18549e43b8 100644
--- a/kernel/printk/nbcon.c
+++ b/kernel/printk/nbcon.c
@@ -961,6 +961,22 @@ static bool nbcon_atomic_emit_one(struct nbcon_write_context *wctxt)
return ctxt->backlog;
}

+/**
+ * nbcon_get_default_prio - The appropriate nbcon priority to use for nbcon
+ * printing on the current CPU
+ *
+ * Context: Any context which could not be migrated to another CPU.
+ * Return: The nbcon_prio to use for acquiring an nbcon console in this
+ * context for printing.
+ */
+enum nbcon_prio nbcon_get_default_prio(void)
+{
+ if (this_cpu_in_panic())
+ return NBCON_PRIO_PANIC;
+
+ return NBCON_PRIO_NORMAL;
+}
+
/**
* nbcon_legacy_emit_next_record - Print one record for an nbcon console
* in legacy contexts
@@ -994,7 +1010,7 @@ bool nbcon_legacy_emit_next_record(struct console *con, bool *handover,
stop_critical_timings();

ctxt->console = con;
- ctxt->prio = NBCON_PRIO_NORMAL;
+ ctxt->prio = nbcon_get_default_prio();

progress = nbcon_atomic_emit_one(&wctxt);

@@ -1038,7 +1054,6 @@ static void __nbcon_atomic_flush_all(u64 stop_seq)
memset(ctxt, 0, sizeof(*ctxt));
ctxt->console = con;
ctxt->spinwait_max_us = 2000;
- ctxt->prio = NBCON_PRIO_NORMAL;

/*
* Atomic flushing does not use console driver
@@ -1047,9 +1062,14 @@ static void __nbcon_atomic_flush_all(u64 stop_seq)
* disabled to avoid being interrupted and then
* calling into a driver that will deadlock trying
* acquire console ownership.
+ *
+ * This also disables migration in order to get the
+ * current CPU priority.
*/
local_irq_save(irq_flags);

+ ctxt->prio = nbcon_get_default_prio();
+
any_progress |= nbcon_atomic_emit_one(&wctxt);

local_irq_restore(irq_flags);
@@ -1161,6 +1181,9 @@ static inline bool uart_is_nbcon(struct uart_port *up)
*
* If @up is an nbcon console, this console will be acquired and marked as
* unsafe. Otherwise this function does nothing.
+ *
+ * nbcon consoles acquired via the port lock wrapper always use priority
+ * NBCON_PRIO_NORMAL.
*/
void uart_nbcon_acquire(struct uart_port *up)
{
@@ -1195,6 +1218,9 @@ EXPORT_SYMBOL_GPL(uart_nbcon_acquire);
*
* If @up is an nbcon console, the console will be marked as safe and
* released. Otherwise this function does nothing.
+ *
+ * nbcon consoles acquired via the port lock wrapper always use priority
+ * NBCON_PRIO_NORMAL.
*/
void uart_nbcon_release(struct uart_port *up)
{
--
2.39.2



2024-02-29 13:50:19

by Petr Mladek

[permalink] [raw]
Subject: Re: [PATCH printk v2 17/26] printk: nbcon: Assign priority based on CPU state

On Sun 2024-02-18 20:03:17, John Ogness wrote:
> Use the current state of the CPU to determine which priority to
> assign to the printing context.
>
> Note: The uart_port wrapper, which is responsible for non-console-
> printing activities, will always use NORMAL priority.
>
> --- a/kernel/printk/nbcon.c
> +++ b/kernel/printk/nbcon.c
> @@ -961,6 +961,22 @@ static bool nbcon_atomic_emit_one(struct nbcon_write_context *wctxt)
> return ctxt->backlog;
> }
>
> +/**
> + * nbcon_get_default_prio - The appropriate nbcon priority to use for nbcon
> + * printing on the current CPU
> + *
> + * Context: Any context which could not be migrated to another CPU.

Strictly speaking, this variant does not need to be called with
disabled CPU migration. The panic-CPU could not be migrated.

It would be nice to mention in the commit message, something like:

</proposal>
The emergency context handling is going to be added in a separate
patch. It will use a per-CPU variable.
</proposal>

It would also explain why this context is not handled in this patch.

> + * Return: The nbcon_prio to use for acquiring an nbcon console in this
> + * context for printing.
> + */
> +enum nbcon_prio nbcon_get_default_prio(void)
> +{
> + if (this_cpu_in_panic())
> + return NBCON_PRIO_PANIC;
> +
> + return NBCON_PRIO_NORMAL;
> +}
> +
> /**
> * nbcon_legacy_emit_next_record - Print one record for an nbcon console
> * in legacy contexts

Otherwise, it looks good. With the added commit message:

Reviewed-by: Petr Mladek <[email protected]>

Best Regards,
Petr