linux-kernel - Re: [PATCH v3 2/2] hung_task: Enable runtime reset of hung_task_detect

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <4c2ewriu3tfrcz6ffxbmi2nozbhedwbffh4nizvoks6klfajzs@kshe6uxgzm6k>
Date: Thu, 18 Dec 2025 10:08:07 +0100
From: Joel Granados <joel.granados@...nel.org>
To: Lance Yang <lance.yang@...ux.dev>
Cc: Petr Mladek <pmladek@...e.com>, akpm@...ux-foundation.org, 
	mhiramat@...nel.org, gregkh@...uxfoundation.org, sean@...e.io, 
	linux-kernel@...r.kernel.org, Aaron Tomlin <atomlin@...mlin.com>
Subject: Re: [PATCH v3 2/2] hung_task: Enable runtime reset of
 hung_task_detect_count

On Wed, Dec 17, 2025 at 09:21:01PM +0800, Lance Yang wrote:
> 
> 
> 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.
I see that this might be refactored due to dependency that was not
considered. I'll write my comments, but will also drop it from my radar
for now.

> > > 
> > > --- 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.
Would it make more sense to write it like this:

  Indicates the total number of tasks that have been detected as hung
  since the system boot or since the counter was reset. Counter is
  zeroed 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@kernel.org/
> > 
> > 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
You are correct but wouldn't it make more sense to zero it out only when
the write value passed by the user is zero. And return -EINVAL for any
other value?


best

-- 

Joel Granados

Download attachment "signature.asc" of type "application/pgp-signature" (660 bytes)