Received-SPF: pass (google.com: domain of linux-kernel+bounces-135422-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Mon, 8 Apr 2024 15:17:46 +0200
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,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: Re: [PATCH printk v4 03/27] printk: nbcon: Remove return value for
 write_atomic()
Message-ID: <ZhPuep7cMc3i6wro@localhost.localdomain>
References: <20240402221129.2613843-1-john.ogness@linutronix.de>
 <20240402221129.2613843-4-john.ogness@linutronix.de>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240402221129.2613843-4-john.ogness@linutronix.de>

On Wed 2024-04-03 00:17:05, John Ogness wrote:
> The return value of write_atomic() does not provide any useful
> information. On the contrary, it makes things more complicated
> for the caller to appropriately deal with the information.
> 
> Change write_atomic() to not have a return value. If the
> message did not get printed due to loss of ownership, the
> caller will notice this on its own. If ownership was not lost,
> it will be assumed that the driver successfully printed the
> message and the sequence number for that console will be
> incremented.
> 
> Signed-off-by: John Ogness <john.ogness@linutronix.de>

Reviewed-by: Petr Mladek <pmladek@suse.com>

> --- a/kernel/printk/nbcon.c
> +++ b/kernel/printk/nbcon.c
> @@ -889,16 +888,16 @@ static bool nbcon_emit_next_record(struct nbcon_write_context *wctxt)
>  	wctxt->unsafe_takeover = cur.unsafe_takeover;
>  
>  	if (con->write_atomic) {
> -		done = con->write_atomic(con, wctxt);
> +		con->write_atomic(con, wctxt);
>  	} else {
> -		nbcon_context_release(ctxt);
> +		/*
> +		 * This function should never be called for legacy consoles.
> +		 * Handle it as if ownership was lost and try to continue.
> +		 */
>  		WARN_ON_ONCE(1);
> -		done = false;
> -	}
> -
> -	/* If not done, the emit was aborted. */
> -	if (!done)
> +		nbcon_context_release(ctxt);

I thought a bit whether it is better to release the context before
or after the WARN(). My conclusion is that it does not really matter.

Anyway, we must make sure that it is safe to call WARN_ON_ONCE()
when nbcon context is acquired. People will use it. And I believe
that it _is_ safe.

Best Regards,
Petr