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

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

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@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
accepts any write value to trigger the refresh operation without
validating the specific value ;)