Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 24 Nov 2022 16:22:29 +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 5/7] printk: Use struct console_buffers
Message-ID: <Y3+MNS0yarwSgpR+@alley>
References: <20221123231400.614679-1-john.ogness@linutronix.de>
 <20221123231400.614679-6-john.ogness@linutronix.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20221123231400.614679-6-john.ogness@linutronix.de>
Precedence: bulk

On Thu 2022-11-24 00:19:58, John Ogness wrote:
> From: Thomas Gleixner <tglx@linutronix.de>
> 
> Replace the separately allocated output buffers with a single
> instance of struct console_buffers.
> 
> Note that the buffer size of devkmsg_user.text_buf, when replaced
> with console_buffers.text, reduces from CONSOLE_EXT_LOG_MAX to
> CONSOLE_LOG_MAX. However, the buffer is only used to read ringbuffer
> records. These records have a maximum size of LOG_LINE_MAX, which
> is defined as CONSOLE_LOG_MAX - PREFIX_MAX.
> 
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2007,6 +2007,10 @@ static int console_trylock_spinning(void)
>  	return 1;
>  }
>  
> +#if DROPPED_TEXT_MAX > CONSOLE_EXT_LOG_MAX
> +#error "DROPPED_TEXT_MAX too large for console_buffers->ext_text"
> +#endif

A better solution would be to remove DROPPED_TEXT_MAX completely
and pass the buffer size to call_console_driver(). I mean to do:

static void call_console_driver(struct console *con, const char *text, size_t len,
-				char *dropped_text)
+				char *dropped_text, size_t dropped_text_size)
{
	size_t dropped_len;

-	if (con->dropped && dropped_text) {
-		dropped_len = snprintf(dropped_text, DROPPED_TEXT_MAX,
+	if (con->dropped && dropped_text && dropped_text_size) {
+		dropped_len = snprintf(dropped_text, dropped_text_size,
				       "** %lu printk messages dropped **\n",
				       con->dropped);
		con->dropped = 0;
		con->write(con, dropped_text, dropped_len);
	}

	con->write(con, text, len);
}

The rest looks good. And it is a nice clean up.

Best Regards,
Petr