Re: [PATCH v3] Fix "log" command when crash is, started with "--minimal" option

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

 



在 2020年08月16日 00:00, crash-utility-request@xxxxxxxxxx 写道:
> Send Crash-utility mailing list submissions to
> 	crash-utility@xxxxxxxxxx
> 
> To subscribe or unsubscribe via the World Wide Web, visit
> 	https://www.redhat.com/mailman/listinfo/crash-utility
> or, via email, send a message with subject or body 'help' to
> 	crash-utility-request@xxxxxxxxxx
> 
> You can reach the person managing the list at
> 	crash-utility-owner@xxxxxxxxxx
> 
> When replying, please edit your Subject line so it is more specific
> than "Re: Contents of Crash-utility digest..."
> 
> 
> Today's Topics:
> 
>    1. Re: Crash-utility Digest, Vol 179, Issue 4 (David Wysochanski)
> 
> 
> ----------------------------------------------------------------------
> 
> Message: 1
> Date: Sat, 15 Aug 2020 04:45:49 -0400
> From: David Wysochanski <dwysocha@xxxxxxxxxx>
> To: HAGIO KAZUHITO(?? ??) <k-hagio-ab@xxxxxxx>
> Cc: lijiang <lijiang@xxxxxxxxxx>, "Discussion list for crash utility
> 	usage,	maintenance and development" <crash-utility@xxxxxxxxxx>
> Subject: Re:  Crash-utility Digest, Vol 179, Issue 4
> Message-ID:
> 	<CALF+zOkMQoyDn8cP5fBpMrrusvcqytMUW+tzs=npKUWwG=nH1g@xxxxxxxxxxxxxx>
> Content-Type: text/plain; charset="UTF-8"
> 
> On Thu, Aug 13, 2020 at 8:47 PM HAGIO KAZUHITO(?????)
> <k-hagio-ab@xxxxxxx> wrote:
>>
>> -----Original Message-----
>>> From: crash-utility-bounces@xxxxxxxxxx <crash-utility-bounces@xxxxxxxxxx> On Behalf Of lijiang
>>> Sent: Friday, August 14, 2020 8:31 AM
>>> To: David Wysochanski <dwysocha@xxxxxxxxxx>
>>> Cc: Discussion list for crash utility usage, maintenance and development <crash-utility@xxxxxxxxxx>
>>> Subject: Re:  Crash-utility Digest, Vol 179, Issue 4
>>>
>>> ? 2020?08?13? 22:58, David Wysochanski ??:
>>>> On Thu, Aug 13, 2020 at 9:08 AM lijiang <lijiang@xxxxxxxxxx> wrote:
>>>>>
>>>>> ? 2020?08?13? 16:33, David Wysochanski ??:
>>>>>> Hi Lianbo
>>>>>>
>>>>>> On Sat, Aug 8, 2020 at 10:46 PM lijiang <lijiang@xxxxxxxxxx> wrote:
>>>>>>>
>>>>>>> ? 2020?08?07? 00:00, crash-utility-request@xxxxxxxxxx ??:
>>>>>>>> Message: 5
>>>>>>>> Date: Thu,  6 Aug 2020 09:30:22 -0400
>>>>>>>> From: Dave Wysochanski <dwysocha@xxxxxxxxxx>
>>>>>>>> To: crash-utility@xxxxxxxxxx
>>>>>>>> Subject:  [PATCH v3] Fix "log" command when crash is
>>>>>>>>       started with "--minimal" option
>>>>>>>> Message-ID: <20200806133022.2127538-1-dwysocha@xxxxxxxxxx>
>>>>>>>>
>>>>>>>> Commit c86250bce29f introduced the useful '-T' option to print the
>>>>>>>> log timestamp in human-readable form.  However, this option does
>>>>>>>> not work when crash is invoked with '--minimal' mode, and if tried,
>>>>>>>> crash will spin at 100% and continuously crash at a divide by 0
>>>>>>>> because machdep->hz == 0.
>>>>>>>>
>>>>>>>> Fix this by disallowing this option in minimal mode.  In addition,
>>>>>>>> only calculate the logic to calculate kt->boot_date.tv_sec
>>>>>>>> when this option is enabled.
>>>>>>>>
>>>>>>> Hi, Dave Wysochanski
>>>>>>>
>>>>>>> Thank you for the patch.
>>>>>>>
>>>>>>>> Fixes: c86250bce29f ("Introduction of the "log -T" option...")
>>>>>>>> Signed-off-by: Dave Wysochanski <dwysocha@xxxxxxxxxx>
>>>>>>>> Reviewed-by: Wang Long <w@xxxxxxxxxxxxx>
>>>>>>>> Tested-by: Mathias Krause <minipli@xxxxxxxxxxxxxx>
>>>>>>>> ---
>>>>>>>>  kernel.c | 5 ++++-
>>>>>>>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>>>>>>>
>>>>>>>> diff --git a/kernel.c b/kernel.c
>>>>>>>> index 5ed6021..95119f3 100644
>>>>>>>> --- a/kernel.c
>>>>>>>> +++ b/kernel.c
>>>>>>>> @@ -4939,7 +4939,10 @@ cmd_log(void)
>>>>>>>>          if (argerrs)
>>>>>>>>                  cmd_usage(pc->curcmd, SYNOPSIS);
>>>>>>>>
>>>>>>>> -     if (kt->boot_date.tv_sec == 0) {
>>>>>>>> +     if (msg_flags & SHOW_LOG_CTIME && pc->flags & MINIMAL_MODE)
>>>>>>>> +             error(FATAL, "log: option 'T' not available in minimal mode\n");
>>>>>>>> +
>>>>>>>> +     if (msg_flags & SHOW_LOG_CTIME && kt->boot_date.tv_sec == 0) {
>>>>>>>
>>>>>>> The above two 'if' statements have the same checking condition, would you mind putting them together
>>>>>>> as a statement block? E.g:
>>>>>>>
>>>>>> Sure I can resubmit a fixup of v4 patch once there are no more changes needed.
>>>>>>
>>>>>>> +       if (msg_flags & SHOW_LOG_CTIME) {
>>>>>>> +               if (pc->flags & MINIMAL_MODE) {
>>>>>>> +                       error(WARNING, "the option '-T' not available in minimal mode\n");
>>>>>>> +                       return;
>>>>>>> +               }
>>>>>>> +
>>>>>>> +               if (kt->boot_date.tv_sec == 0) {
>>>>>>> ...
>>>>>>> +               }
>>>>>>>         }
>>>>>>>
>>>>>>> In addition, might it be more reasonable to issue a warning instead of a fatal error?
>>>>>>>
>>>>>>
>>>>>> If you use WARNING it will not fix the infinite loop / CPU spin at
>>>>>> 100%.  You have to CTRL-C the crash program to get the prompt back.
>>>>>> So I do not think this is a good idea.
>>>>>>
>>>>> How did you reproduce it? Can you help to confirm if you have applied the correct patch
>>>>> as below?
>>>>>
>>>>> [root@intel-sharkbay-mb-03 crash]# git diff kernel.c
>>>>> diff --git a/kernel.c b/kernel.c
>>>>> index 5ed6021..6375b24 100644
>>>>> --- a/kernel.c
>>>>> +++ b/kernel.c
>>>>> @@ -4939,13 +4939,20 @@ cmd_log(void)
>>>>>          if (argerrs)
>>>>>                  cmd_usage(pc->curcmd, SYNOPSIS);
>>>>>
>>>>> -       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) {
>>>>> +               if (pc->flags & MINIMAL_MODE) {
>>>>> +                       error(WARNING, "the option '-T' 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_AUDIT) {
>>>>>
>>>>>
>>>>> I didn't see any problems, it's strange, this is my test steps.
>>>>>
>>>>
>>>> You are right - I missed the 'return;' in your patch.  The WARNING is fine.
>>>>
>>> Thanks for your confirmation.
>>>
>>>> How do you want to handle this?  Do you want to take the original header
>>>> and add your signed-off-by line and commit your patch?  Or do you want
>>>> me to resubmit with review-by or signed-off-by lines?
>>>>
>>> No, please do not add my signed-off-by and review-by line.
>>>
>>> If you and Kazu have no objection, you could post it again with the above changes.
>>
>> No objection.  I can ack a new one with the above change.
>>
> 
> I would suggest taking the v3 patch as is because Lianbo has said not
> to add his signed-off-by line because I did not write that portion.  I'm not
> going to modify something written by someone else and omit where it
> came from.
> 
Hi, David

Sorry for the misunderstanding, I don't oppose to adding my signature, but, I just think
that, maybe it doesn't make sense to add my signature.

If you like the improvement suggestions, can you please post it again with or without
my signature?

Crash utility really needs the fix ASAP. :-)

Thanks.
Lianbo

> Thanks!
> 
> 
>> Thanks,
>> Kazu
>>
>>> Otherwise Kazu can help to merge your last patch, because it can also work.
>>>
>>> Thanks.
>>> Lianbo
>>>
>>> --
>>> Crash-utility mailing list
>>> Crash-utility@xxxxxxxxxx
>>> https://www.redhat.com/mailman/listinfo/crash-utility
> 
> 
> 
> 
> ------------------------------
> 
> --
> Crash-utility mailing list
> Crash-utility@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/crash-utility
> 
> End of Crash-utility Digest, Vol 179, Issue 19
> **********************************************
> 

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




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

 

Powered by Linux