Re: [v5 PATCH 2/2] hung_task: Enable runtime reset of hung_task_detect_count

[Lance Yang <lance.yang@linux.dev>]

On 2025/12/31 08:41, 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>
> ---

Overall, looks good to me :)

>   Documentation/admin-guide/sysctl/kernel.rst |  3 +-
>   kernel/hung_task.c                          | 82 ++++++++++++++++++---
>   2 files changed, 73 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..3bc72a4e4032 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, cur_detect_count;
>   
>   	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++;
> +	cur_detect_count = atomic_long_inc_return_release(&sysctl_hung_task_detect_count);
> +	if (cur_detect_count >= prev_detect_count)
> +		total_hung_task = cur_detect_count - prev_detect_count;
> +	else
> +		total_hung_task = cur_detect_count;
>   
> -	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,10 +325,12 @@ 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;
> +	unsigned long cur_detect_count, prev_detect_count, delta;
>   	int need_warning = sysctl_hung_task_warnings;
>   	unsigned long si_mask = hung_task_si_mask;
>   
> +	/* Acquire prevents reordering task checks before this point. */
> +	prev_detect_count = atomic_long_read_acquire(&sysctl_hung_task_detect_count);
>   	/*
>   	 * If the system crashed already then all bets are off,
>   	 * do not report extra hung tasks:
> @@ -346,7 +355,14 @@ 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. */
> +	cur_detect_count = atomic_long_read_acquire(&sysctl_hung_task_detect_count);
> +	if (cur_detect_count < prev_detect_count)
> +		delta = cur_detect_count;
> +	else
> +		delta = cur_detect_count - prev_detect_count;
> +
> +	if (!delta)
>   		return;

Right. The underflow check applied in both check_hung_task() and
check_hung_uninterruptible_tasks() to handle reset during scan
looks properly addressed.

>   
>   	if (need_warning || hung_task_call_panic) {
> @@ -371,6 +387,51 @@ static long hung_timeout_jiffies(unsigned long last_checked,
>   }
>   
>   #ifdef CONFIG_SYSCTL
> +
> +/**
> + * proc_dohung_task_detect_count - proc handler for hung_task_detect_count
> + * @table: Pointer to the struct ctl_table definition for this proc entry
> + * @dir: Flag indicating the operation
> + * @buffer: User space buffer for data transfer
> + * @lenp: Pointer to the length of the data being transferred
> + * @ppos: Pointer to the current file offset
> + *
> + * This handler is used for reading the current hung task detection count
> + * and for resetting it to zero when a write operation is performed using a
> + * zero value only. Returns 0 on success or a negative error code on
> + * failure.
> + */
> +static int proc_dohung_task_detect_count(const struct ctl_table *table, int dir,
> +					 void *buffer, size_t *lenp, loff_t *ppos)
> +{

This proc_handler is probably better left for Petr and Joel to review ;)

Nothing else jumped out at me, so:

Acked-by: Lance Yang <lance.yang@linux.dev>

> +	unsigned long detect_count;
> +	struct ctl_table proxy_table;
> +	int err;
> +
> +	proxy_table = *table;
> +	proxy_table.data = &detect_count;
> +
> +	if (SYSCTL_KERN_TO_USER(dir)) {
> +		detect_count = atomic_long_read(&sysctl_hung_task_detect_count);
> +
> +		return proc_doulongvec_minmax(&proxy_table, dir, buffer, lenp, ppos);
> +	}
> +
> +	err = proc_doulongvec_minmax(&proxy_table, dir, buffer, lenp, ppos);
> +	if (err < 0)
> +		return err;
> +
> +	if (SYSCTL_USER_TO_KERN(dir)) {
> +		/* The only valid value for clearing is zero. */
> +		if (detect_count)
> +			return -EINVAL;
> +		atomic_long_set(&sysctl_hung_task_detect_count, 0);
> +	}
> +
> +	*ppos += *lenp;
> +	return err;
> +}
> +
>   /*
>    * Process updating of timeout sysctl
>    */
> @@ -451,10 +512,9 @@ static const struct ctl_table hung_task_sysctls[] = {
>   	},
>   	{
>   		.procname	= "hung_task_detect_count",
> -		.data		= &sysctl_hung_task_detect_count,
>   		.maxlen		= sizeof(unsigned long),
> -		.mode		= 0444,
> -		.proc_handler	= proc_doulongvec_minmax,
> +		.mode		= 0644,
> +		.proc_handler	= proc_dohung_task_detect_count,
>   	},
>   	{
>   		.procname	= "hung_task_sys_info",