From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out-171.mta0.migadu.com (out-171.mta0.migadu.com [91.218.175.171]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 2DCD41FD4 for ; Fri, 7 Aug 2026 04:40:56 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=91.218.175.171 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786077660; cv=none; b=W8xfmTEuZ+UhAulEvOpVCwZBOP4gocT8+v3jHrfut6o3C2UxEMXuJj9JXSPVHumQboQjU7lI8HdmKUkceHSeezwPfr68fXpnhf5jOL9qfAS/AQbp7KHMswMa+mBKZy8qow+6S5TWoYs3KW6vWT04ZeU9ZsHpW2G9NkT/rK6IBYg= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786077660; c=relaxed/simple; bh=K/IJdTvXhS44wqKYyJuR1oAHj7jYdeuUzASud1ARKQA=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version:Content-Type; b=f4vj/rpmBPxR6Fsl4TTYvQTNBcQvLhHaTLJbDOkyAXLC/dCIFSrzd9o1Qfl8aZy8pBATM4pExAuhEFzrBTyDGVg1HQL99yjxm09QtEXh0cd03oNOsDbt6TsN34e560ktbE+Kc51/3PV12WE9p1ASl2Qt43j4pMgYTn8g3SrSYVI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=aGEXrZZa; arc=none smtp.client-ip=91.218.175.171 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="aGEXrZZa" X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1786077654; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ttczckG0jNi+qurJW4iVDyWuD/ebLRFvJuJPoKUdu2g=; b=aGEXrZZaL1/ycUxhu2XsbPXjex0u9TjbzvrhbcT8sKA/VKW6Q4bwImIUnlRucCPiiO79Lt 02uh5v3mbsis64nFwRft0HpH068zv8Ea77tkwQ46Q91n/cXTAWyvmtwDeq+6dSlWMHW/yM itzepwbekoY3lhA5i9+3Y5hdTa9cMcQ= From: Lance Yang To: atomlin@atomlin.com Cc: mhiramat@kernel.org, pmladek@suse.com, linux-kernel@vger.kernel.org, david.laight.linux@gmail.com, neelx@suse.com, sean@ashe.io, chjohnst@gmail.com, steve@abita.co, mproche@gmail.com, nick.lange@gmail.com, akpm@linux-foundation.org, Lance Yang Subject: Re: [PATCH v8 0/2] hung_task: Improve warning budget handling and task reporting Date: Fri, 7 Aug 2026 12:40:27 +0800 Message-Id: <20260807044027.21232-1-lance.yang@linux.dev> In-Reply-To: References: Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Migadu-Flow: FLOW_OUT 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 wrote: >> > > > [...] >> > > > >> > > > Thanks. A couple of concerns from AI review: >> > > > https://sashiko.dev/#/patchset/20260804202050.262427-1-atomlin@atomlin.com >> > > >> > > 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 [...]