Re: [PATCH] Move the initialization of "boot_date" to task_init()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Thank you for the comment, Kazu.
On Tue, Jan 18, 2022 at 2:26 PM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab@xxxxxxx> 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@xxxxxxx>

Thanks,
Kazu

> Signed-off-by: Lianbo Jiang <lijiang@xxxxxxxxxx>
> ---
>  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

--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/crash-utility

[Index of Archives]     [Fedora Development]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]

 

Powered by Linux