public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Lance Yang <lance.yang@linux.dev>
To: Aaron Tomlin <atomlin@atomlin.com>
Cc: sean@ashe.io, linux-kernel@vger.kernel.org, pmladek@suse.com,
	joel.granados@kernel.org, gregkh@linuxfoundation.org,
	akpm@linux-foundation.org, mhiramat@kernel.org
Subject: Re: [v4 PATCH 2/2] hung_task: Enable runtime reset of hung_task_detect_count
Date: Mon, 22 Dec 2025 10:21:31 +0800	[thread overview]
Message-ID: <77749a90-9209-4495-8995-9e4535483e07@linux.dev> (raw)
In-Reply-To: <20251222014210.2032214-3-atomlin@atomlin.com>



On 2025/12/22 09:42, Aaron Tomlin wrote:
> Introduce support for writing to /proc/sys/kernel/hung_task_detect_count.
> 
> Writing a value of zero to this file atomically resets the counter of
> detected hung tasks. This grants system administrators the ability to
> clear the cumulative diagnostic history after resolving an incident,
> simplifying monitoring without requiring a system restart.
> 
> Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>
> ---
>   Documentation/admin-guide/sysctl/kernel.rst |  3 +-
>   kernel/hung_task.c                          | 76 ++++++++++++++++++---
>   2 files changed, 67 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/admin-guide/sysctl/kernel.rst b/Documentation/admin-guide/sysctl/kernel.rst
> index 239da22c4e28..68da4235225a 100644
> --- a/Documentation/admin-guide/sysctl/kernel.rst
> +++ b/Documentation/admin-guide/sysctl/kernel.rst
> @@ -418,7 +418,8 @@ hung_task_detect_count
>   ======================
>   
>   Indicates the total number of tasks that have been detected as hung since
> -the system boot.
> +the system boot or since the counter was reset. The counter is zeroed when
> +a value of 0 is written.
>   
>   This file shows up if ``CONFIG_DETECT_HUNG_TASK`` is enabled.
>   
> diff --git a/kernel/hung_task.c b/kernel/hung_task.c
> index 00c3296fd692..70b3db047f5d 100644
> --- a/kernel/hung_task.c
> +++ b/kernel/hung_task.c
> @@ -17,6 +17,7 @@
>   #include <linux/export.h>
>   #include <linux/panic_notifier.h>
>   #include <linux/sysctl.h>
> +#include <linux/atomic.h>
>   #include <linux/suspend.h>
>   #include <linux/utsname.h>
>   #include <linux/sched/signal.h>
> @@ -36,7 +37,7 @@ static int __read_mostly sysctl_hung_task_check_count = PID_MAX_LIMIT;
>   /*
>    * Total number of tasks detected as hung since boot:
>    */
> -static unsigned long __read_mostly sysctl_hung_task_detect_count;
> +static atomic_long_t sysctl_hung_task_detect_count = ATOMIC_LONG_INIT(0);
>   
>   /*
>    * Limit number of tasks checked in a batch.
> @@ -246,20 +247,26 @@ static inline void hung_task_diagnostics(struct task_struct *t)
>   }
>   
>   static void check_hung_task(struct task_struct *t, unsigned long timeout,
> -		unsigned long prev_detect_count)
> +			    unsigned long prev_detect_count)
>   {
> -	unsigned long total_hung_task;
> +	unsigned long total_hung_task, current_detect;
>   
>   	if (!task_is_hung(t, timeout))
>   		return;
>   
>   	/*
>   	 * This counter tracks the total number of tasks detected as hung
> -	 * since boot.
> +	 * since boot. If a reset occurred during the scan, we treat the
> +	 * current count as the new delta to avoid an underflow error.
> +	 * Ensure hang details are globally visible before the counter
> +	 * update.
>   	 */
> -	sysctl_hung_task_detect_count++;
> +	current_detect = atomic_long_inc_return_acquire(&sysctl_hung_task_detect_count);
> +	if (current_detect >= prev_detect_count)
> +		total_hung_task = current_detect - prev_detect_count;
> +	else
> +		total_hung_task = current_detect;
>   
> -	total_hung_task = sysctl_hung_task_detect_count - prev_detect_count;
>   	trace_sched_process_hang(t);
>   
>   	if (sysctl_hung_task_panic && total_hung_task >= sysctl_hung_task_panic) {
> @@ -318,7 +325,8 @@ static void check_hung_uninterruptible_tasks(unsigned long timeout)
>   	int max_count = sysctl_hung_task_check_count;
>   	unsigned long last_break = jiffies;
>   	struct task_struct *g, *t;
> -	unsigned long prev_detect_count = sysctl_hung_task_detect_count;
> +	/* Acquire prevents reordering task checks before this point. */
> +	unsigned long prev_detect_count = atomic_long_read_acquire(&sysctl_hung_task_detect_count);
>   	int need_warning = sysctl_hung_task_warnings;
>   	unsigned long si_mask = hung_task_si_mask;
>   
> @@ -346,7 +354,9 @@ static void check_hung_uninterruptible_tasks(unsigned long timeout)
>    unlock:
>   	rcu_read_unlock();
>   
> -	if (!(sysctl_hung_task_detect_count - prev_detect_count))
> +	/* Ensures we see all hang details recorded during the scan. */
> +	if (!(atomic_long_read_acquire(&sysctl_hung_task_detect_count) -
> +				       prev_detect_count))
>   		return;

Hmm, I think we're missing the same underflow check here ...

If reset happens mid-scan, it can also underflow and cause
false positives in the diagnostics :)

we should apply the same "if (current < prev) use current" logic
here as Petr metioned before.

[...]

Cheers,
Lance

  reply	other threads:[~2025-12-22  2:21 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-22  1:42 [v4 PATCH 0/2] hung_task: Provide runtime reset interface for hung task detector Aaron Tomlin
2025-12-22  1:42 ` [v4 PATCH 1/2] hung_task: Introduce helper for hung task warning Aaron Tomlin
2025-12-22  1:42 ` [v4 PATCH 2/2] hung_task: Enable runtime reset of hung_task_detect_count Aaron Tomlin
2025-12-22  2:21   ` Lance Yang [this message]
2025-12-23  1:27     ` Aaron Tomlin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=77749a90-9209-4495-8995-9e4535483e07@linux.dev \
    --to=lance.yang@linux.dev \
    --cc=akpm@linux-foundation.org \
    --cc=atomlin@atomlin.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=joel.granados@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=pmladek@suse.com \
    --cc=sean@ashe.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox