From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (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 DA41F1FDA8E for ; Fri, 14 Nov 2025 13:31:29 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763127090; cv=none; b=buIv9N3VpzpvBTC25NrZgn8xduF89GHf1KqFteE9gTK4odR2z9uCWzxNTFOU4pUqQUWaVHYvJgXhyaUIJuZAxOjgupw3SkC/yLXDhe6kXv53bNbfURne9a1P2R5SSqUq3PUj2fupOsKPIVXxEI/6asoTviJ3oFrihMXtP3Qenzs= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763127090; c=relaxed/simple; bh=5kBTVongfTUoX9faG8XLHaDPy6n69DRleRbZuvHPlQA=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=guxQ2SRXBd410IM8zU9RKyU0hoBFk4XO/vJ03DPg1XEC0x8C3MlSWES+ZCzVazXhNKbViOiTbR08zPAolwkSKg2VRzOTKgHREpte3zMmn7ZUSJhYohTGC083kDpUS1NTZKfKDh2vcThajb4IpWK6oznERfpELc0sWC1pnMJjVIg= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=Lon/CDMD; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="Lon/CDMD" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 56EE0C4CEFB; Fri, 14 Nov 2025 13:31:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1763127089; bh=5kBTVongfTUoX9faG8XLHaDPy6n69DRleRbZuvHPlQA=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Lon/CDMDydP99iq+oLluJnQeu/Q3VejAYIbpXUkB9mg6mbIRE0QaeYpyE+JVZYMT9 uc4L0w2e2TNYmHlWPWZmL1qpZOgitpcURUk+r7ab5kSWV+H0NEj1fOWCm2bRaP/aST gxPe6AAWcggYSxpJdOD1o25grvF48ARg1SCL7SpYAMU54ixl6MKheqxQnh/hFI24jb OcNSfi+rvRN9BgCdDSz2G4+JdNVhztyJ8wMk7tyIwF+THMPN1Or01lljmmv5exSVA8 z7HCwWjur3mPYDN0IrWOBvl/3jnDmOdqyil1H+3JJpY+K5S7KQzpa3iyADZgNSDfqa yQNkzvNzn3cMw== Date: Fri, 14 Nov 2025 03:31:28 -1000 From: Tejun Heo To: Lai Jiangshan Cc: linux-kernel@vger.kernel.org, Lai Jiangshan , ying chen Subject: Re: [PATCH] workqueue: Process rescuer work items one-by-one using a positional marker Message-ID: References: <20251113163426.2950-1-jiangshanlai@gmail.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: Hello, On Fri, Nov 14, 2025 at 10:41:04AM +0800, Lai Jiangshan wrote: > > Also, I wonder whether it'd be simpler to think about if we just exclude the > > work item from flush color management. e.g. we can just flag the work item > > and then make work scanning skip them, so that they really are just markers; > > then, we don't have to worry about colors or othre states at all. We just > > insert the marker and use it purely as iteration marker. > > I think it should be processable by the normal workers, so that we don't > have to add special code in the fast path in worker_thread(), and it should work > like a normal work item or the wq_barrier. I don't know. I don't think a single unlikely() test is going to be noticeable. > The ability of processable by the normal workers has an additional benefit: > if it is processed by a normal worker rather than the rescuer, this > indicates that the pool has made forward progress by normal workers > and the pwq does not need to be rescued until the next mayday event. Is that correct tho? A worker thread executes any work item on the list and the pool may be shared by any number of workqueues, so the cursor can be cleared without any of the work items that belong to the rescuer's workqueue being executed, no? > It intended to update the positional work color to avoid back-to-back > work items to > stall the flush_workqueue(). > > But it did it wrong. Maybe this is better: > > if (get_work_color(*work_data_bits(&pwq->mayday_pos_work)) != pwq->work_color) { > { > remove_mayday_pos(pwq); > insert_mayday_pos(pwq, n); > } else { > list_move_tail(&pwq->mayday_pos_work.entry, &n->entry); > } The colored flush mechanism is already complex enough and difficult wrap one's head around. I don't really like adding more complications there. Thanks. -- tejun