From patchwork Wed Mar 5 13:05:51 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Philipp Stanner X-Patchwork-Id: 14002601 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id C839BC19F32 for ; Wed, 5 Mar 2025 13:06:10 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 514A310E728; Wed, 5 Mar 2025 13:06:10 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=kernel.org header.i=@kernel.org header.b="ZUJBL0wN"; dkim-atps=neutral Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by gabe.freedesktop.org (Postfix) with ESMTPS id 1BFA510E297 for ; Wed, 5 Mar 2025 13:06:09 +0000 (UTC) Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id B34B25C68ED; Wed, 5 Mar 2025 13:03:51 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id B8A8DC4CEE2; Wed, 5 Mar 2025 13:06:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1741179968; bh=VnltsXK2yo0tjAFJEBLC6Hm+o1kgcKTB55+828iZ6bQ=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=ZUJBL0wNT4YmROIrj4Fp2+MnlOAoyUpnAWfKf9lhouge7LMBwpm+iS5PadAjKA6M5 07c+5QCfZ5Gd3AUd+wKzRars0LItrkJdLkeVSWHR5Ns6U2dXS6KU9Taj4wGGot14VQ QQesttSeKg1Jt+YIhxYNeCMstaWM1bcuL3tpb0JLC9+bJPOek37+e0ANE6brNx3L0O Xuz+0aaRsrCvY6gow7pQN37FRbmDqrdMRmMPWjPTVGOcBMvHT8XyyzrInN4UKBYo28 MP5RojZv3a78c8uYwJu7/5Pv/MaXF3mAFVpdXu1CPEtsER4UjT7DpCodoZjjM/28ed 1lh/KEKbMMCVg== From: Philipp Stanner To: Matthew Brost , Danilo Krummrich , Philipp Stanner , =?utf-8?q?Christian_K=C3=B6nig?= , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Simona Vetter , Sumit Semwal Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Philipp Stanner Subject: [PATCH v7 2/3] drm/sched: Document run_job() refcount hazard Date: Wed, 5 Mar 2025 14:05:51 +0100 Message-ID: <20250305130551.136682-4-phasta@kernel.org> X-Mailer: git-send-email 2.48.1 In-Reply-To: <20250305130551.136682-2-phasta@kernel.org> References: <20250305130551.136682-2-phasta@kernel.org> MIME-Version: 1.0 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" From: Philipp Stanner drm_sched_backend_ops.run_job() returns a dma_fence for the scheduler. That fence is signalled by the driver once the hardware completed the associated job. The scheduler does not increment the reference count on that fence, but implicitly expects to inherit this fence from run_job(). This is relatively subtle and prone to misunderstandings. This implies that, to keep a reference for itself, a driver needs to call dma_fence_get() in addition to dma_fence_init() in that callback. It's further complicated by the fact that the scheduler even decrements the refcount in drm_sched_run_job_work() since it created a new reference in drm_sched_fence_scheduled(). It does, however, still use its pointer to the fence after calling dma_fence_put() - which is safe because of the aforementioned new reference, but actually still violates the refcounting rules. Move the call to dma_fence_put() to the position behind the last usage of the fence. Suggested-by: Danilo Krummrich Signed-off-by: Philipp Stanner Reviewed-by: Danilo Krummrich --- drivers/gpu/drm/scheduler/sched_main.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c index c634993f1346..c8da6b4edf0a 100644 --- a/drivers/gpu/drm/scheduler/sched_main.c +++ b/drivers/gpu/drm/scheduler/sched_main.c @@ -1216,20 +1216,23 @@ static void drm_sched_run_job_work(struct work_struct *w) drm_sched_job_begin(sched_job); trace_drm_run_job(sched_job, entity); + /* + * The run_job() callback must by definition return a fence whose + * refcount has been incremented for the scheduler already. + */ fence = sched->ops->run_job(sched_job); complete_all(&entity->entity_idle); drm_sched_fence_scheduled(s_fence, fence); if (!IS_ERR_OR_NULL(fence)) { - /* Drop for original kref_init of the fence */ - dma_fence_put(fence); - r = dma_fence_add_callback(fence, &sched_job->cb, drm_sched_job_done_cb); if (r == -ENOENT) drm_sched_job_done(sched_job, fence->error); else if (r) DRM_DEV_ERROR(sched->dev, "fence add callback failed (%d)\n", r); + + dma_fence_put(fence); } else { drm_sched_job_done(sched_job, IS_ERR(fence) ? PTR_ERR(fence) : 0);