From mboxrd@z Thu Jan 1 00:00:00 1970 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 7A839CF9C6B for ; Tue, 24 Sep 2024 18:44:40 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 225DC10E732; Tue, 24 Sep 2024 18:44:40 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="mpDj06IS"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.13]) by gabe.freedesktop.org (Postfix) with ESMTPS id E807110E732 for ; Tue, 24 Sep 2024 18:44:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1727203478; x=1758739478; h=from:to:cc:subject:date:message-id:mime-version: content-transfer-encoding; bh=439/0P77tQqEdVoBa5tBh48V1UhjDYn5NS0Th39Dwz4=; b=mpDj06IST4cL76C+15CuOX9Z52wBElm2YIfrcoiwfJohEP3TE86QcGO4 y1I3yJLMdaw5du2mM9J1xIlwxUfNiCDngP7xUpYYwRcvTl3rUQmGdANn0 Lzzgn/SJFSkZ+j2/g7mJWh6vytgRiQaKerdVyFyA9EBxBP5ClgW+qfjNX 4eUsPqy535mjfkg8wlm8jfqU6fu5OvBE3GUljN7BKsCje0TGKXi21cTNQ 6NPXXSMyFS8Tr4qjgh4meQB1DPqkl1eCvMTEPrVyga3lCR4BhGyyc2aq5 F9k11dhyqT59joq16c/+LAoc1pcB/8RysQVb7HQGVuNhRfoDlH1rKmlql w==; X-CSE-ConnectionGUID: o8yrJXiRSfet+7mK3GDWfg== X-CSE-MsgGUID: DciBR/zrQxeRl3r1fWZZow== X-IronPort-AV: E=McAfee;i="6700,10204,11205"; a="29109399" X-IronPort-AV: E=Sophos;i="6.10,255,1719903600"; d="scan'208";a="29109399" Received: from orviesa001.jf.intel.com ([10.64.159.141]) by fmvoesa107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 24 Sep 2024 11:44:37 -0700 X-CSE-ConnectionGUID: h5LDDrHjSJWO1JiCJw7s/A== X-CSE-MsgGUID: lx/uqySVTqWudLuhY2qKOQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.10,255,1719903600"; d="scan'208";a="108969679" Received: from lstrano-desk.jf.intel.com ([10.54.39.91]) by smtpauth.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 24 Sep 2024 11:44:37 -0700 From: Matthew Brost To: intel-xe@lists.freedesktop.org Cc: matthew.auld@intel.com Subject: [PATCH v2] drm/xe: Take ref to job and job's fence in xe_sched_job_arm Date: Tue, 24 Sep 2024 11:45:41 -0700 Message-Id: <20240924184541.2992459-1-matthew.brost@intel.com> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-BeenThere: intel-xe@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel Xe graphics driver List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-xe-bounces@lists.freedesktop.org Sender: "Intel-xe" Fixes two possible races: - Submission to hardware signals job's fence before dma_fence_get at end of run_job - TDR fires and signals fence + free job before run_job completes Taking refs in xe_sched_job_arm to job and job's fence solves these by ensure all refs collected before entering the DRM scheduler. The refs are dropped in run_job and DRM scheduler respectfully. Safe as once xe_sched_job_arm is called execution of job through DRM sched is guaranteed. v2: - Take job ref on resubmit (Matt Auld) Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/2811 Signed-off-by: Matthew Brost Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel GPUs") Cc: Matthew Auld Cc: # v6.8+ --- drivers/gpu/drm/xe/xe_execlist.c | 4 +++- drivers/gpu/drm/xe/xe_gpu_scheduler.c | 17 +++++++++++++++++ drivers/gpu/drm/xe/xe_gpu_scheduler.h | 6 +----- drivers/gpu/drm/xe/xe_guc_submit.c | 11 +++++++---- drivers/gpu/drm/xe/xe_sched_job.c | 5 ++--- drivers/gpu/drm/xe/xe_sched_job_types.h | 1 - 6 files changed, 30 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_execlist.c b/drivers/gpu/drm/xe/xe_execlist.c index f3b71fe7a96d..b70706c9caf2 100644 --- a/drivers/gpu/drm/xe/xe_execlist.c +++ b/drivers/gpu/drm/xe/xe_execlist.c @@ -309,11 +309,13 @@ execlist_run_job(struct drm_sched_job *drm_job) struct xe_sched_job *job = to_xe_sched_job(drm_job); struct xe_exec_queue *q = job->q; struct xe_execlist_exec_queue *exl = job->q->execlist; + struct dma_fence *fence = job->fence; q->ring_ops->emit_job(job); xe_execlist_make_active(exl); + xe_sched_job_put(job); - return dma_fence_get(job->fence); + return fence; } static void execlist_job_free(struct drm_sched_job *drm_job) diff --git a/drivers/gpu/drm/xe/xe_gpu_scheduler.c b/drivers/gpu/drm/xe/xe_gpu_scheduler.c index c518d1d16d82..7ea0c8e9e7a9 100644 --- a/drivers/gpu/drm/xe/xe_gpu_scheduler.c +++ b/drivers/gpu/drm/xe/xe_gpu_scheduler.c @@ -4,6 +4,7 @@ */ #include "xe_gpu_scheduler.h" +#include "xe_sched_job.h" static void xe_sched_process_msg_queue(struct xe_gpu_scheduler *sched) { @@ -106,3 +107,19 @@ void xe_sched_add_msg_locked(struct xe_gpu_scheduler *sched, list_add_tail(&msg->link, &sched->msgs); xe_sched_process_msg_queue(sched); } + +/** + * xe_sched_resubmit_jobs() - Resubmit scheduler jobs + * @sched: Xe GPU scheduler + * + * Take a ref all jobs on scheduler and resubmit. + */ +void xe_sched_resubmit_jobs(struct xe_gpu_scheduler *sched) +{ + struct drm_sched_job *s_job; + + list_for_each_entry(s_job, &sched->base.pending_list, list) + xe_sched_job_get(to_xe_sched_job(s_job)); /* Paired with put in run_job */ + + drm_sched_resubmit_jobs(&sched->base); +} diff --git a/drivers/gpu/drm/xe/xe_gpu_scheduler.h b/drivers/gpu/drm/xe/xe_gpu_scheduler.h index cee9c6809fc0..ecbe5dd6664e 100644 --- a/drivers/gpu/drm/xe/xe_gpu_scheduler.h +++ b/drivers/gpu/drm/xe/xe_gpu_scheduler.h @@ -26,6 +26,7 @@ void xe_sched_add_msg(struct xe_gpu_scheduler *sched, struct xe_sched_msg *msg); void xe_sched_add_msg_locked(struct xe_gpu_scheduler *sched, struct xe_sched_msg *msg); +void xe_sched_resubmit_jobs(struct xe_gpu_scheduler *sched); static inline void xe_sched_msg_lock(struct xe_gpu_scheduler *sched) { @@ -47,11 +48,6 @@ static inline void xe_sched_tdr_queue_imm(struct xe_gpu_scheduler *sched) drm_sched_tdr_queue_imm(&sched->base); } -static inline void xe_sched_resubmit_jobs(struct xe_gpu_scheduler *sched) -{ - drm_sched_resubmit_jobs(&sched->base); -} - static inline bool xe_sched_invalidate_job(struct xe_sched_job *job, int threshold) { diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c index fbbe6a487bbb..689279fdef80 100644 --- a/drivers/gpu/drm/xe/xe_guc_submit.c +++ b/drivers/gpu/drm/xe/xe_guc_submit.c @@ -766,6 +766,7 @@ guc_exec_queue_run_job(struct drm_sched_job *drm_job) struct xe_guc *guc = exec_queue_to_guc(q); struct xe_device *xe = guc_to_xe(guc); bool lr = xe_exec_queue_is_lr(q); + struct dma_fence *fence = NULL; xe_assert(xe, !(exec_queue_destroyed(q) || exec_queue_pending_disable(q)) || exec_queue_banned(q) || exec_queue_suspended(q)); @@ -782,12 +783,14 @@ guc_exec_queue_run_job(struct drm_sched_job *drm_job) if (lr) { xe_sched_job_set_error(job, -EOPNOTSUPP); - return NULL; - } else if (test_and_set_bit(JOB_FLAG_SUBMIT, &job->fence->flags)) { - return job->fence; + dma_fence_put(job->fence); /* Drop ref from xe_sched_job_arm */ } else { - return dma_fence_get(job->fence); + fence = job->fence; } + + xe_sched_job_put(job); /* Pairs with get from xe_sched_job_arm */ + + return fence; } static void guc_exec_queue_free_job(struct drm_sched_job *drm_job) diff --git a/drivers/gpu/drm/xe/xe_sched_job.c b/drivers/gpu/drm/xe/xe_sched_job.c index eeccc1c318ae..d0f4b908411f 100644 --- a/drivers/gpu/drm/xe/xe_sched_job.c +++ b/drivers/gpu/drm/xe/xe_sched_job.c @@ -280,16 +280,15 @@ void xe_sched_job_arm(struct xe_sched_job *job) fence = &chain->base; } - job->fence = fence; + xe_sched_job_get(job); /* Pairs with put in run_job */ + job->fence = dma_fence_get(fence); /* Pairs with put in scheduler */ drm_sched_job_arm(&job->drm); } void xe_sched_job_push(struct xe_sched_job *job) { - xe_sched_job_get(job); trace_xe_sched_job_exec(job); drm_sched_entity_push_job(&job->drm); - xe_sched_job_put(job); } /** diff --git a/drivers/gpu/drm/xe/xe_sched_job_types.h b/drivers/gpu/drm/xe/xe_sched_job_types.h index 0d3f76fb05ce..8ed95e1a378f 100644 --- a/drivers/gpu/drm/xe/xe_sched_job_types.h +++ b/drivers/gpu/drm/xe/xe_sched_job_types.h @@ -40,7 +40,6 @@ struct xe_sched_job { * @fence: dma fence to indicate completion. 1 way relationship - job * can safely reference fence, fence cannot safely reference job. */ -#define JOB_FLAG_SUBMIT DMA_FENCE_FLAG_USER_BITS struct dma_fence *fence; /** @user_fence: write back value when BB is complete */ struct { -- 2.34.1