Based on patch commit 401c636a0eeb ("kernel/hung_task.c:
show all hung tasks before panic"), we could get the
call stack of hung task.
However, if the console loglevel is not high, we still
can not get the useful information in practice, and
in most cases users don't set console loglevel to
high level.
This patch is to force ignore_loglevel before system
panic, so that the real useful information can be seen
in the console, instead of being like the following,
which doesn't have hung task information.
[ 246.916600] INFO: task init:1 blocked for more than 120 seconds.
[ 246.922320] Tainted: G U W 4.19.0-quilt-2e5dc0ac-g51b6c21d76cc #1
[ 246.926790] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[ 246.932553] Kernel panic - not syncing: hung_task: blocked tasks
[ 246.938503] CPU: 2 PID: 479 Comm: khungtaskd Tainted: G U W 4.19.0-quilt-2e5dc0ac-g51b6c21d76cc #1
[ 246.990266] Call Trace:
[ 246.991707] dump_stack+0x4f/0x65
[ 246.993710] panic+0xde/0x231
[ 246.995445] watchdog+0x290/0x410
[ 246.997390] kthread+0x12c/0x150
[ 246.999301] ? reset_hung_task_detector+0x20/0x20
[ 247.004825] ? kthread_create_worker_on_cpu+0x70/0x70
[ 247.007735] ret_from_fork+0x35/0x40
[ 247.010280] reboot: panic mode set: p,w
[ 247.012619] Kernel Offset: 0x34000000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffffbfffffff)
Signed-off-by: Chuansheng Liu <[email protected]>
---
include/linux/printk.h | 2 +-
kernel/hung_task.c | 7 +++++++
kernel/printk/printk.c | 2 +-
3 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/include/linux/printk.h b/include/linux/printk.h
index cf3eccf..24748c1 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -59,8 +59,8 @@ static inline const char *printk_skip_headers(const char *buffer)
*/
#define CONSOLE_LOGLEVEL_DEFAULT CONFIG_CONSOLE_LOGLEVEL_DEFAULT
#define CONSOLE_LOGLEVEL_QUIET CONFIG_CONSOLE_LOGLEVEL_QUIET
-
extern int console_printk[];
+extern bool ignore_loglevel;
#define console_loglevel (console_printk[0])
#define default_message_loglevel (console_printk[1])
diff --git a/kernel/hung_task.c b/kernel/hung_task.c
index cb8e3e8..7d942d1 100644
--- a/kernel/hung_task.c
+++ b/kernel/hung_task.c
@@ -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;
+
sched_show_task(t);
hung_task_show_lock = true;
}
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 1b2a029..31a7a56 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -1135,7 +1135,7 @@ void __init setup_log_buf(int early)
free, (free * 100) / __LOG_BUF_LEN);
}
-static bool __read_mostly ignore_loglevel;
+bool __read_mostly ignore_loglevel;
static int __init ignore_loglevel_setup(char *str)
{
--
2.7.4
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()?
-ss
> -----Original Message-----
> From: Sergey Senozhatsky [mailto:[email protected]]
> Sent: Monday, December 10, 2018 1:46 PM
> To: Liu, Chuansheng <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH] kernel/hung_task.c: force ignore_loglevel before panic
>
> 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:)
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.
-ss
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;
- }
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?
On (12/10/18 18:58), Tetsuo Handa wrote:
> >>>> + */
> >>>> + 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_panic)
> + console_verbose();
Sure, that was what I meant. IOW console_verbose() instead of
`ignore_loglevel = true' assignment.
-ss
> -----Original Message-----
> From: Tetsuo Handa [mailto:[email protected]]
> Sent: Monday, December 10, 2018 5:59 PM
> To: Sergey Senozhatsky <[email protected]>; Liu,
> Chuansheng <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]
> 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--;
On 2018/12/11 10:16, Liu, Chuansheng wrote:
> 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--;
Why ignore sysctl_hung_task_warnings? The administrator can already
configure as sysctl_hung_task_warnings == -1 && sysctl_hung_task_panic == 1
if he/she does not want to suppress neither sched_show_task() nor
debug_show_all_locks()/trigger_all_cpu_backtrace(). Someone might want
that sysctl_hung_task_warnings == 0 (which is a request to suppress only
sched_show_task()) should not be ignored by sysctl_hung_task_panic == 1
(which is a request to trigger panic).
On Tue 2018-12-11 01:16:10, Liu, Chuansheng wrote:
> > From: Tetsuo Handa [mailto:[email protected]]
> > 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
> > - 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.
I like this variant as well. Also using console_verbose() looks
like a better choice.
> 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--;
I agree with Tetsuo that we should not touch this.
The warnings will get typically printed because panic() will get
triggered on the first occasion. And we should respect when the admin
disables the warnings completely.
Best Regards,
Petr
On Wed 2018-12-12 01:16:11, Liu, Chuansheng wrote:
>
>
> > -----Original Message-----
> > From: Tetsuo Handa [mailto:[email protected]]
> > Sent: Tuesday, December 11, 2018 6:02 PM
> > To: Liu, Chuansheng <[email protected]>; Sergey Senozhatsky
> > <[email protected]>
> > Cc: [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]
> > Subject: Re: [PATCH] kernel/hung_task.c: force ignore_loglevel before panic
> >
> > On 2018/12/11 10:16, Liu, Chuansheng wrote:
> > > 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--;
> >
> > Why ignore sysctl_hung_task_warnings? The administrator can already
> > configure as sysctl_hung_task_warnings == -1 && sysctl_hung_task_panic == 1
> > if he/she does not want to suppress neither sched_show_task() nor
> > debug_show_all_locks()/trigger_all_cpu_backtrace(). Someone might want that
> > sysctl_hung_task_warnings == 0 (which is a request to suppress only
> > sched_show_task()) should not be ignored by sysctl_hung_task_panic == 1
> > (which is a request to trigger panic).
>
>
> My complete idea is in patch V1 which has been sent. Paste here:
> If sysctl_hung_task_panic == 1, I will force sched_show_task(t) and set
> hung_task_call_panic = true
> hung_task_show_lock = true
Please, do not mix two changes into one patch.
Add console_verbose() in one patch. It is simple and
everyone has agreed with it so far.
Force sched_show_task() when hung_task_call_panic == 1 in
another patch. It seems to be controversial and should be
discussed/changed separately.
Best Regards,
Petr
> -----Original Message-----
> From: Petr Mladek [mailto:[email protected]]
> Sent: Wednesday, December 12, 2018 6:18 PM
> To: Liu, Chuansheng <[email protected]>
> Cc: Tetsuo Handa <[email protected]>; Sergey Senozhatsky
> <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]
> Subject: Re: [PATCH] kernel/hung_task.c: force ignore_loglevel before panic
>
> On Wed 2018-12-12 01:16:11, Liu, Chuansheng wrote:
> >
> >
> > > -----Original Message-----
> > > From: Tetsuo Handa [mailto:[email protected]]
> > > Sent: Tuesday, December 11, 2018 6:02 PM
> > > To: Liu, Chuansheng <[email protected]>; Sergey Senozhatsky
> > > <[email protected]>
> > > Cc: [email protected]; [email protected];
> > > [email protected]; [email protected];
> [email protected];
> > > [email protected]
> > > Subject: Re: [PATCH] kernel/hung_task.c: force ignore_loglevel before panic
> > >
> > > On 2018/12/11 10:16, Liu, Chuansheng wrote:
> > > > 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--;
> > >
> > > Why ignore sysctl_hung_task_warnings? The administrator can already
> > > configure as sysctl_hung_task_warnings == -1 && sysctl_hung_task_panic
> == 1
> > > if he/she does not want to suppress neither sched_show_task() nor
> > > debug_show_all_locks()/trigger_all_cpu_backtrace(). Someone might want
> that
> > > sysctl_hung_task_warnings == 0 (which is a request to suppress only
> > > sched_show_task()) should not be ignored by sysctl_hung_task_panic == 1
> > > (which is a request to trigger panic).
> >
> >
> > My complete idea is in patch V1 which has been sent. Paste here:
> > If sysctl_hung_task_panic == 1, I will force sched_show_task(t) and set
> > hung_task_call_panic = true
> > hung_task_show_lock = true
>
> Please, do not mix two changes into one patch.
Thanks your suggestion.
>
> Add console_verbose() in one patch. It is simple and
> everyone has agreed with it so far.
Has sent it out, please help review.
>
> Force sched_show_task() when hung_task_call_panic == 1 in
> another patch. It seems to be controversial and should be
> discussed/changed separately.
I found some other points also, and will send out patches later.