Re: [PATCH v8 0/2] hung_task: Improve warning budget handling and task reporting

On Thu, Aug 06, 2026 at 10:03:57AM -0400, Aaron Tomlin wrote:
>On Thu, Aug 06, 2026 at 10:05:10AM +0800, Lance Yang wrote:
>> 
>> 
>> On 2026/8/5 22:16, Aaron Tomlin wrote:
>> > On Wed, Aug 05, 2026 at 10:13:05AM +0800, Lance Yang wrote:
>> > > 
>> > > 
>> > > On 2026/8/5 07:05, Andrew Morton wrote:
>> > > > On Tue,  4 Aug 2026 16:20:48 -0400 Aaron Tomlin <[email protected]> wrote:
>> > > > 
[...]
>> > > > 
>> > > > Thanks.  A couple of concerns from AI review:
>> > > > 	https://sashiko.dev/#/patchset/[email protected]
>> > > 
>> > > I'm not quite sure what the cleanest way to handle these is yet, but
>> > > both points look fair.
>> > > 
>> > > 1) Concurrent writes to hung_task_warnings can race and leave
>> > >     hung_task_warnings_printed out of sync with it.
>> > > 
>> > > 2) The unconditional pr_err() is also no longer bounded by
>> > >     hung_task_warnings. With lots of hung tasks, every scan can flood
>> > >     the log and console with one line per task. Maybe rate-limit those
>> > >     messages or cap them per scan.
>> > > 
>> > > > Apologies if these were considered during review of previous
>> > > > iterations.
>> > 
>> > Hi Andrew, Lance,
>> > 
>> > Yes. However, I feel the first one is of a lesser concern. For instance,
>> > consider the following race scenario, when two threads write to the sysctl
>> > concurrently:
>> > 
>> >      - Thread A writes value 10, 'writes sysctl_hung_task_warnings = 10'
>> >      - Thread B writes value 20, 'writes sysctl_hung_task_warnings = 20'
>> >      - Thread B executes 'hung_task_warnings_printed =
>> >        sysctl_hung_task_warnings' (i.e., sets 20)
>> > 
>> >      - Thread A resumes and executes 'hung_task_warnings_printed =
>> >        sysctl_hung_task_warnings' using its _cached_ register value 10
>> > 
>> > The result, sysctl_hung_task_warnings holds 20, but
>> > hung_task_warnings_printed holds 10.
>> > 
>> > I suspect the severity is low since concurrent sysctl writes are likely
>> > rare—restricted to CAP_SYS_ADMIN. Finally, if de-synchronisation occurs,
>> > the system automatically self-heals as soon as a watchdog check finds zero
>> > hung tasks (this_round_count == 0), resetting hung_task_warnings_printed =
>> > sysctl_hung_task_warnings.
>> > 
>> > However, I would rather not leave the data race unresolved. How about using
>> > READ_ONCE() and WRITE_ONCE()? I think multi-variable transactional
>> > atomicity is unnecessary:
>> 
>> Doesn't close the race. A can read 10, B can finish both updates
>> with 20, then A writes 10 back. Still ends up 20/10.
>> 
>> READ_ONCE()/WRITE_ONCE() don't serialize anything here ...
>
>Yes; READ_ONCE() and WRITE_ONCE() guarantee single-copy load/store
>atomicity to prevent compiler optimisations. However, I agree, they do not
>establish a critical section or serialise multi-step operations across
>variables 'sysctl_hung_task_warnings' and 'hung_task_warnings_printed'.
>How about the following?

Still races with khungtaskd ... It can read 10, sysctl resets budget
to 0, then khungtaskd writes 9 back. We end up with sysctl at 0 and
runtime budget at 9 :)

READ_ONCE()/WRITE_ONCE() don't fix that lost update ...

Let's keep it simpler: make khungtaskd sole owner of
hung_task_warnings_printed. Sysctl write just sets a reset flag:

static atomic_t reset_hung_task_warnings = ATOMIC_INIT(0);

...

static int proc_dohung_task_warnings(const struct ctl_table *table, int write,
...
{	ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);
	if (!ret && write)
		atomic_set_release(&reset_hung_task_warnings, 1);
...
}

khungtaskd picks it up next scan:

	if (atomic_xchg(&reset_hung_task_warnings, 0))
		hung_task_warnings_printed =
			READ_ONCE(sysctl_hung_task_warnings);


