From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f44.google.com (mail-wr1-f44.google.com [209.85.221.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 7808A21D00A for ; Mon, 23 Mar 2026 14:09:20 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.44 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774274962; cv=none; b=ot1uugxHwXlO0RWVwpbsdW5dAUMx0oR/NWYm3FV/anbAaXPcz7Zu52ltsFAUE3sltE73O6n/mWhRenAQwtwQM4SPXSHg174TeDOBmGk+zjCaJIuTJXcm22SF7NwbiCnuMofp+u9sIq/TmrPDoqQyHpg8aLDWdagStxSh+K3vGbc= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774274962; c=relaxed/simple; bh=KV9jHNbYgnbMDzXjDzOjCe6QSsi5WMCk51JL6ZYBE3E=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=cqjflRiDb6Ap9G43QKnikCY4M7o5OpDYgRk6IX075o9QyzROJbvC+NjqjpeSVoFY58zfJCg34dDKEamueiJkihYmdOWkOgnx9hoG3doKn8wrB0bhYjc6y0hUqe+Vow8ZFY4XxlbjSiAQa5Gmggte+27ktSMqU7kpDeLC8gzvOk8= 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=bvJfLuIu; arc=none smtp.client-ip=209.85.221.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="bvJfLuIu" Received: by mail-wr1-f44.google.com with SMTP id ffacd0b85a97d-439b9b1900bso2040233f8f.1 for ; Mon, 23 Mar 2026 07:09:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=google; t=1774274959; x=1774879759; 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=b4//hX/DmZxS+qzQgli6f6UbcfIP/Us5GP0uWl4rYcg=; b=bvJfLuIueJWoCMnaXTJU/6mT/bWOJZYIqOpmUJZE5itXdZVL0DHyZvcetBGXoS4U+Q Yz0Uqe2A1xMlL+Yu3dVu/FPTk6NZk3Fl9OpH3T0dLHDoMlxkVt8Yeu66haQ9Woh8ggvq 2ayh77QAHJUI+QQ+fG+dCsMkdQdfLqCrzI5WuKwsJzdMH89s25MpRFRFk+vkm1dboQA6 YXFEAxqZxTEndF/jWdD1+3Dv31MWiKp2S5QF8Uwf2k5hvs1HosfjHV1UfXkHuHyChJzg Yoh92iRHh0pIsmmeOJRghT3TGUsVoCb0sfnyK1ImQRh+62xgcbhEy4i7w/2jJDxqMzvC dyhg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1774274959; x=1774879759; 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=b4//hX/DmZxS+qzQgli6f6UbcfIP/Us5GP0uWl4rYcg=; b=RoW7PaE4qIDv49U1dKHdDaQ5tgTMIfGReqrU2P/PUPUxX20ykHlCqik/Zfbi7+Sbej 1GAe2m9m7/S86/cDqvKvt/9Vmqvu/Q3B71CSQv1w8nMOVgnPRw1VQPID7TSbpcIc+fPV YRSG63TfjzgOWkUEJXskAAe8Wk3HxpXHz2LtOztD03PBEhsSd1rVK2T1C3veQD1K5PHs S1VjyZk1Fy9+aLPN1w8nhOkconUdraWaz2P5Fv8molGxPxoiEDTFo3GvX4+8lJRFOFYt 1v1jTgGdlp01aj22wsNyAKPnq2TJtkh3qc6hok+JxyDf8mEudDL+KoL0HQIrlpXAaU28 khZw== X-Gm-Message-State: AOJu0YymYmqkCm3XA/Z55jMRWxYwW5IY5Fi8X7zlr8CexlN2/ljeMJbX Yfe/RfU+7FVMjdeXlu0gpWXllLFfAqIikt3w/bT8RCYVj9RgPUPWIElwWcOr+9v4j9o= X-Gm-Gg: ATEYQzwRpN2N4Pq+Xi8kYh7pAvRUjwcoPPhSeBQ5bG4V2ecwDs+WVxDtY9BfikRNXUA 8yZL+rAEP+HEVro0P2ECsZA5DEdSTj2tTyLk0HXo1cSbgt1OEwqv8WS9kotFqHQPHFloAZ/gAqG ePObeVzRRX7Vglrl5+uAWFvhJRBMi4okzL5ygpAfco6JcgvIlOyPW1j1Xva9gdx1Lc1o/JCVhdF dRqJO0W1SkGSinXC15yf1AaBo9q7mqJ0cgPsFxxoBwiJAUcGtLSnSm1tpr1srGwSK85Ypc5iZKr u/O8WrFJZHGFFKIbn/yfzL8s8T94lzqroEOQO92SDshbG0CG8kXEFjJ9b3Jmxh1Xns5DugJHP5M BVP/VlsGkB5ELeO9ZLATG1VbUnQTfVW1rFBUe0odHIKLx33a9uIXfBCWV6BR2v+hOT66/PyBPpM sCNsj26lc55CTKa2WY2BI+DZngZg== X-Received: by 2002:a05:6000:310b:b0:439:f605:afde with SMTP id ffacd0b85a97d-43b6428ae11mr19217962f8f.51.1774274958623; Mon, 23 Mar 2026 07:09:18 -0700 (PDT) Received: from pathway.suse.cz ([176.114.240.130]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-43b644bd0dcsm28785717f8f.11.2026.03.23.07.09.17 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 23 Mar 2026 07:09:18 -0700 (PDT) Date: Mon, 23 Mar 2026 15:09:16 +0100 From: Petr Mladek To: Song Liu Cc: 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=us-ascii Content-Disposition: inline In-Reply-To: <20260322033045.3405807-1-song@kernel.org> On Sat 2026-03-21 20:30:45, Song Liu wrote: > On weakly ordered architectures (e.g., arm64), the lockless check in > wq_watchdog_timer_fn() can observe a reordering between the worklist > insertion and the last_progress_ts update. Specifically, the watchdog > can see a non-empty worklist (from a list_add) while reading a stale > last_progress_ts value, causing a false positive stall report. > > This was confirmed by reading pool->last_progress_ts again after holding > pool->lock in wq_watchdog_timer_fn(): > > workqueue watchdog: pool 7 false positive detected! > lockless_ts=4784580465 locked_ts=4785033728 > diff=453263ms worklist_empty=0 > > To avoid slowing down the hot path (queue_work, etc.), recheck > last_progress_ts with pool->lock held. This will eliminate the false > positive with minimal overhead. > > Remove two extra empty lines in wq_watchdog_timer_fn() as we are on it. > > --- a/kernel/workqueue.c > +++ b/kernel/workqueue.c > @@ -7699,8 +7699,28 @@ static void wq_watchdog_timer_fn(struct timer_list *unused) > else > ts = touched; > > - /* did we stall? */ > + /* > + * 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. > + */ > if (time_after(now, ts + thresh)) { > + scoped_guard(raw_spinlock_irqsave, &pool->lock) { > + pool_ts = pool->last_progress_ts; > + if (time_after(pool_ts, touched)) > + ts = pool_ts; > + else > + ts = touched; > + } > + if (!time_after(now, ts + thresh)) > + continue; The new code is pretty hairy. It might make sense to take the lock around the original check and keep it as is. IMHO, if a contention on a pool->lock has ever been a problem than maybe the non-trivial workqueue API was not a good choice for the affected use-case. Or am I wrong? Best Regards, Petr > + > lockup_detected = true; > stall_time = jiffies_to_msecs(now - pool_ts) / 1000; > max_stall_time = max(max_stall_time, stall_time);