[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aW446yxd-FQ1JJ9Q@pathway.suse.cz>
Date: Mon, 19 Jan 2026 15:00:11 +0100
From: Petr Mladek <pmladek@...e.com>
To: Breno Leitao <leitao@...ian.org>
Cc: John Ogness <john.ogness@...utronix.de>, osandov@...ndov.com,
mpdesouza@...e.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, Simon Horman <horms@...nel.org>,
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>,
rostedt@...dmis.org
Subject: Re: [PATCH net-next 0/2] net: netconsole: convert to NBCON console
infrastructure
On Fri 2026-01-16 10:07:08, Breno Leitao wrote:
> Hello Petr,
>
> On Fri, Jan 16, 2026 at 04:53:48PM +0100, Petr Mladek wrote:
> > > Otherwise, it looks good to me.
> > >
> > > I tried to update your patch with the above proposal to see how
> > > it looks and I got:
> >
> > The change seems to work. I have tested it with the following patch:
>
> First of all, *thank you* so much for spending your time on it, this is
> helpful.
You are welcome.
> > Then the extended console format should show also:
> >
> > ,cpu=XXX,pid=YYY,comm=ZZZ
>
> Are you using this just for testing, or do you plan to get this output?
I used this just for testing. But it looks like a good variant for me.
Note that the above is /dev/kmsg output. It would show dev_printk()
messages the following way:
6,295,1164587,-,caller=T1,cpu=10,pid=1,comm=swapper/0;pci 0000:00:02.1: enabling Extended Tags
SUBSYSTEM=pci
DEVICE=+pci:0000:00:02.1
6,296,1167287,-,caller=T1,cpu=10,pid=1,comm=swapper/0;pci 0000:00:02.2: [1b36:000c] type 01 class 0x060400 PCIe Root Port
SUBSYSTEM=pci
DEVICE=+pci:0000:00:02.2
We would need to use another format for (slow) consoles, for example:
<level>[timestamp][Cxxx][context]comm[pid] message
, where:
+ [Cxxx] would show cpu number like the current context field, e.g [ C123]
+ [context] would show context, ideal in some existing format. But
I know only about lockdep format which looks a bit hard to
understand for me.
For example, it might be, for example [TC,HD] for a task context
with hard irqs disabled.
+ comm[pid] is inspired by the existing messages passed from systemd, e.g.
<30>[ 5.317305][ T1] systemd[1]: Starting Journal Service...
<30>[ 5.349000][ T1] systemd[1]: Starting Load Kernel Modules...
But I guess that this might be long bikesheding about this.
> Context: netconsole outputs the message in a different way, similarly to the
> printk dictionary. I.e, taskname and cpu come after, one entry per line:
>
> <message>
> SUBSYSTEM=net
> DEVICE=+pci:0000:00:1f.6
> cpu=42
> taskname=NetworkManager
> ...
I see. Honestly, I never liked the way how dictionary was printed.
It used another syntax than loglevel, sequence id, and timestamp.
It might be confusing and complicate parsing.
But it is a personal opinion. Others might like it because
it might be easier for human eyes parsing.
IMHO, the information needs some parsing anyway to make it human readable:
+ people do not know the log level meaning out of head
+ message id is there primary to detect lost messages
+ timestamp needs conversion, definitely
Sigh, the use of lower case letters for "cpu" and "taskname" made it
even more inconsistent. I would voted against it if I did the review ;-)
> I would like to keep the same format, given users might be used to this format
> already, where netconsole grabs teh cpu,pid,comm data and massage it before
> outputing. Something as:
>
> static int sysdata_append_taskname(struct netconsole_target *nt, int offset,
> struct nbcon_write_context *wctxt)
> {
> return scnprintf(&nt->sysdata[offset],
> MAX_EXTRADATA_ENTRY_LEN, " taskname=%s\n",
> - current->comm);
> + wctxt->msg_comm);
> }
I see. I think that you could keep it for now. It might take quite
some time until we integrate this for /dev/kmsg, ...
> Here is the full patch I was using to test the integration of netconsole and
> the previous printk patch:
>
> https://github.com/leitao/linux/commit/4175dc10719a15844b3a0bd7aa38158a913181a3
BTW.1: I see that netconsole actually does not show pid. So that we do not
need the trick with caller_id2. But people might want to add it in
the future.
I do not have strong opinion about it. We could change it
anytime later if we did not export the new fields for crash
dump, via the VMCOREINFO_OFFSET() macro.
BTW.2: I also noticed that sysdata_append_msgid() uses netconsole-specific
message counter.
Note that each message has its own sequence number. It is the
.seq member in struct printk_info. It is printed in the extended
console output, see info_print_ext_header(). So it is printed
even on netconsole when this extended format is used.
I wonder if the netconsole-specific counter was added
intentionally.
Best Regards,
Petr
Powered by blists - more mailing lists