From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from galois.linutronix.de (Galois.linutronix.de [193.142.43.55]) (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 08C7D25B092 for ; Wed, 24 Jun 2026 08:48:00 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=193.142.43.55 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782290882; cv=none; b=krdZDZrGpEsnT5Rd7+YKW6CPpl4SvzBu6TVt2CEcOB7/FnCOYTEg6fQQAq+3gwBYOaRvtPtHKeo4njIDHxT2S1HTAMjPeqSOeVQ6QijTP4M7KlXzep2S+sdIuRJxN5Rn7GSL5GWOr9CxlyJuc3DHc+m6+yK4PWkEMH8daNFdBpA= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782290882; c=relaxed/simple; bh=brPWTF4CrUvYbDZCTQv+UVCKJV+JMGZd3vdjbkdyyUQ=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=bFiKpziwKis7uFMWDO/UY6y85JZJDlmiTYmUfSBjbSbnIW3rSBAasPGvxxqjCzNoZeb0N+oLhGRRtjzTkouYMSbYWyr7OzPIFlv+dsEUyfrV1YHW9WlWDiTHvhKj+SK5xuLIkiMh5LE3xEDW7ZzuoguFuuN2VjuBOpcl10J+lOQ= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linutronix.de; spf=pass smtp.mailfrom=linutronix.de; dkim=pass (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b=Zrzqi18V; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b=hz4atQMT; arc=none smtp.client-ip=193.142.43.55 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linutronix.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linutronix.de Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="Zrzqi18V"; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="hz4atQMT" Date: Wed, 24 Jun 2026 10:47:56 +0200 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1782290878; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=WnJiO/VQcJ/NHMqCCjmQ12YIHCZ6jZFbS+B+qkA20pg=; b=Zrzqi18V60xuOV7PYvzEogK4qWrOQ6QM5D6XqVNY2pFBQ1OBoUJGxVO4Mc+gAl3Lm0rq2P kG3wo/1UlhaOtsIW/I5buTi+gjIDuKA5YNSSrfpgGKpWAyc0MV/KzP6QaTc5hDhg0ddPlU YQCt0NbGpARJyw4e9UAzu2Bo3kqZEFhS+ZdFMXxlfXhs2gtkz4Uiw6GayKLq21J58QeRrW e7oQY37tvxZ55obyRNZwD0PWlCFPSz0U9S/flzYXx3C02/p321Y8UHdu5g1jQ5enEijDXR TSPFtj+rG4IbgTloZi81dRLVpmbG0C0uNroNyd3rgnw6PiC2AElIbwsCJFDB3A== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1782290878; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=WnJiO/VQcJ/NHMqCCjmQ12YIHCZ6jZFbS+B+qkA20pg=; b=hz4atQMTIpkv+5Tb4thjqazSJB6WOTZJz3bg0A3MO1Y3ZajNqOwQWXDsfVhhnFb0A2ueiX XuBHYz8XCPYPUNAg== From: Sebastian Andrzej Siewior To: Breno Leitao Cc: Tejun Heo , Lai Jiangshan , linux-kernel@vger.kernel.org, marco.crivellari@suse.com, frederic@kernel.org, Hillf Danton , kernel-team@meta.com, kmagar@redhat.com, psuriset@redhat.com Subject: Re: [PATCH v3 2/3] workqueue: defer the worker wakeup outside pool->lock in __queue_work() Message-ID: <20260624084756.X2i4QiPm@linutronix.de> References: <20260616-fastwake-v3-0-79da19fcd08f@debian.org> <20260616-fastwake-v3-2-79da19fcd08f@debian.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: quoted-printable In-Reply-To: <20260616-fastwake-v3-2-79da19fcd08f@debian.org> On 2026-06-16 06:33:32 [-0700], Breno Leitao wrote: > __queue_work() is the enqueue hot path: it inserts the work item and > calls kick_pool() while holding pool->lock. kick_pool() ends in a > wakeup, which takes the target task's rq->lock, so rq->lock nests under > pool->lock on every enqueue that wakes a worker on a contended unbound > pool. >=20 > Use kick_pool_pick() to select and claim the worker under pool->lock, > queue it on an on-stack wake_q, and issue the wakeup with wake_up_q() > right after dropping the lock via raw_spin_unlock_wake(). Worker > selection, wake_cpu setup and claiming the worker off pool->idle_list > still happen under the lock; only the rq->lock acquisition moves out. >=20 > Signed-off-by: Breno Leitao =E2=80=A6 > --- a/kernel/workqueue.c > +++ b/kernel/workqueue.c =E2=80=A6 > out: > - raw_spin_unlock(&pool->lock); > + raw_spin_unlock_wake(&pool->lock, &wakeq); > rcu_read_unlock(); > } It is not wrong but I am not sure if this is really needed here. The pattern preempt_disable(); raw_spin_unlock(); wake_up_q() preempt_enable(); is used to prevent task preemption after the unlock operation. The futex/ locking code needs to wake a task but before the unlock operation the task priority might have been lowered as result of dropping the lock. This means it might not be the task with the highest priority in the system and the task with highest is not yet active and we schedule a task in middle in the instead. To form this easier: say we have Task A prio=3D1, B prio=3D2 and C prio=3D3 with higher number higher priority.=20 A owns a lock and is preempted by B. C gets on the CPU preempts B. C wants A's lock so it passes its priority to A (PI-boost) and goes idle. A gets on the CPU and unlocks. Now: As part of the unlock operation (before the raw_spin_unlock()) A goes back to its initial priority and C is not yet woken up meaning B is the task with highest priority and the held raw_spinlock_t is the only thing preventing scheduling. So we disable preemption, unlock and then perform the wake-up so C becomes the next candidate (as it should be) to occupy the CPU. Otherwise it would be B which means per definition a task with lower priority runs before a task with higher priority. I don't think workqueue has this requirements here. Worst case something else gets on the CPU and worker wakeup is delayed until the task is scheduled again. It could be fine since the preemption could happen before queue-work(). It probably does not lead to a huge performance regression but who knows. Your goal was to lower the contention on the pool lock during the wake up which you achieved. The __queue_work() remains still not preemptible until after the wake up which might be fine. Sebastian