linux-kernel - Re: [PATCH printk v2 17/26] printk: nbcon: Assign priority based on CPU state

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZeCLic3zZEdi0-o2@alley>
Date: Thu, 29 Feb 2024 14:50:01 +0100
From: Petr Mladek <pmladek@...e.com>
To: John Ogness <john.ogness@...utronix.de>
Cc: Sergey Senozhatsky <senozhatsky@...omium.org>,
	Steven Rostedt <rostedt@...dmis.org>,
	Thomas Gleixner <tglx@...utronix.de>, linux-kernel@...r.kernel.org
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 <pmladek@...e.com>

Best Regards,
Petr