Messages in this thread |  | | Date | Tue, 14 May 2019 15:28:52 -0700 | From | "Paul E. McKenney" <> | Subject | Re: [PATCH] kernel/hung_task.c: Monitor killed tasks. |
| |
On Mon, May 13, 2019 at 08:02:11PM +0900, Tetsuo Handa wrote: > syzbot's second top report is "no output from test machine" where the > userspace process failed to spawn a new test process for 300 seconds > for some reason. One of reasons which can result in this report is that > an already spawned test process was unable to terminate (e.g. trapped at > an unkillable retry loop due to some bug) after SIGKILL was sent to that > process. Therefore, reporting when a thread is failing to terminate > despite a fatal signal is pending would give us more useful information. > > This version shares existing sysctl settings (e.g. check interval, > timeout, whether to panic) used for detecting TASK_UNINTERRUPTIBLE > threads, for I don't know whether people want to use a new kernel > config option and different sysctl settings for monitoring killed > threads. > > Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > Cc: Dmitry Vyukov <dvyukov@google.com>
Looks good to me.
Acked-by: Paul E. McKenney <paulmck@linux.ibm.com>
A few inconsequential comments below.
> --- > include/linux/sched.h | 1 + > kernel/hung_task.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 45 insertions(+) > > diff --git a/include/linux/sched.h b/include/linux/sched.h > index a2cd1585..d42bdd7 100644 > --- a/include/linux/sched.h > +++ b/include/linux/sched.h > @@ -850,6 +850,7 @@ struct task_struct { > #ifdef CONFIG_DETECT_HUNG_TASK > unsigned long last_switch_count; > unsigned long last_switch_time; > + unsigned long killed_time; > #endif > /* Filesystem information: */ > struct fs_struct *fs; > diff --git a/kernel/hung_task.c b/kernel/hung_task.c > index f108a95..34e7b84 100644 > --- a/kernel/hung_task.c > +++ b/kernel/hung_task.c > @@ -141,6 +141,47 @@ static void check_hung_task(struct task_struct *t, unsigned long timeout) > touch_nmi_watchdog(); > } > > +static void check_killed_task(struct task_struct *t, unsigned long timeout) > +{ > + unsigned long stamp = t->killed_time; > + > + /* > + * Ensure the task is not frozen. > + * Also, skip vfork and any other user process that freezer should skip. > + */ > + if (unlikely(t->flags & (PF_FROZEN | PF_FREEZER_SKIP))) > + return; > + /* > + * Skip threads which are already inside do_exit(), for exit_mm() etc. > + * might take many seconds. > + */ > + if (t->flags & PF_EXITING) > + return; > + if (!stamp) { > + stamp = jiffies; > + if (!stamp) > + stamp++;
Cute trick to avoid issues with jiffy overflow on 32-bit systems. ;-)
> + t->killed_time = stamp; > + return; > + } > + if (time_is_after_jiffies(stamp + timeout * HZ))
And if I understand correctly, timeout of zero disables everything, so we don't get the backwards false-positive comparison above.
> + return; > + trace_sched_process_hang(t); > + if (sysctl_hung_task_panic) { > + console_verbose(); > + hung_task_call_panic = true; > + } > + /* > + * This thread failed to terminate for more than > + * sysctl_hung_task_timeout_secs seconds, complain: > + */ > + pr_err("INFO: task %s:%d can't die for more than %ld seconds.\n", > + t->comm, t->pid, (jiffies - stamp) / HZ); > + sched_show_task(t); > + hung_task_show_lock = true; > + touch_nmi_watchdog(); > +} > + > /* > * To avoid extending the RCU grace period for an unbounded amount of time, > * periodically exit the critical section and enter a new one. > @@ -192,6 +233,9 @@ static void check_hung_uninterruptible_tasks(unsigned long timeout) > goto unlock; > last_break = jiffies; > } > + /* Check threads which are about to terminate. */ > + if (unlikely(fatal_signal_pending(t))) > + check_killed_task(t, timeout); > /* use "==" to skip the TASK_KILLABLE tasks waiting on NFS */ > if (t->state == TASK_UNINTERRUPTIBLE) > check_hung_task(t, timeout); > -- > 1.8.3.1 >
|  |