From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ej1-f44.google.com (mail-ej1-f44.google.com [209.85.218.44]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id EE9EE380FC3 for ; Wed, 26 Aug 2026 11:17:52 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.218.44 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787743076; cv=none; b=JpKS1MHqJfGDZnScWx6FpIO7Zz5GE6HvJEcjerbuqQUCq7LB8hQYBwTC+5h9sZcuVSSLcP6a7RDlMSMjftaMuti0rLAKckyLycpCdXdl6rBqumeofUXiImTugRI8Sw0pdlN3HvCgSZLvFdloer4pBLI2EI1lWiLDGeBO5eSNX+o= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787743076; c=relaxed/simple; bh=lF9fzQ5H34gc0fox++H+KqxiqNwWw/ma3vzHOrPAFdo=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=lgx4gCwXKIO8Plds1KBuXK64YR50fymSJZPXjb3dJxRSiZtvZt0mks6MYjq+XuQvc0P9LHVhEghIn8RDny+7P0B+g01kDhFT3k7bwkX4krJfw9iXhPSSgoDfuF0lw6Dh8Jio0UpM3nYBLKjJkp4qDWvtPwRA3udNlb0C/NMTo6s= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com; spf=pass smtp.mailfrom=suse.com; dkim=pass (2048-bit key) header.d=suse.com header.i=@suse.com header.b=BrbQPvg6; arc=none smtp.client-ip=209.85.218.44 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=suse.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=suse.com header.i=@suse.com header.b="BrbQPvg6" Received: by mail-ej1-f44.google.com with SMTP id a640c23a62f3a-c15d3cd51b2so116345866b.3 for ; Wed, 26 Aug 2026 04:17:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=google; t=1787743070; x=1788347870; darn=vger.kernel.org; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:from:date:from:to:cc:subject :date:message-id:reply-to:content-type; bh=Murt1llViKT4uADpD6XO2EKKOMvr9/LpWgt/soAH39Q=; b=BrbQPvg6JXE34PsfXljSxWrDrOBzJ0JU+e1rt1vGnYqZsDiLLBSF8pSabCKy6x7oXu 0MA/Ag3H6m1AZhfcQ7ipXjSlEAfexASbFwSC5/JvzqsvMA4tbA9BU+jZU3o1QKhXTPso zXUAl2aLeQ1Bk1mEK2Fr57gLw36Bc2Bfa6MkQQXUmsRlEFlJrDUohwrWygpJiaQvDQmp P77szZJTzRVTLd+ES1wfAZJhD+SdOpDOnilv/rxZFJctkyVkywsRZFxaCBhQ+SZP+MyD kRfNuhdvdonxA4mAmNIMoOvTDPloYxJcRrGIQmInnL/9DPzx2c46WVjD8AIoekPoVaMG nSaQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1787743070; x=1788347870; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=Murt1llViKT4uADpD6XO2EKKOMvr9/LpWgt/soAH39Q=; b=CfJUAz36pHHNoG0X9rItRzVJL+a6ld+PKmWwu8P5c4fuQroWkuh6ySBMRPKflaiJcf FmT1jMbl/FEX2vUmMEuabZBHa3QRftcE+6XV20ccyXaOB5AAkJEneH75uNQ6jv53WBi4 w4/EaqzcNJg9m+ydSn/IhUWtwP/A8Wqv6xFJJC5xqHE3sUaASQ1nKeLcTIxiE/ze5mYF 5HPV5PggQiWocrYf//UYfE3NeTdVb8GBQSGv+1c3YuZf9Pv0CRgyquM9/HlJQTCoJNPo tL9DmY0cvh6w/E5/25bEC87+uwPxKgVn5dbNex34bjwk85ljkhfxKZ7O/K6AbXB4qlkE aY5A== X-Forwarded-Encrypted: i=1; AHgh+RrjHJPO/ex9dh0o1A+CbIOc2Ro8XuHTowyQ8sDYUbymHz2MW7hLfZwgy1YTbVmvproU9rzdrj24rXi9YmU=@vger.kernel.org X-Gm-Message-State: AFuF++lFkSJDJaUK8astMXajUJ0I1fpeV37BsGE8ZxZHXlEfBuR7XeiV XPVwEg5od1aHo74gx9BCS99W09D1Pg0VmEyL4MbRORXS+tIP+B+CDzxbXi57MHXsKVY= X-Gm-Gg: AR+sD10jpcFEi1PYjW8IqqDTL/2v3vhbLvoAwDRw7Jh6P9HNxryCkz7InGYnv9z5+wT 1PyG9XQfifKwyuORjxdnGBpWzkQwwmBwyi/5Wrc2w60g6bvAyBAtZZJA2Yby4UGjHFRMocXhzhr MNVFqn6MK7UvK8HbXyRgY4YDBOL9fMdKUgQXrYlpFJk59qUosmzoXxQhHF//0d/qRv1N+tI4R6s GVyx0YEg12l6TFcCOF1/xOMhZyHzpEYL68i8am3/ROhaca7QVbAiY1hXPbfJjenW9sZf2CHrQRF xG47fQsjQOUx/bGYOGpjN5MJIgm1n4HTC+GofEZPuHefyr+v35lOpPHDbb5b0oV7RjxuHHFPvSG aN/Smjs1ve/rNjZqpFGwdPx4yQY1y8CgrntczNq3Xb2vyOowJmRbqdGX9/x1QEdQ9JqCHUG/OyX YxCbyzrZg2DzMOX8WqfRV9NLYluNQTf3ofw7Paz+EY+LIWrdef1SWE64dP5HGkGQ== X-Received: by 2002:a17:907:d0e:b0:c1f:c7f0:b433 with SMTP id a640c23a62f3a-c250bc09ef9mr795519566b.9.1787743070093; Wed, 26 Aug 2026 04:17:50 -0700 (PDT) Received: from pathway.suse.cz ([176.114.240.130]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-c250a72d6dbsm392242366b.24.2026.08.26.04.17.48 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 Aug 2026 04:17:49 -0700 (PDT) Date: Wed, 26 Aug 2026 13:17:47 +0200 From: Petr Mladek To: Aaron Tomlin Cc: akpm@linux-foundation.org, lance.yang@linux.dev, mhiramat@kernel.org, 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 Subject: Re: [PATCH v9 1/2] hung_task: Reset warning budget when problem gets resolved Message-ID: References: <20260814135718.494513-1-atomlin@atomlin.com> <20260814135718.494513-2-atomlin@atomlin.com> 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=us-ascii Content-Disposition: inline In-Reply-To: <20260814135718.494513-2-atomlin@atomlin.com> Hi, first, I am sorry for so late review. I had vacation, many things accumulated, ... On Fri 2026-08-14 09:57:17, Aaron Tomlin wrote: > The sysctl hung_task_warnings currently holds both the configured warning > limit and the remaining budget. Each detailed report decrements the > sysctl, so once it reaches zero, the configured limit is lost and cannot > be restored automatically. > > Keep sysctl hung_task_warnings unchanged and track the remaining budget > in hung_task_warnings_printed. Reset the runtime budget via an atomic flag > when a watchdog check sees no hung tasks or when userspace writes a new > sysctl value. > > --- a/kernel/hung_task.c > +++ b/kernel/hung_task.c > @@ -59,6 +59,9 @@ 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; Nit: The name of the variable is a bit misleading in this final version. It does not longer count the number of printed messages. A better name might be "hung_task_warnings_budget" or so. It would be nice to change it if we need another version. And I am afraid that we would need it, see below. If we change it then I would also add comments explaining the difference between the two values, something like: /* * Limit the number of printed hung tasks to prevent printing * the same or similar backtraces repeatedly. */ static int __read_mostly sysctl_hung_task_warnings = 10; /* * The number of hung tasks which still can be reported. * The budget gets restored to the original limit when * the previous stall is resolved. */ static int hung_task_warnings_budget = 10; > +static atomic_t reset_hung_task_warnings = ATOMIC_INIT(0); > + > static int __read_mostly did_panic; > static bool hung_task_call_panic; > > @@ -245,11 +248,11 @@ static void hung_task_info(struct task_struct *t, unsigned long timeout, > /* > * The given task did not get scheduled for more than > * CONFIG_DEFAULT_HUNG_TASK_TIMEOUT. Therefore, complain > - * accordingly > + * accordingly with full details if the budget is not exhausted. > */ > - if (sysctl_hung_task_warnings || hung_task_call_panic) { > - if (sysctl_hung_task_warnings > 0) > - sysctl_hung_task_warnings--; > + 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); > @@ -264,7 +267,7 @@ static void hung_task_info(struct task_struct *t, unsigned long timeout, > sched_show_task(t); > debug_show_blocker(t, timeout); > > - if (!sysctl_hung_task_warnings) > + if (!hung_task_warnings_printed) > pr_info("Future hung task reports are suppressed, see sysctl kernel.hung_task_warnings\n"); > } > > @@ -304,7 +307,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 = sysctl_hung_task_warnings; > + int need_warning; > unsigned long si_mask = hung_task_si_mask; > > /* > @@ -314,6 +317,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)) I would use here atomic_xchg_acquire(). It serializes the ordering of reset_hung_task_warnings vs sysctl_hung_task_warnings. It would make it symetric with the barrier in the sysctl handler. > + hung_task_warnings_printed = > + READ_ONCE(sysctl_hung_task_warnings); This would work only when "sysctl_hung_task_warnings" is updated using WRITE_ONCE(). But it seems that this is not the case. My understading is that it is updated by: + proc_dointvec_minmax() + do_proc_vec() + proc_get_long() + strtoul_lenient() which does a plain assigment: static int strtoul_lenient(const char *cp, char **endp, unsigned int base, unsigned long *res) { [...] *res = (unsigned long)result; [...] } It can be solved by using temporary variable in proc_dointvec_minmax(). We have a custom proc_dohung_task_warnings() handler anyway. See below. > + need_warning = hung_task_warnings_printed; > + > this_round_count = 0; > rcu_read_lock(); > for_each_process_thread(g, t) { > @@ -340,8 +348,11 @@ static void check_hung_uninterruptible_tasks(unsigned long timeout) > unlock: > rcu_read_unlock(); > > - if (!this_round_count) > + if (!this_round_count) { > + hung_task_warnings_printed = > + READ_ONCE(sysctl_hung_task_warnings); > return; > + } > > if (need_warning || hung_task_call_panic) { > si_mask |= SYS_INFO_LOCKS; > @@ -425,6 +436,19 @@ static int proc_dohung_task_timeout_secs(const struct ctl_table *table, int writ > return ret; > } > > +static int proc_dohung_task_warnings(const struct ctl_table *table, int write, > + void *buffer, > + size_t *lenp, loff_t *ppos) > +{ > + int ret; > + > + ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos); > + if (!ret && write) > + atomic_set_release(&reset_hung_task_warnings, 1); > + > + return ret; > +} We should use WRITE_ONCE() when updating proc_dohung_task_warnings. So, we need similar trick with proxy_table like in proc_dohung_task_detect_count. Something like, on top of this patch: --- a/kernel/hung_task.c +++ b/kernel/hung_task.c @@ -444,13 +444,26 @@ static int proc_dohung_task_warnings(const struct ctl_table *table, int write, void *buffer, size_t *lenp, loff_t *ppos) { + struct ctl_table proxy_table; + int warnings; int ret; - ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos); - if (!ret && write) - atomic_set_release(&reset_hung_task_warnings, 1); + proxy_table = *table; + proxy_table.data = &warnings; - return ret; + if (SYSCTL_KERN_TO_USER(write)) + warnings = READ_ONCE(sysctl_hung_task_warnings); + + ret = proc_dointvec_minmax(&proxy_table, write, buffer, lenp, ppos); + if (ret < 0) + return ret; + + if (SYSCTL_USER_TO_KERN(write)) { + WRITE_ONCE(sysctl_hung_task_warnings, warnings); + atomic_set_release(&reset_hung_task_warnings, 1); + } + + return 0; } /* Otherwise, it looks good to me. Best Regards, Petr