Received-SPF: pass (google.com: domain of linux-kernel+bounces-86779-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Thu, 29 Feb 2024 14:50:01 +0100
From: Petr Mladek <pmladek@suse.com>
To: John Ogness <john.ogness@linutronix.de>
Cc: Sergey Senozhatsky <senozhatsky@chromium.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Thomas Gleixner <tglx@linutronix.de>, linux-kernel@vger.kernel.org
Subject: Re: [PATCH printk v2 17/26] printk: nbcon: Assign priority based on
 CPU state
Message-ID: <ZeCLic3zZEdi0-o2@alley>
References: <20240218185726.1994771-1-john.ogness@linutronix.de>
 <20240218185726.1994771-18-john.ogness@linutronix.de>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240218185726.1994771-18-john.ogness@linutronix.de>

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@suse.com>

Best Regards,
Petr