[Crash-utility] [PATCH] Move the initialization of "boot_date" to task_init()

[lijiang]

Thank you for the comment, Kazu.
On Tue, Jan 18, 2022 at 2:26 PM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab at nec.com>
wrote:

> Hi Lianbo,
>
> -----Original Message-----
> > The "boot_date" is initialized conditionally in the cmd_log(), which may
> > display incorrect "boot_date" value with the following command before
> > running the "log -T" command:
> >
> > crash> help -k | grep date
> >           date: Wed Dec 22 13:39:29 IST 2021
> >      boot_date: Thu Jan  1 05:30:00 IST 1970
> >                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> The "help -k" prints internal variables for debugging and it's merely
> uninitialized, so personally I don't think this is a bug, but I agree that
> the current behavior is not friendly and initializing it during startup
> will be helpful.
>
> Yes, you are right.


> > The calculation of "boot_date" depends on the HZ value, and the HZ will
> > be calculated in task_init() at the latest, so let's move it here.
>
> Hmm, initializing the boot_date in task_init() looks a bit strange to me,
>

Me too.
But anyway, as mentioned above, the calculation of HZ is in task_init(), at
least crash can get an expected behavior there.


> but possibly the HZ can be set in machdep_init(POST_GDB), so it's fine..
>
> >
> > Fixes: c86250bce29f ("Introduction of the "log -T" option...")
>
> Could you please remove this Fixes: tag?
>
> Sure. Thanks.
Lianbo


> Without this,
> Acked-by: Kazuhito Hagio <k-hagio-ab at nec.com>
>
> Thanks,
> Kazu
>
> > Signed-off-by: Lianbo Jiang <lijiang at redhat.com>
> > ---
> >  kernel.c | 18 +++---------------
> >  task.c   | 10 ++++++++++
> >  2 files changed, 13 insertions(+), 15 deletions(-)
> >
> > diff --git a/kernel.c b/kernel.c
> > index 36c57ed501ad..094fe9b2efad 100644
> > --- a/kernel.c
> > +++ b/kernel.c
> > @@ -5025,21 +5025,9 @@ cmd_log(void)
> >          if (argerrs)
> >                  cmd_usage(pc->curcmd, SYNOPSIS);
> >
> > -     if (msg_flags & SHOW_LOG_CTIME) {
> > -             if (pc->flags & MINIMAL_MODE) {
> > -                     error(WARNING, "the option '-T' is not available
> in minimal mode\n");
> > -                     return;
> > -             }
> > -
> > -             if (kt->boot_date.tv_sec == 0) {
> > -                     ulonglong uptime_jiffies;
> > -                     ulong  uptime_sec;
> > -
> > -                     get_uptime(NULL, &uptime_jiffies);
> > -                     uptime_sec =
> (uptime_jiffies)/(ulonglong)machdep->hz;
> > -                     kt->boot_date.tv_sec = kt->date.tv_sec -
> uptime_sec;
> > -                     kt->boot_date.tv_nsec = 0;
> > -             }
> > +     if (msg_flags & SHOW_LOG_CTIME && pc->flags & MINIMAL_MODE) {
> > +             error(WARNING, "the option '-T' is not available in
> minimal mode\n");
> > +             return;
> >       }
> >
> >       if (msg_flags & SHOW_LOG_AUDIT) {
> > diff --git a/task.c b/task.c
> > index 76e184ae70b1..263a8344dd94 100644
> > --- a/task.c
> > +++ b/task.c
> > @@ -692,6 +692,16 @@ task_init(void)
> >
> >       stack_overflow_check_init();
> >
> > +     if (machdep->hz) {
> > +             ulonglong uptime_jiffies;
> > +             ulong  uptime_sec;
> > +
> > +             get_uptime(NULL, &uptime_jiffies);
> > +             uptime_sec = (uptime_jiffies)/(ulonglong)machdep->hz;
> > +             kt->boot_date.tv_sec = kt->date.tv_sec - uptime_sec;
> > +             kt->boot_date.tv_nsec = 0;
> > +     }
> > +
> >       tt->flags |= TASK_INIT_DONE;
> >  }
> >
> > --
> > 2.20.1
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/crash-utility/attachments/20220118/f7703824/attachment.htm>