From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f52.google.com (mail-wm1-f52.google.com [209.85.128.52]) (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 5F0053587D0 for ; Wed, 17 Dec 2025 13:09:19 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.52 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1765976962; cv=none; b=IdAC2VZ4rWPD27bwDHNoEVRLErJF9esEWyQ/syYMCsA11TmCTNfRx0Av9JkNLsxvSDxbjxAGtJzeo5/4yRODilzRiMZ0Zvgwu9m9W0X2e7HH5aT6rVl5W7mlpFKn9ihnzo0SRAGgFdBSSYvQMFEoKIwPOYJfcT5yXr6AWpn2lTQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1765976962; c=relaxed/simple; bh=9L08EYwgH3/HTQT0gioQ18dlNcJuR/r3aBpJYX9KJuU=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=YkfqJBezB6lDkTI9Zmq7k+JnhZ7o8bDz8LWBPx5Vav+iWd7KoVi4drzYlIWB3fZrX+HQhEAkvJNIB6yvJVcbYwDwzwv1wNs65kLV4xTnLvULwck+ilfLT9bOQALFbFe5E9j+rAu38G/0C0bZPS2Q79916ZzGxESR3Le8KRpdFaw= 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=F5PbXl3L; arc=none smtp.client-ip=209.85.128.52 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="F5PbXl3L" Received: by mail-wm1-f52.google.com with SMTP id 5b1f17b1804b1-47798ded6fcso37787305e9.1 for ; Wed, 17 Dec 2025 05:09:19 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=google; t=1765976958; x=1766581758; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=ck1sN1pWTNwI61WjDlqM076btj6Q+wSC2DlG+xFUko8=; b=F5PbXl3L5np/w2Po/IiD/WO24uHqNP3Jq7ETx5wc9HYnQvaKvlvCOIYh/k+ki/b2X1 6MG6Qpa7CHQ9DQp3wGyZz3O8BOOyA9nmxZJajzDo5RWAnrPIbVUD9w1aJ55iofrsnGwq Wub9GC8UJE68oSuvCs1tzjAkVdD5F2xTv2wlKsrylrRXdtN/Bu0cOi/Umm6ETTCHr0mM Md8JooKFfNs7fhrjWupFJ3lTXDUueyrPVRghOqJjeG7J//aogJaGNPHY1RjkZygJumuz JEs7k7dGXuzxBoLlLQPSq3G4ICT2beBBIQbRgjK9PIfT7t0kBU5q6JptvAEc9Q6IPZLS Czkw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1765976958; x=1766581758; h=in-reply-to:content-disposition: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; bh=ck1sN1pWTNwI61WjDlqM076btj6Q+wSC2DlG+xFUko8=; b=T8ROaCo6Km22YHBpErJTSsM2xMR3W/u0+L4N4TRoREgeU8jqDkBx4g/BKuxu5d2v7I 53HvVpq3j7ct5W5r5BWwEOpQO7A0J7rzx8wbzz3QuiqB9QsYBcxyFHBPgjuFfpS5284Y FDsPZ5KIFnM6OU5z5uM5jXkFKxkOXImVnGdRRLQVshe5STWdgTIrwIKFazf/sXhmBCHr poKfXGA+jmHlPoPyHtllGlHQCG7VEVpVFb83Q+LLsCK2CWaeEo/O6vMNEqoFIoaX+bGO 6BncJSIpiPyNsN/7Py4S88aytl0/sev/GqhDCXQNxIH8nTm03gaRuxGJhjGjL/hv8nHE yBQQ== X-Forwarded-Encrypted: i=1; AJvYcCXWTMbIMU6cWfwIQuCQaa66/N9lwxK7aP2/9u5K9b5xktANXd99YJHJwxq8NdyR7oiF05z72UWPdtXQzNc=@vger.kernel.org X-Gm-Message-State: AOJu0YyxKBisBjpdElTLt+uDRgY7nZmwNkIrYfV2inYdiGmFClhSrOzK Uoz/N9OVqok2bnW9NaVWUJhJ++jbJ3hR7TN0dIT2hWzF/DFtb5s5rQasbM3yjCQj+kA= X-Gm-Gg: AY/fxX5mPgAm6nH1zOFwq+VHd+/UoE7OZkvr5T4onKGe2WDby7W2Ps98rbxNbFN0oYC f/isJ5o1c8/Q7l7pqQcW92g2v6JJJlzmq4h+8JINlko3DVWqKfdlqFT/UpUE4k5+0Xg/8pWzJbv Gz57WyJ9fE4emLXICPyTtgtgR+SBIXdtqaD9a6t+HQdZFdusQgpuyE6mS6rdoj6C8uK3+8xue7v BAPYjGW3JuaYx2hYYyQEGe+Iz1z1a0sHIAK1s2Z+quP9M5b1h87SVY1PFv6DjVSF7zEuQjBgOqx cXDj3at8iyRlJ9rW1FzrO+ld8+EJW77K/duXoNq3eZ+u/frovx2hyG34ZCQQBbWsPOgrqGbwAJY ArxSSzqG+Vihd9lPWR+0bSpoevuCES2hkRSWjP7YJjYlvIv6/UzzQsQqmKtDjjjNN98mfWwBO6f 0Rsx4wQyV2PK9Pdw== X-Google-Smtp-Source: AGHT+IHMoa9sWCMBQuV3PT6Liiora9k3CtR0jmVAsXmUEWL1Te762eWjWaQLxa+1ziQ+9bBEvqYgmg== X-Received: by 2002:a05:6000:4383:b0:430:f182:7896 with SMTP id ffacd0b85a97d-430f1827af4mr17977440f8f.23.1765976957535; Wed, 17 Dec 2025 05:09:17 -0800 (PST) Received: from pathway.suse.cz ([176.114.240.130]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-4310ada8299sm4702709f8f.2.2025.12.17.05.09.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 17 Dec 2025 05:09:16 -0800 (PST) Date: Wed, 17 Dec 2025 14:09:14 +0100 From: Petr Mladek To: Lance Yang Cc: Aaron Tomlin , sean@ashe.io, linux-kernel@vger.kernel.org, mhiramat@kernel.org, akpm@linux-foundation.org, gregkh@linuxfoundation.org Subject: Re: [PATCH v3 2/2] hung_task: Enable runtime reset of hung_task_detect_count Message-ID: References: <20251216030036.1822217-1-atomlin@atomlin.com> <20251216030036.1822217-3-atomlin@atomlin.com> <79e00527-0ada-4c43-915e-1f8f6589d051@linux.dev> 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: <79e00527-0ada-4c43-915e-1f8f6589d051@linux.dev> On Wed 2025-12-17 15:31:25, Lance Yang wrote: > On 2025/12/16 11:00, 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); > > + > > + WRITE_ONCE(sysctl_hung_task_detect_count, 0); > > The reset uses WRITE_ONCE() but the increment in check_hung_task() > is plain ++, it's just a stat cunter so fine though ;) I was about to wave this away as well. But I am afraid that it is even more complicated. The counter is used to decide how many hung tasks were found in by a single check, see: static void check_hung_uninterruptible_tasks(unsigned long timeout) { [...] unsigned long prev_detect_count = sysctl_hung_task_detect_count; [...] for_each_process_thread(g, t) { [...] check_hung_task(t, timeout, prev_detect_count); } [...] if (!(sysctl_hung_task_detect_count - prev_detect_count)) return; if (need_warning || hung_task_call_panic) { si_mask |= SYS_INFO_LOCKS; if (sysctl_hung_task_all_cpu_backtrace) si_mask |= SYS_INFO_ALL_BT; } sys_info(si_mask); if (hung_task_call_panic) panic("hung_task: blocked tasks"); } Any race might cause false positives and even panic() !!! And the race window is rather big (checking all processes). This race can't be prevented by an atomic read/write. IMHO, the only solution would be to add some locking, e.g. mutex and take it around the entire check_hung_uninterruptible_tasks(). On one hand, it might work. The code is called from a task context... But adding a lock into a lockup-detector code triggers some warning bells in my head. Honestly, I am not sure if it is worth it. I understand the motivation for the reset. But IMHO, even more important is to make sure that the watchdog works as expected. Best Regards, Petr