linux-kernel - Re: [v5 PATCH 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: <eypytopufn62pvnkvzvmsajna5sceq6ffqltc3sjyz6gp45lqt@k3btitq77yna>
Date: Sat, 10 Jan 2026 10:55:58 -0500
From: Aaron Tomlin <atomlin@...mlin.com>
To: Petr Mladek <pmladek@...e.com>
Cc: akpm@...ux-foundation.org, lance.yang@...ux.dev, mhiramat@...nel.org, 
	gregkh@...uxfoundation.org, joel.granados@...nel.org, sean@...e.io, 
	linux-kernel@...r.kernel.org
Subject: Re: [v5 PATCH 2/2] hung_task: Enable runtime reset of
 hung_task_detect_count

On Thu, Jan 08, 2026 at 03:41:28PM +0100, Petr Mladek wrote:
> The _release() feels a bit weird here because the counter might
> get incremented more times during one scan.
> 
> IMHO, it should be perfectly fine to use the _relaxed version here
> because it is in the middle of the acquire/release, see below.
> The important thing here is that the load/modify/store operation
> is done atomically.

Hi Petr,

Thank you for your review and the detailed feedback.

I agree with your assessment regarding the counter update within
check_hung_task(). The _release semantics are indeed unnecessary in that
specific context; I shall switch to atomic_long_inc_return_relaxed() as the
atomicity of the operation suffices there.

> > -	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);
> 
> This value is read at the end of the scan => _release
> semantic/barrier should be here.

Since we are performing a read/load operation here to capture the current
state, atomic_long_read_acquire() is the correct primitive. It ensures that
if we observe the updated counter, we are also guaranteed to observe any
associated data (such as the hang details) that were "published" by the
writer before the counter was updated. This approach is preferable to
inserting a full memory barrier (smp_mb()), as it provides the necessary
ordering guarantees with less overhead.

> Otherwise, I do not have anything more to add. I agree with the other
> proposals, for example:
> 
>    + remove 1st patch
>    + split 2nd patch into two
>    + changes in the sysctl code proposed by Joel

I entirely agree with your other points regarding the patch structure. I
shall discard the first patch, split the second into distinct logical
changes, and incorporate Joel’s suggestions for the sysctl code.

I will prepare a follow-up patch series incorporating these changes.

Kind regards,
-- 
Aaron Tomlin

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