linux-kernel - Re: printk: Add process name information to printk() output.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200904103541.GB2693206@kroah.com>
Date:   Fri, 4 Sep 2020 12:35:41 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     John Ogness <john.ogness@...utronix.de>
Cc:     Changki Kim <changki.kim@...sung.com>, pmladek@...e.com,
        sergey.senozhatsky@...il.com, rostedt@...dmis.org,
        changbin.du@...el.com, masahiroy@...nel.org, rd.dunlap@...il.com,
        krzk@...nel.org, linux-kernel@...r.kernel.org
Subject: Re: printk: Add process name information to printk() output.

On Fri, Sep 04, 2020 at 11:53:42AM +0206, John Ogness wrote:
> On 2020-09-04, Changki Kim <changki.kim@...sung.com> wrote:
> > Printk() meesages are the most basic and useful debug method.
> > However, additional information needs in multi-processor.
> > If we add messages with processor id and process name, we can find
> > a problem only with messages when the problem occurs with H/W IP or CPU.
> > This is very useful in narrowing down the scope of the problems.
> 
> [...]
> 
> > diff --git a/kernel/printk/printk_ringbuffer.h b/kernel/printk/printk_ringbuffer.h
> > index e6302da041f9..fcefe9516606 100644
> > --- a/kernel/printk/printk_ringbuffer.h
> > +++ b/kernel/printk/printk_ringbuffer.h
> > @@ -21,6 +22,12 @@ struct printk_info {
> >  	u8	flags:5;	/* internal record flags */
> >  	u8	level:3;	/* syslog level */
> >  	u32	caller_id;	/* thread id or processor id */
> > +#ifdef CONFIG_PRINTK_PROCESS
> > +	int	pid;			/* process id */
> > +	u8	cpu_id;			/* processor id */
> > +	u8	in_interrupt;		/* interrupt conext */
> > +	char process[TASK_COMM_LEN];	/* process name */
> > +#endif
> >  };
> 
> I can understand the desire to have more information with messages. But
> IMHO adding it to the ringbuffer descriptor is the wrong place for
> it. The descriptor should really be limited to data that the printk
> subsystem needs for _itself_. With respect to LOG_CONT, I think we can
> agree that @caller_id is not enough. But there has been discussions [0]
> of having @caller_id provide a better context representation.
> 
> If we want to support adding more meta information to messages, I would
> prefer that the information is either prepended directly to the message
> text string or appended to the dictionary text string. We could even go
> so far as providing a boot argument where a list of information could be
> specified, what should be automatically added to the text/dict strings
> of each message. That would not require any ringbuffer changes and would
> allow new types of information to be added later.
> 
> Something like:
> 
>     printk.format=ts,cpu,comm,pid,in_atomic
> 
> John Ogness
> 
> [0] https://lkml.kernel.org/r/20200719143527.GA566@jagdpanzerIV.localdomain

Ah, finally a good use of the "dictionary" that we all can agree makes
sense :)

This does seem like a better solution overall, thanks.

greg k-h