| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20250731065050.38905afadd094ddc0d7a8bcf@kernel.org>
Date: Thu, 31 Jul 2025 06:50:50 +0900
From: Masami Hiramatsu (Google) <mhiramat@...nel.org>
To: Lance Yang <lance.yang@...ux.dev>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
Will Deacon <will@...nel.org>, Andrew Morton <akpm@...ux-foundation.org>,
Boqun Feng <boqun.feng@...il.com>, Waiman Long <longman@...hat.com>, Joel
Granados <joel.granados@...nel.org>, Anna Schumaker
<anna.schumaker@...cle.com>, Lance Yang <ioworker0@...il.com>, Kent
Overstreet <kent.overstreet@...ux.dev>, Yongliang Gao
<leonylgao@...cent.com>, Steven Rostedt <rostedt@...dmis.org>, Tomasz Figa
<tfiga@...omium.org>, linux-kernel@...r.kernel.org, Sergey Senozhatsky
<senozhatsky@...omium.org>
Subject: Re: [RFC PATCH] hung_task: Dump blocker task if it is not hung
On Wed, 30 Jul 2025 21:46:16 +0800
Lance Yang <lance.yang@...ux.dev> wrote:
>
>
> On 2025/7/30 17:53, Masami Hiramatsu (Google) wrote:
> > From: Masami Hiramatsu (Google) <mhiramat@...nel.org>
> >
> > Dump the lock blocker task if it is not hung because if the blocker
> > task is also hung, it should be dumped by the detector. This will
> > de-duplicate the same stackdumps if the blocker task is also blocked
> > by another task (and hung).
>
> Makes sense to me ;)
>
> >
> > Suggested-by: Sergey Senozhatsky <senozhatsky@...omium.org>
> > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@...nel.org>
> > ---
> > kernel/hung_task.c | 86 ++++++++++++++++++++++++++++++----------------------
> > 1 file changed, 49 insertions(+), 37 deletions(-)
> >
> > diff --git a/kernel/hung_task.c b/kernel/hung_task.c
> > index d2432df2b905..52d72beb2233 100644
> > --- a/kernel/hung_task.c
> > +++ b/kernel/hung_task.c
> > @@ -94,9 +94,49 @@ static struct notifier_block panic_block = {
> > .notifier_call = hung_task_panic,
> > };
> >
> > +static bool task_is_hung(struct task_struct *t, unsigned long timeout)
> > +{
> > + unsigned long switch_count = t->nvcsw + t->nivcsw;
> > + unsigned int state;
> > +
> > + /*
> > + * skip the TASK_KILLABLE tasks -- these can be killed
> > + * skip the TASK_IDLE tasks -- those are genuinely idle
> > + */
> > + state = READ_ONCE(t->__state);
> > + if (!(state & TASK_UNINTERRUPTIBLE) ||
> > + (state & TASK_WAKEKILL) ||
> > + (state & TASK_NOLOAD))
> > + return false;
> > +
> > + /*
> > + * Ensure the task is not frozen.
> > + * Also, skip vfork and any other user process that freezer should skip.
> > + */
> > + if (unlikely(READ_ONCE(t->__state) & TASK_FROZEN))
> > + return false;
>
>
> Nit: the two separate checks on t->__state could be combined into
> a single read and one conditional check ;)
>
> Something like:
>
> state = READ_ONCE(t->__state);
>
> if (!(state & TASK_UNINTERRUPTIBLE) ||
> (state & (TASK_WAKEKILL | TASK_NOLOAD | TASK_FROZEN)))
> return false;
Ah, Indeed.
>
>
> Otherwise, looks good to me:
> Acked-by: Lance Yang <lance.yang@...ux.dev>
Thanks, let me update it. (also on the next tree)
Thank you!
>
> Thanks,
> Lance
>
> > +
> > + /*
> > + * When a freshly created task is scheduled once, changes its state to
> > + * TASK_UNINTERRUPTIBLE without having ever been switched out once, it
> > + * musn't be checked.
> > + */
> > + if (unlikely(!switch_count))
> > + return false;
> > +
> > + if (switch_count != t->last_switch_count) {
> > + t->last_switch_count = switch_count;
> > + t->last_switch_time = jiffies;
> > + return false;
> > + }
> > + if (time_is_after_jiffies(t->last_switch_time + timeout * HZ))
> > + return false;
> > +
> > + return true;
> > +}
> >
> > #ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
> > -static void debug_show_blocker(struct task_struct *task)
> > +static void debug_show_blocker(struct task_struct *task, unsigned long timeout)
> > {
> > struct task_struct *g, *t;
> > unsigned long owner, blocker, blocker_type;
> > @@ -153,41 +193,21 @@ static void debug_show_blocker(struct task_struct *task)
> > task->comm, task->pid, t->comm, t->pid);
> > break;
> > }
> > - sched_show_task(t);
> > + /* Avoid duplicated task dump, skip if the task is also hung. */
> > + if (!task_is_hung(t, timeout))
> > + sched_show_task(t);
> > return;
> > }
> > }
> > #else
> > -static inline void debug_show_blocker(struct task_struct *task)
> > +static inline void debug_show_blocker(struct task_struct *task, unsigned long timeout)
> > {
> > }
> > #endif
> >
> > static void check_hung_task(struct task_struct *t, unsigned long timeout)
> > {
> > - unsigned long switch_count = t->nvcsw + t->nivcsw;
> > -
> > - /*
> > - * Ensure the task is not frozen.
> > - * Also, skip vfork and any other user process that freezer should skip.
> > - */
> > - if (unlikely(READ_ONCE(t->__state) & TASK_FROZEN))
> > - return;
> > -
> > - /*
> > - * When a freshly created task is scheduled once, changes its state to
> > - * TASK_UNINTERRUPTIBLE without having ever been switched out once, it
> > - * musn't be checked.
> > - */
> > - if (unlikely(!switch_count))
> > - return;
> > -
> > - if (switch_count != t->last_switch_count) {
> > - t->last_switch_count = switch_count;
> > - t->last_switch_time = jiffies;
> > - return;
> > - }
> > - if (time_is_after_jiffies(t->last_switch_time + timeout * HZ))
> > + if (!task_is_hung(t, timeout))
> > return;
> >
> > /*
> > @@ -222,7 +242,7 @@ static void check_hung_task(struct task_struct *t, unsigned long timeout)
> > pr_err("\"echo 0 > /proc/sys/kernel/hung_task_timeout_secs\""
> > " disables this message.\n");
> > sched_show_task(t);
> > - debug_show_blocker(t);
> > + debug_show_blocker(t, timeout);
> > hung_task_show_lock = true;
> >
> > if (sysctl_hung_task_all_cpu_backtrace)
> > @@ -278,7 +298,6 @@ static void check_hung_uninterruptible_tasks(unsigned long timeout)
> > hung_task_show_lock = false;
> > rcu_read_lock();
> > for_each_process_thread(g, t) {
> > - unsigned int state;
> >
> > if (!max_count--)
> > goto unlock;
> > @@ -287,15 +306,8 @@ static void check_hung_uninterruptible_tasks(unsigned long timeout)
> > goto unlock;
> > last_break = jiffies;
> > }
> > - /*
> > - * skip the TASK_KILLABLE tasks -- these can be killed
> > - * skip the TASK_IDLE tasks -- those are genuinely idle
> > - */
> > - state = READ_ONCE(t->__state);
> > - if ((state & TASK_UNINTERRUPTIBLE) &&
> > - !(state & TASK_WAKEKILL) &&
> > - !(state & TASK_NOLOAD))
> > - check_hung_task(t, timeout);
> > +
> > + check_hung_task(t, timeout);
> > }
> > unlock:
> > rcu_read_unlock();
> >
>
--
Masami Hiramatsu (Google) <mhiramat@...nel.org>
Powered by blists - more mailing lists