[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20260102035415.4094835-1-mpdesouza@suse.com>
Date: Fri, 2 Jan 2026 00:54:14 -0300
From: Marcos Paulo de Souza <mpdesouza@...e.com>
To: Breno Leitao <leitao@...ian.org>
Cc: Marcos Paulo de Souza <mpdesouza@...e.com>,
Andrew Lunn <andrew+netdev@...n.ch>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
netdev@...r.kernel.org,
linux-kernel@...r.kernel.org,
asantostc@...il.com,
efault@....de,
gustavold@...il.com,
calvin@...nvd.org,
jv@...sburgh.net,
kernel-team@...a.com
Subject: Re: [PATCH net-next 2/2] netconsole: convert to NBCON console infrastructure
On Mon, 22 Dec 2025 06:52:11 -0800 Breno Leitao <leitao@...ian.org> wrote:
> Convert netconsole from the legacy console API to the NBCON framework.
> NBCON provides threaded printing which unblocks printk()s and flushes in
> a thread, decoupling network TX from printk() when netconsole is
> in use.
>
> Since netconsole relies on the network stack which cannot safely operate
> from all atomic contexts, mark both consoles with
> CON_NBCON_ATOMIC_UNSAFE. (See discussion in [1])
>
> CON_NBCON_ATOMIC_UNSAFE restricts write_atomic() usage to emergency
> scenarios (panic) where regular messages are sent in threaded mode.
>
> Implementation changes:
> - Unify write_ext_msg() and write_msg() into netconsole_write()
> - Add device_lock/device_unlock callbacks to manage target_list_lock
> - Use nbcon_enter_unsafe()/nbcon_exit_unsafe() around network operations
> - Set write_thread and write_atomic callbacks (both use same function)
>
> Link: https://lore.kernel.org/all/b2qps3uywhmjaym4mht2wpxul4yqtuuayeoq4iv4k3zf5wdgh3@tocu6c7mj4lt/ [1]
> Signed-off-by: Breno Leitao <leitao@...ian.org>
> ---
> drivers/net/netconsole.c | 95 +++++++++++++++++++++++++++++-------------------
> 1 file changed, 58 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
> index dc3bd7c9b049..248b401bcaa4 100644
> --- a/drivers/net/netconsole.c
> +++ b/drivers/net/netconsole.c
> @@ -1709,22 +1709,6 @@ static void send_ext_msg_udp(struct netconsole_target *nt, const char *msg,
> sysdata_len);
> }
>
> -static void write_ext_msg(struct console *con, const char *msg,
> - unsigned int len)
> -{
> - struct netconsole_target *nt;
> - unsigned long flags;
> -
> - if ((oops_only && !oops_in_progress) || list_empty(&target_list))
> - return;
> -
> - spin_lock_irqsave(&target_list_lock, flags);
> - list_for_each_entry(nt, &target_list, list)
> - if (nt->extended && nt->enabled && netif_running(nt->np.dev))
> - send_ext_msg_udp(nt, msg, len);
> - spin_unlock_irqrestore(&target_list_lock, flags);
> -}
> -
> static void send_msg_udp(struct netconsole_target *nt, const char *msg,
> unsigned int len)
> {
> @@ -1739,29 +1723,60 @@ static void send_msg_udp(struct netconsole_target *nt, const char *msg,
> }
> }
>
> -static void write_msg(struct console *con, const char *msg, unsigned int len)
> +/**
> + * netconsole_write - Generic function to send a msg to all targets
> + * @wctxt: nbcon write context
> + * @extended: "true" for extended console mode
> + *
> + * Given a nbcon write context, send the message to the netconsole
> + * targets
> + */
> +static void netconsole_write(struct nbcon_write_context *wctxt,
> + bool extended)
> {
> - unsigned long flags;
> struct netconsole_target *nt;
>
> if (oops_only && !oops_in_progress)
> return;
> - /* Avoid taking lock and disabling interrupts unnecessarily */
> - if (list_empty(&target_list))
> - return;
>
> - spin_lock_irqsave(&target_list_lock, flags);
> list_for_each_entry(nt, &target_list, list) {
> - if (!nt->extended && nt->enabled && netif_running(nt->np.dev)) {
> - /*
> - * We nest this inside the for-each-target loop above
> - * so that we're able to get as much logging out to
> - * at least one target if we die inside here, instead
> - * of unnecessarily keeping all targets in lock-step.
> - */
> - send_msg_udp(nt, msg, len);
> - }
> + if (nt->extended != extended || !nt->enabled ||
> + !netif_running(nt->np.dev))
> + continue;
> +
> + if (!nbcon_enter_unsafe(wctxt))
> + continue;
In this case, I believe that it should return directly? If it can't enter in the
unsafe region the output buffer is not reliable anymore, so retrying the send
the buffer to a different target isn't correct anymore. Petr, John, do you
agree?
> +
> + if (extended)
> + send_ext_msg_udp(nt, wctxt->outbuf, wctxt->len);
> + else
> + send_msg_udp(nt, wctxt->outbuf, wctxt->len);
> +
> + nbcon_exit_unsafe(wctxt);
> }
> +}
> +
> +static void netconsole_write_ext(struct console *con __always_unused,
> + struct nbcon_write_context *wctxt)
> +{
> + netconsole_write(wctxt, true);
> +}
> +
> +static void netconsole_write_basic(struct console *con __always_unused,
> + struct nbcon_write_context *wctxt)
> +{
> + netconsole_write(wctxt, false);
> +}
> +
> +static void netconsole_device_lock(struct console *con __always_unused,
> + unsigned long *flags)
> +{
> + spin_lock_irqsave(&target_list_lock, *flags);
> +}
> +
> +static void netconsole_device_unlock(struct console *con __always_unused,
> + unsigned long flags)
> +{
> spin_unlock_irqrestore(&target_list_lock, flags);
> }
>
> @@ -1924,15 +1939,21 @@ static void free_param_target(struct netconsole_target *nt)
> }
>
> static struct console netconsole_ext = {
> - .name = "netcon_ext",
> - .flags = CON_ENABLED | CON_EXTENDED,
> - .write = write_ext_msg,
> + .name = "netcon_ext",
> + .flags = CON_ENABLED | CON_EXTENDED | CON_NBCON | CON_NBCON_ATOMIC_UNSAFE,
> + .write_thread = netconsole_write_ext,
> + .write_atomic = netconsole_write_ext,
> + .device_lock = netconsole_device_lock,
> + .device_unlock = netconsole_device_unlock,
> };
>
> static struct console netconsole = {
> - .name = "netcon",
> - .flags = CON_ENABLED,
> - .write = write_msg,
> + .name = "netcon",
> + .flags = CON_ENABLED | CON_NBCON | CON_NBCON_ATOMIC_UNSAFE,
> + .write_thread = netconsole_write_basic,
> + .write_atomic = netconsole_write_basic,
> + .device_lock = netconsole_device_lock,
> + .device_unlock = netconsole_device_unlock,
> };
>
> static int __init init_netconsole(void)
>
> --
> 2.47.3
Sent using hkml (https://github.com/sjp38/hackermail)
Powered by blists - more mailing lists