From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mout-p-102.mailbox.org (mout-p-102.mailbox.org [80.241.56.152]) (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 B9A7421ADB7 for ; Wed, 5 Nov 2025 08:39:58 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=80.241.56.152 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1762332000; cv=none; b=Svya2S2lA9wgPP3VXsOIG1ucpyxzvvRf7oo1yur+yBCBNs/EQaM15h61jqSEQb3JzhLZk2IrTB1katjnypDg9j0SqVpwdWogaXnar3ewAS0Xz1Rk1LzO4TNvqb82VVPeKH2GXpvmfmZMJIrrZnz5UlpR72uV5VO7LzM+8uDXlrQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1762332000; c=relaxed/simple; bh=R/qHXAEVOHE3d1dWBODBSjYaMxmDe0e2rTQH8K7PDFs=; h=Message-ID:Subject:From:To:Cc:Date:In-Reply-To:References: Content-Type:MIME-Version; b=foB9b7UvdxLJHvfoKxm8NxrUmTT8I5ZEv9QnPWdddlri/jdpvpSSV8nvkClceotlKlOohbB9EdeaoeU9wY71bMshCyLhuIlElg1QFw3jIwK8Tk7C/ipHECsQrBa8D7Bmksd1KbDu9il6CECRuQ7sMiLTn8XYx1TnLgBNsoovUMo= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=mailbox.org; spf=pass smtp.mailfrom=mailbox.org; dkim=pass (2048-bit key) header.d=mailbox.org header.i=@mailbox.org header.b=RKckWx8v; arc=none smtp.client-ip=80.241.56.152 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=mailbox.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=mailbox.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=mailbox.org header.i=@mailbox.org header.b="RKckWx8v" Received: from smtp102.mailbox.org (smtp102.mailbox.org [10.196.197.102]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mout-p-102.mailbox.org (Postfix) with ESMTPS id 4d1dzt5kTRz9vCR; Wed, 5 Nov 2025 09:39:54 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mailbox.org; s=mail20150812; t=1762331994; h=from:from:reply-to: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=yWwJvJiQhpOXo2m19sGkCSOjZ04ykFuMGX6vhFuj3tE=; b=RKckWx8vDdQTusdYmMivfYpC8pyNxVxxyRQaZJGr1jrSW/C3l0nzoM4XYcmW8Cy1J7rC5r HYRjERxE9IwNErXsSuZ6HYdm1VGQweiq4T+z5ezEk5NRT8GgPlMjeH/ESD2P6nNAkIhqjd wDCJYEAyMduhm1TPBf0SIV+qSECt8/H5bm1x/UiArma2hA+Eo3tvgJaAcE7/0hwRhg+VGH S82Y2vOPgnk+JxHKpJ8yc34csNtsmbj3HX3APcn9rYv9oDJKCgGITFE70aKz080LL/C4Au Uum9s4JpN6OR1jMthMT0zlERq48Mpt4MRjvHs993szUW2H68wM8ogY4WnwsF5A== Message-ID: <7808bc5fcac1236640f481733d1c8aaaf8accb02.camel@mailbox.org> Subject: Re: [PATCH] drm/sched: replace use of system_wq with system_percpu_wq From: Philipp Stanner Reply-To: phasta@kernel.org To: Marco Crivellari , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org Cc: Tejun Heo , Lai Jiangshan , Frederic Weisbecker , Sebastian Andrzej Siewior , Michal Hocko , Matthew Brost , Danilo Krummrich , Philipp Stanner , Christian Konig , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Simona Vetter Date: Wed, 05 Nov 2025 09:39:46 +0100 In-Reply-To: <20251104165209.309545-1-marco.crivellari@suse.com> References: <20251104165209.309545-1-marco.crivellari@suse.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-MBO-RS-META: 6798g4q11qanmdn67zdf7c735ufzcbt6 X-MBO-RS-ID: ca2565784354206ff6b nit: s/replace/Replace On Tue, 2025-11-04 at 17:52 +0100, Marco Crivellari wrote: > Currently if a user enqueue a work item using schedule_delayed_work() the s/enqueue/enqueues Also: maybe start the sentence with something like "In the general workqueue implementation, if a user [=E2=80=A6]". Otherwise it at first rea= ds as if we're talking about a drm/sched user here. In general, the commit message should focus more on drm/sched. See below, too. > used wq is "system_wq" (per-cpu wq) while queue_delayed_work() use > WORK_CPU_UNBOUND (used when a cpu is not specified). The same applies to > schedule_work() that is using system_wq and queue_work(), that makes use > again of WORK_CPU_UNBOUND. >=20 > This lack of consistentcy cannot be addressed without refactoring the API= . >=20 > This patch continues the effort to refactor worqueue APIs, which has begu= n > with the change introducing new workqueues and a new alloc_workqueue flag= : >=20 > commit 128ea9f6ccfb ("workqueue: Add system_percpu_wq and system_dfl_wq") > commit 930c2ea566af ("workqueue: Add new WQ_PERCPU flag") >From my POV it would be enough if you provide these commits in this commit message and maybe a Link: to the overall discussion in the workqueue subsystem / implementation. You can give the details above if you want, but I think drm/sched doesn't care too much about them. The drm/sched users who really care about the timeout_wq's exact behavior use one they allocate themselves anyways. >=20 > system_wq should be the per-cpu workqueue, yet in this name nothing makes > that clear, so replace system_wq with system_percpu_wq. >=20 > The old wq (system_wq) will be kept for a few release cycles. Please state in your commit message what you're actually doing to drm/sched. Like: "Use the successor of system_wq, system_percpu_wq, for the scheduler's default timeout_wq. system_wq will be removed in a few release cycles." >=20 > Suggested-by: Tejun Heo > Signed-off-by: Marco Crivellari > --- > =C2=A0drivers/gpu/drm/scheduler/sched_main.c | 2 +- > =C2=A01 file changed, 1 insertion(+), 1 deletion(-) >=20 > diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/sch= eduler/sched_main.c > index c39f0245e3a9..13192e99637a 100644 > --- a/drivers/gpu/drm/scheduler/sched_main.c > +++ b/drivers/gpu/drm/scheduler/sched_main.c > @@ -1315,7 +1315,7 @@ int drm_sched_init(struct drm_gpu_scheduler *sched,= const struct drm_sched_init_ > =C2=A0 sched->name =3D args->name; > =C2=A0 sched->timeout =3D args->timeout; > =C2=A0 sched->hang_limit =3D args->hang_limit; > - sched->timeout_wq =3D args->timeout_wq ? args->timeout_wq : system_wq; > + sched->timeout_wq =3D args->timeout_wq ? args->timeout_wq : system_perc= pu_wq; Actual change looks fine by me. Thanks for your patch, P. > =C2=A0 sched->score =3D args->score ? args->score : &sched->_score; > =C2=A0 sched->dev =3D args->dev; > =C2=A0