| 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: <c40f5578-7c07-4328-9885-500957a7dcab@linux.dev>
Date: Wed, 30 Jul 2025 21:46:16 +0800
From: Lance Yang <lance.yang@...ux.dev>
To: "Masami Hiramatsu (Google)" <mhiramat@...nel.org>
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 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;
Otherwise, looks good to me:
Acked-by: Lance Yang <lance.yang@...ux.dev>
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();
>
Powered by blists - more mailing lists