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

[Lance Yang]

On 2025/12/17 20:48, Petr Mladek wrote:
> Adding Joel into Cc. He is improving the sysctl API...
>
> On Mon 2025-12-15 22:00:36, Aaron Tomlin wrote:
> > Introduce support for writing to /proc/sys/kernel/hung_task_detect_count.
> >
> > Writing any value to this file atomically resets the counter of detected
> > hung tasks to zero. This grants system administrators the ability to clear
> > the cumulative diagnostic history after resolving an incident, simplifying
> > monitoring without requiring a system restart.
> >
> > --- a/Documentation/admin-guide/sysctl/kernel.rst
> > +++ b/Documentation/admin-guide/sysctl/kernel.rst
> > @@ -418,7 +418,7 @@ hung_task_detect_count
> >   ======================
> >   
> >   Indicates the total number of tasks that have been detected as hung since
> > -the system boot.
> > +the system boot. The counter can be reset to zero when written to.
> >   
> >   This file shows up if ``CONFIG_DETECT_HUNG_TASK`` is enabled.
> >   
> > diff --git a/kernel/hung_task.c b/kernel/hung_task.c
> > index 5902573200c0..01ce46a107b0 100644
> > --- a/kernel/hung_task.c
> > +++ b/kernel/hung_task.c
> > @@ -375,6 +375,31 @@ 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
> > + * @write: 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.
> > + * Returns 0 on success or a negative error code on failure.
> > + */
> > +static int proc_dohung_task_detect_count(const struct ctl_table *table, int write,
> > +					 void *buffer, size_t *lenp, loff_t *ppos)
> > +{
> > +	if (!write)
> > +		return proc_doulongvec_minmax(table, write, buffer, lenp, ppos);
>
> There have been some changes in the sysctl API recently, see
> https://lore.kernel.org/lkml/20251016-jag-sysctl_conv-v2-0-a2f16529acc4@xxxxxxxxxx/
>
> They are backward compatible, so the above code works. But it would be
> nice to make it up-to-date, namely:
>
>    + Replace "write" with "dir"
>    + Use SYSCTL_USER_TO_KERN(dir) instead of (!write)
>
>
> > +	WRITE_ONCE(sysctl_hung_task_detect_count, 0);
>
> I might be too conservative. But it looks weird to allow clearing the
> value by any write. It would be better to return -EINVAL for non-zero
> values. This would require using a copy of struct ctl_table and read
> the value into a temporary variable.

That's okay, I think. See vmstat_refresh() for a similar pattern - it
accepts any write value to trigger the refresh operation without
validating the specific value ;)