That's it. Only khungtaskd touches runtime budget, so no lock needed
there. Write during a scan takes effect next scan, keeping whole scan
on one budget. Multiple writes collapse into one reset, and writing
same value still refills budget.

Played around with it a bit and ended up with the following on top:

---8<---
diff --git a/kernel/hung_task.c b/kernel/hung_task.c
index 6ebb3a87ac65..ea7817e1856b 100644
--- a/kernel/hung_task.c
+++ b/kernel/hung_task.c
@@ -60,6 +60,7 @@ static unsigned long __read_mostly sysctl_hung_task_check_interval_secs;
 static int __read_mostly sysctl_hung_task_warnings = 10;

 static int hung_task_warnings_printed = 10;
+static atomic_t reset_hung_task_warnings = ATOMIC_INIT(0);

 static int __read_mostly did_panic;
 static bool hung_task_call_panic;
@@ -244,11 +245,6 @@ static void hung_task_info(struct task_struct *t, unsigned long timeout,
 		hung_task_call_panic = true;
 	}

-	/* Always print the blocked message */
-	pr_err("INFO: task %s:%d blocked%s for more than %ld seconds.\n",
-	       t->comm, t->pid, t->in_iowait ? " in I/O wait" : "",
-	       (jiffies - t->last_switch_time) / HZ);
-
 	/*
 	 * The given task did not get scheduled for more than
 	 * CONFIG_DEFAULT_HUNG_TASK_TIMEOUT. Therefore, complain
@@ -257,6 +253,10 @@ static void hung_task_info(struct task_struct *t, unsigned long timeout,
 	if (hung_task_warnings_printed || hung_task_call_panic) {
 		if (hung_task_warnings_printed > 0)
 			hung_task_warnings_printed--;
+		pr_err("INFO: task %s:%d blocked%s for more than %ld seconds.\n",
+		       t->comm, t->pid,
+		       t->in_iowait ? " in I/O wait" : "",
+		       (jiffies - t->last_switch_time) / HZ);
 		pr_err("      %s %s %.*s\n",
 			print_tainted(), init_utsname()->release,
 			(int)strcspn(init_utsname()->version, " "),
@@ -308,7 +308,7 @@ static void check_hung_uninterruptible_tasks(unsigned long timeout)
 	unsigned long last_break = jiffies;
 	struct task_struct *g, *t;
 	unsigned long this_round_count;
-	int need_warning = hung_task_warnings_printed;
+	int need_warning;
 	unsigned long si_mask = hung_task_si_mask;

 	/*
@@ -318,6 +318,11 @@ static void check_hung_uninterruptible_tasks(unsigned long timeout)
 	if (test_taint(TAINT_DIE) || did_panic)
 		return;

+	if (atomic_xchg(&reset_hung_task_warnings, 0))
+		hung_task_warnings_printed =
+			READ_ONCE(sysctl_hung_task_warnings);
+	need_warning = hung_task_warnings_printed;
+
 	this_round_count = 0;
 	rcu_read_lock();
 	for_each_process_thread(g, t) {
@@ -345,10 +350,15 @@ static void check_hung_uninterruptible_tasks(unsigned long timeout)
 	rcu_read_unlock();

 	if (!this_round_count) {
-		hung_task_warnings_printed = sysctl_hung_task_warnings;
+		hung_task_warnings_printed =
+			READ_ONCE(sysctl_hung_task_warnings);
 		return;
 	}

+	if (!hung_task_warnings_printed && !hung_task_call_panic)
+		pr_info("khungtaskd: %lu hung tasks detected (warning budget exhausted)\n",
+			this_round_count);
+
 	if (need_warning || hung_task_call_panic) {
 		si_mask |= SYS_INFO_LOCKS;

@@ -438,13 +448,10 @@ static int proc_dohung_task_warnings(const struct ctl_table *table, int write,
 	int ret;

 	ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);
+	if (!ret && write)
+		atomic_set_release(&reset_hung_task_warnings, 1);

-	if (ret || !write)
-		return ret;
-
-	hung_task_warnings_printed = sysctl_hung_task_warnings;
-
-	return 0;
+	return ret;
 }

 /*
---

Cheers, Lance

[...]