From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f42.google.com (mail-wm1-f42.google.com [209.85.128.42]) (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 3FC1131A07B for ; Wed, 25 Mar 2026 13:19:29 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.42 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774444771; cv=none; b=mkiDpN1FO6mQh515U7GfniPhgDfWGPuGSr2hTiUqB/3YwJLPevRJwyEdTOcLBXR/1W5Ob8pyIda2fZY0iyEo2d9wz+dmg9qcmJmA8thQ5FtXn/EWSl3IeQiTm0P14wfDxqBt1bQRKYJH8yN460sXBq2IN7Bvb/MM6CtzccFNvFs= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774444771; c=relaxed/simple; bh=zM9oyzSscDGt721SRVAtdoLU0E+WOFV44QQlDVG2StI=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=cY7PlSm1/BLxsM3W0Xkm08txYUyM1owt6KBXCCAa/7c4cTdC4t0JA1k5fstNY/ucE7G2avCweUfu5W9wboIvwTkPcm3PsSkyxA8uepCyy+S94+oa7pmIYad1+2uKHGNmFxwapCMTrYBeBvySP+YuOwnx9R3sInN21BsXjhCelEA= 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=LdKV7lHB; arc=none smtp.client-ip=209.85.128.42 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="LdKV7lHB" Received: by mail-wm1-f42.google.com with SMTP id 5b1f17b1804b1-48558d6ef83so48684995e9.3 for ; Wed, 25 Mar 2026 06:19:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=google; t=1774444767; x=1775049567; darn=vger.kernel.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=GeCF+2rl/rDMBNfKZ38nI+LjiuxrKxc+sAIoZKPxTV0=; b=LdKV7lHB6vWJ5acbxhP82YXaVsPqKcSQTO+3WFvUqQmPdkQwBABX9Kcjb0OPROp60H s9BMxdr5M/uxFjvxduoLQcTIURTGIYzwM3but1Cj6EaI/p0HRPHDDO7dfbhvnxQENpr6 luVhVO2eBMyiRivXUwz8ANZNoo7yvpRTgIizVaG/2Bfn5eR9SaDyxUeAsE8ywNKQg58q nm50h6EJto6DPF5Qh/OVfG0M8yJUHI3aPsP7crFjpvuhikl8xFsLTmCeoktJO3lReCON zSG7Zsw3rtptbmWCmsU5A1QmXbfLWOEmdXcxRZoQnJTRoyyVvii4uPIZz14hYLcmU6f1 1YaA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1774444767; x=1775049567; h=in-reply-to:content-transfer-encoding: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=GeCF+2rl/rDMBNfKZ38nI+LjiuxrKxc+sAIoZKPxTV0=; b=ehg1eVb6KNkVD5G9q+GdshNyLh3hXLGDlkXUQvBKz2TEx9mZSPu6+zNgSkDW17WO8B KA1tl5R2Eeoy+gB5B5rHhk+9oQqQ0z+I6gdvJLCL9yjJdQ0csVvgbM4vpbsOzE9JpiNM 9TNwSXdcXnD3h4LObZCZgbFxEhcXlGwMbjUcwBhRCn2x8anZCGid+YpD61k1O8lZndFA LtJTkClLEUO8OeUCETLuMVh17r3iCKrJtjVnRg8eSGD0FkHD+9Lb7v50T59l4eEAM0lP tDkEOsaigGNB2rO/pDvW/HLXjl+liWDs0hweMJ23vAdhxGCbLFNbLVqGxftoh88XQRjg q3LQ== X-Forwarded-Encrypted: i=1; AJvYcCV3RFsVaZabs435kensi8VDIohFKa5yFVO1U9vmRR5yuYBn0zDMxcPrkh0wM+u1t08F09YkqVed6OVOpoI=@vger.kernel.org X-Gm-Message-State: AOJu0YxoDiP3I+MlRGAOXaTgvkJjwjHj5sFrCSSktdVwtnyTsVKystJc hEg5m9G2k9dtNayP2c12N2dDSLoEMxiGdsXFvV5ObGhApInhG+i2XQqasKIqi022qMA= X-Gm-Gg: ATEYQzzwM83s1r9l2qH+1H7CGjBm7e8ml7fXvpdFM9K7p4ZmjohD2ExVbMGSAhLGB9H K0yzC4Go0QoeGmpGueCBmmYrwXU1U8t8Fqne98PgiYs0X4JkS9cn/3zxhdg1Te9LsS9+5MULj09 Z7njmrUdCSfQZhuGAWMpJ8p11J9qKsZhiFeM7Ou86tbBKJeSa2aUNQ9jEd0hSmkiBGmUP4L6N8W vPqLh6koDkt8kFI5661pgYzF53AtAsRTxl93jmaHnOMN9c0obg73j8/IkZ5iv/fuMsfh8gSINzF 3hg07PamWB8RjVR8pQBju8PhIOpEdMud33gLa8Lp5QNCzMxXTzdRlVoRVmZHe6QdcfBvRDWLRO9 JUN12dxWrCXOdhL6C9s7AeMfvWEgzRSsNVIj7Jv15zwOZuHGOxThiqG2dmpIktxM2YYk5asLhR1 ADur3Bj24HtVp5KXF7MG8323/OAw== X-Received: by 2002:a05:600c:c177:b0:487:2e8:69c5 with SMTP id 5b1f17b1804b1-48715fe2aa7mr48642435e9.15.1774444767355; Wed, 25 Mar 2026 06:19:27 -0700 (PDT) Received: from pathway.suse.cz ([176.114.240.130]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-43b8e0afb63sm4439145f8f.30.2026.03.25.06.19.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 25 Mar 2026 06:19:26 -0700 (PDT) Date: Wed, 25 Mar 2026 14:19:25 +0100 From: Petr Mladek To: Song Liu Cc: Song Liu , linux-kernel@vger.kernel.org, tj@kernel.org, jiangshanlai@gmail.com, leitao@debian.org, kernel-team@meta.com, puranjay@kernel.org Subject: Re: [PATCH v2] workqueue: Fix false positive stall reports Message-ID: References: <20260322033045.3405807-1-song@kernel.org> 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-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: On Tue 2026-03-24 11:22:11, Song Liu wrote: > On Tue, Mar 24, 2026 at 3:01 AM Petr Mladek wrote: > [...] > > This explains why taking the lock is needed. > > > > > > > + Since > > > > > + * __queue_work() is a much hotter path than the timer > > > > > + * function, we handle false positive here by reading > > > > > + * last_progress_ts again with pool->lock held. > > > > But this is confusing. It says that __queue_work() is a much hotter path > > but it already takes pool->lock. The sentence makes a feeling that > > the watchdog patch is less hot. Then it is weird why the watchdog > > path ignores the lock by default. > > This comment primarily concerns the cost of making the read lockless. > To do that, we need to add a memory barrier in __queue_work(), > which will slow down the hot path. __queue_work() does take > pool->lock, but in most cases, __queue_work() only takes the lock > in local pool, which is faster than taking all the pool->locks from the > watchdog timer. > > That said, I am open to other suggestions to get rid of this false > positive. See below a patch which tries to improve the comment. Honestly, I am not sure if it is better than the original. It probably better answers the questions which I had. But it might open another questions. I feel that I have lost a detached view because I already know the details. Also it is hard to mention everything and keep it "short". Feel free to take it, ignore it, or squash it into the original commit. >From c65eeaac600f069c7f3398c87186f73f9c437735 Mon Sep 17 00:00:00 2001 From: Petr Mladek Date: Wed, 25 Mar 2026 13:34:18 +0100 Subject: [PATCH] workqueue: Better describe stall check Try to be more explicit why the workqueue watchdog does not take pool->lock by default. Spin locks are full memory barriers which delay anything. Obviously, they would primary delay operations on the related worker pools. Explain why it is enough to prevent the false positive by re-checking the timestamp under the pool->lock. Finally, make it clear what would be the alternative solution in __queue_work() which is a hotter path. Signed-off-by: Petr Mladek --- kernel/workqueue.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/kernel/workqueue.c b/kernel/workqueue.c index ff97b705f25e..e3e97d59d2f4 100644 --- a/kernel/workqueue.c +++ b/kernel/workqueue.c @@ -7702,13 +7702,13 @@ static void wq_watchdog_timer_fn(struct timer_list *unused) /* * Did we stall? * - * Do a lockless check first. On weakly ordered - * architectures, the lockless check can observe a - * reordering between worklist insert_work() and - * last_progress_ts update from __queue_work(). Since - * __queue_work() is a much hotter path than the timer - * function, we handle false positive here by reading - * last_progress_ts again with pool->lock held. + * Try to block or slow down queue_work() as less as possible + * because it might be used in hot paths. Just prevent false + * positives. + * + * Do a lockless check first. The spin_lock() makes sure + * that the re-check reads an updated pool->last_progress_ts + * when this CPU saw an already updated pool->worklist. */ if (time_after(now, ts + thresh)) { scoped_guard(raw_spinlock_irqsave, &pool->lock) { -- 2.53.0