Re: [PATCH] printk: add cpu id information to printk() output

Petr Mladek
2023-09-22T08:05:36Z

On Fri 2023-09-22 15:20:37, Enlin Mu wrote:
> Petr Mladek <pmladek@suse.com> 于2023年9月16日周六 00:34写道：
> >
> > On Fri 2023-09-15 11:53:13, Greg KH wrote:
> > > On Fri, Sep 15, 2023 at 04:46:02PM +0800, Enlin Mu wrote:
> > > > John Ogness <john.ogness@linutronix.de> 于2023年9月15日周五 16:34写道：
> > > > >
> > > > > On 2023-09-15, Enlin Mu <enlin.mu@outlook.com> wrote:
> > > > > > Sometimes we want to print cpu id of printk() messages to consoles
> > > > > >
> > > > > > diff --git a/include/linux/threads.h b/include/linux/threads.h
> > > > > > index c34173e6c5f1..6700bd9a174f 100644
> > > > > > --- a/include/linux/threads.h
> > > > > > +++ b/include/linux/threads.h
> > > > > > @@ -34,6 +34,9 @@
> > > > > > #define PID_MAX_LIMIT (CONFIG_BASE_SMALL ? PAGE_SIZE * 8 : \
> > > > > > (sizeof(long) > 4 ? 4 * 1024 * 1024 : PID_MAX_DEFAULT))
> > > > > >
> > > > > > +#define CPU_ID_SHIFT 23
> > > > > > +#define CPU_ID_MASK 0xff800000
> > > > >
> > > > > This only supports 256 CPUs. I think it doesn't make sense to try to
> > > > > squish CPU and Task IDs into 32 bits.
> > > > Yes, it is not good way,
> > > > >
> > > > > What about introducing a caller_id option to always only print the CPU
> > > > > ID? Or do you really need Task _and_ CPU?
> > > > Yes, I need it.Because I need to know which CPU is printing the
> > > > log, so that I can identify the current system operation, such as load
> > > > situation and CPU busy/idle status
> > >
> > > The cpu that is printing the log isn't the one that added the log
> > > message, so I think you will have incorrect data here, right?
> >
> > We already store some metadata about the caller:
> >
> > * All fields are set by the printk code except for @seq, which is
> > * set by the ringbuffer code.
> > */
> > struct printk_info {
> > u64 seq; /* sequence number */
> > u64 ts_nsec; /* timestamp in nanoseconds */
> > u16 text_len; /* length of text message */
> > u8 facility; /* syslog facility */
> > u8 flags:5; /* internal record flags */
> > u8 level:3; /* syslog level */
> > u32 caller_id; /* thread id or processor id */
> >
> > struct dev_printk_info dev_info;
> > };
> >
> > The 32-bit caller ID is generated using:
> >
> > static inline u32 printk_caller_id(void)
> > {
> > return in_task() ? task_pid_nr(current) :
> > 0x80000000 + smp_processor_id();
> > }
> >
> > We could add more metadata and always store the CPU ID and something
> > like:
> >
> > [CTXT][ Tpid][ Ccpu]
> >
> > for example
> >
> > [TASK][ T234][ C4]
> > [ IRQ][ T4567][ C17]
> > [SIRQ][ T5][ C0]
> > [ NMI][ T356][ C128]
> >
> Greate!
> Do you have a plan to push it to linus?

No. It was just a POC. It would require much more effort to make
it ready for upstream, see below.

> > The biggest problem is that it would change the format of the
> > ringbuffer so that it would require updating external tools,
> > working with crashdump, especially crash but there are also
> > alternative python extensions for gdb.

It would require patches for the crash tool,
./scripts/gdb/linux/dmesg.py,
Documentation/admin-guide/kdump/gdbmacros.txt

> > See below POC of the kernel part. It is not even compile tested. The size
> > of the buffers is updated by a guess. Comments are not updated, ...

And of course, make the POC working, update comments, ...

I am sorry but I do not have enough time and motivation to do so.
But I could answer questions, review the patches, ... when any
interested person start working on it.

Best Regards,
Petr