RE: [PATCH] kernel/hung_task.c: force ignore_loglevel before panic

[Liu, Chuansheng]

> -----Original Message-----
> From: Tetsuo Handa [mailto:penguin-kernel@xxxxxxxxxxxxxxxxxxx]
> Sent: Monday, December 10, 2018 5:59 PM
> To: Sergey Senozhatsky <sergey.senozhatsky.work@xxxxxxxxx>; Liu,
> Chuansheng <chuansheng.liu@xxxxxxxxx>
> Cc: akpm@xxxxxxxxxxxxxxxxxxxx; pmladek@xxxxxxxx;
> sergey.senozhatsky@xxxxxxxxx; rostedt@xxxxxxxxxxx; dvyukov@xxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH] kernel/hung_task.c: force ignore_loglevel before panic
> 
> On 2018/12/10 15:11, Sergey Senozhatsky wrote:
> > On (12/10/18 05:58), Liu, Chuansheng wrote:
> >>> On (12/10/18 05:40), Liu, Chuansheng wrote:
> >>>> @@ -130,6 +130,13 @@ static void check_hung_task(struct task_struct
> >>>> *t,
> >>> unsigned long timeout)
> >>>>                         init_utsname()->version);
> >>>>                 pr_err("\"echo 0 > /proc/sys/kernel/hung_task_timeout_secs\""
> >>>>                         " disables this message.\n");
> >>>> +               /* When sysctl_hung_task_panic is set, we have to force
> >>>> +                * ignore_loglevel to get really useful hung task
> >>>> +                * information.
> >>>> +                */
> >>>> +               if (sysctl_hung_task_panic && !ignore_loglevel)
> >>>> +                       ignore_loglevel = true;
> >>>
> >>> 		console_verbose()?
> >>
> >> Thanks Sergey, it is really my need. I will prepare for a new version
> >> of patch:)
> >
> > Let's wait for people to take a look at this patch first.
> 
> Shouldn't console_verbose() be called like
> 
> -    if (!sysctl_hung_task_warnings && !sysctl_hung_task_panic)
> +    if (sysctl_hung_task_panic)
> +        console_verbose();
> +    else if (!sysctl_hung_task_warnings)
>          return;
> 
> or
> 
> -    if (!sysctl_hung_task_warnings && !sysctl_hung_task_panic)
> -        return;
> +    if (sysctl_hung_task_panic)
> +        console_verbose();
> 
> or
> 
> -    if (!sysctl_hung_task_warnings && !sysctl_hung_task_panic)
> -        return;
> +    if (sysctl_hung_task_panic) {
> +        console_verbose();
> +        hung_task_show_lock = true;
> +        hung_task_call_panic = true;
> +    }
> (...snipped...)
> -    if (sysctl_hung_task_panic) {
> -        hung_task_show_lock = true;
> -        hung_task_call_panic = true;
> -    }
Thanks Tetsuo, I prefer this option, which makes code more readable.

> 
> so that sysctl_hung_task_warnings == 0 && sysctl_hung_task_panic == 1 will
> call debug_show_all_locks() and trigger_all_cpu_backtrace() with verbose level?
More thoughts in this condition of sysctl_hung_task_warnings == 0 && sysctl_hung_task_panic == 1,
in this case, debug_show_all_locks() may not output useful information if LOCK DEBUG config is not enabled.
trigger_all_cpu_backtrace() will not show the hung task for debugging either.

We may enhance it by:
-       if (sysctl_hung_task_warnings) {
+       if (sysctl_hung_task_panic || sysctl_hung_task_warnings) {
                if (sysctl_hung_task_warnings > 0)
                        sysctl_hung_task_warnings--;