[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <41fd7652-df1f-26f6-aba0-b87ebae07db6@i-love.sakura.ne.jp>
Date: Fri, 26 Jul 2019 22:28:53 +0900
From: Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>
To: Dmitry Safonov <dima@...sta.com>, linux-kernel@...r.kernel.org
Cc: Dmitry Safonov <0x7f454c46@...il.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Dmitry Vyukov <dvyukov@...gle.com>,
Ingo Molnar <mingo@...nel.org>,
Jonathan Corbet <corbet@....net>,
Thomas Gleixner <tglx@...utronix.de>,
"Peter Zijlstra (Intel)" <peterz@...radead.org>,
Vasiliy Khoruzhick <vasilykh@...sta.com>,
linux-doc@...r.kernel.org, linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH] hung_task: Allow printing warnings every check interval
On 2019/07/26 20:29, Tetsuo Handa wrote:
> On 2019/07/25 23:25, Dmitry Safonov wrote:
>> Yes, also current distributions already using the counter to print
>> warnings number of times and then silently ignore. I.e., on my Arch
>> Linux setup:
>> hung_task_warnings:10
>
> You can propose changing the default value of hung_task_warnings to -1.
>
> Current patch might be inconvenient because printk() from hung_task_warning(t, false)
> fails to go to consoles when that "t" was blocked for more than "timeout" seconds, for
>
> if (sysctl_hung_task_panic) {
> console_verbose();
> hung_task_show_lock = true;
> hung_task_call_panic = true;
> }
>
> path which is intended to force printk() to go to consoles is ignored by
>
> /* Don't print warings twice */
> if (!sysctl_hung_task_interval_warnings)
> hung_task_warning(t, true);
>
> when panic() should be called. (The vmcore would contain the printk() output which
> was not sent to consoles if kdump is configured. But vmcore is not always available.)
>
>> Yes, that's why it's disabled by default (=0).
>> I tend to agree that printing with KERN_DEBUG may be better, but in my
>> point of view the patch isn't enough justification for patching
>> sched_show_task() and show_stack().
>
> You can propose sched_show_task_log_lvl() and show_stack_log_lvl() like show_trace_log_lvl().
>
> I think that sysctl_hung_task_interval_warnings should not be decremented automatically.
> I guess that that variable should become a boolean which controls whether to report threads
> (with KERN_DEBUG level) which was blocked for more than sysctl_hung_task_check_interval_secs
> seconds (or a tristate which also controls whether the report should be sent to consoles
> (because KERN_DEBUG level likely prevents sending to consoles)), and
> hung_task_warning(t, false) should be called like
>
> if (time_is_after_jiffies(t->last_switch_time + timeout * HZ)) {
> if (sysctl_hung_task_interval_warnings)
> hung_task_warning(t, false);
> return;
> }
>
> rather than
>
> if (sysctl_hung_task_interval_warnings)
> hung_task_warning(t, false);
> if (time_is_after_jiffies(t->last_switch_time + timeout * HZ))
> return;
>
> .
>
Well, another direction is to disassociate sysctl_hung_task_panic from
sysctl_hung_task_timeout_secs. Since nobody would want to call panic() when
a thread was blocked for only one second, allow sysctl_hung_task_panic to
specify larger than 1, and interpret it as sysctl_hung_task_timeout_secs for
calling panic(). Roughly speaking:
- if (sysctl_hung_task_panic) {
+ unsigned long panic_timeout = READ_ONCE(sysctl_hung_task_panic)
+ if (panic_timeout == 1 || (panic_timeout > 1 &&
+ (jiffies - t->last_switch_time) / HZ >= panic_timeout)) {
console_verbose();
hung_task_show_lock = true;
hung_task_call_panic = true;
}
If use of different loglevel is not a requirement for you, this would be the simplest.
Powered by blists - more mailing lists