Re: [PATCH 1/2] printk: Store all three timestamps

[Petr Mladek]

On Thu 2020-09-24 02:06:12, John Ogness wrote:
> On 2020-09-23, Petr Mladek <pmladek@xxxxxxxx> wrote:
> > diff --git a/kernel/printk/printk_ringbuffer.h b/kernel/printk/printk_ringbuffer.h
> > index 0adaa685d1ca..09082c8472d3 100644
> > --- a/kernel/printk/printk_ringbuffer.h
> > +++ b/kernel/printk/printk_ringbuffer.h
> > @@ -14,7 +15,7 @@
> >   */
> >  struct printk_info {
> >  	u64	seq;		/* sequence number */
> > -	u64	ts_nsec;	/* timestamp in nanoseconds */
> > +	struct ktime_timestamps ts; /* timestamps */
> 
> Until now struct printk_info has contained generic types. If we add
> struct ktime_timestamps, we may start storing more than we need. For
> example, if more (possibly internal) fields are added to struct
> ktime_timestamps that printk doesn't care about. We may prefer to
> generically and explicitly store the information we care about:
> 
>     u64 ts_mono;
>     u64 ts_boot;
>     u64 ts_real;
> 
> Or create our own struct printk_ts to copy the fields of interest to.

I would like to have a structure if we have more timestamps.

Honestly, printk-specific structure sounds like an overhead to me.
How big is the chance that struct ktime_timestamps ts would get
modified? It has been created for printk after all.

That said, I could live with printk-specific structure.
We might even need it if we need to store also local_clock().

Best Regards,
Petr