From patchwork Mon Oct 12 02:09:42 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rob Clark X-Patchwork-Id: 11831433 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 74C1514D5 for ; Mon, 12 Oct 2020 02:09:48 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 56F022078E for ; Mon, 12 Oct 2020 02:09:48 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="KgxKD7H/" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726461AbgJLCJr (ORCPT ); Sun, 11 Oct 2020 22:09:47 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37210 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729795AbgJLCJj (ORCPT ); Sun, 11 Oct 2020 22:09:39 -0400 Received: from mail-pf1-x441.google.com (mail-pf1-x441.google.com [IPv6:2607:f8b0:4864:20::441]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 54842C0613D6; Sun, 11 Oct 2020 19:09:25 -0700 (PDT) Received: by mail-pf1-x441.google.com with SMTP id 144so12129617pfb.4; Sun, 11 Oct 2020 19:09:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=0Y1ZXL5tzeULxGQEQ98upbCTjOEGFjR0SJeNEjRygY4=; b=KgxKD7H/reOXnjxIspELqz3EGjOjlW01OL0qkzO8E8u33VbhpjUJjBGOiHgz8pF6fU iTyzaFLJGKjMd9MVlq0mmB1TZAu/ikMcN4P4SqAPpJmJL3YrB/p/hk5WPlvHa26OxXFs 9pcxJVqwc12BI2o0Z5GWP6KuBPSdv6Fm/yrfGu/sGLzyTsMwUtRrYO71fkVpm5PY8dUN 3/FdcnVea7ScXPEtqr+JUlN6ntlnD5tcCKKLr5m44pljDchWW5VA1pTZF/QSlGA3Sdqy DjWgM5x9TAZB27g6rdiYIPB8Wxgxdhv25viYt7hMh2sVRkIxo3Ae1ZRn41h+C0DattF6 7hDg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=0Y1ZXL5tzeULxGQEQ98upbCTjOEGFjR0SJeNEjRygY4=; b=qCtPTzCt6ntWBdtkVkUSG/7TnO++Ys3LSavgSogn42XO5CCe4mL2OE9iirC8W411ap ssbTSVbR9OLeAotAFERSrPYh3V+9AlvagtYTjP42nxuKPnZtoAWBgEtF+lzQzO4DHLp6 2Obkbrohup/DS0snXKXX4TMYWe00ROniHB7QSDSXwEjU6dCDNphPwD5nEywCkQ0fD2YF i/8pEm1P6PrYQWzKd17/Oe6MsC+7F0d1rYf8+oVzzuMAaJDrRQUfC7H5FMmPRzruPAvu sqTzCJMNb322E6F1FOVuc0H4EL3+OwY+zqDp2Ow5ZSMCyt8d50l4ay1kDeajX0EkTb0J djmQ== X-Gm-Message-State: AOAM533fe6iae7kaGSpR92HLTGWAA3rdUnM4vNt6OF4HSNFd1xoZvjCz 7P2lGAI5xBjJCr1h/IUBhyk= X-Google-Smtp-Source: ABdhPJwkHmYG3JLUCbaKXxgn2zEXUFi4lRRA+7075Pf3yqklwMpFZlrqr3qhkwPNZ3pPnR6GgPupew== X-Received: by 2002:a17:90b:e15:: with SMTP id ge21mr16365454pjb.188.1602468564802; Sun, 11 Oct 2020 19:09:24 -0700 (PDT) Received: from localhost (c-73-25-156-94.hsd1.or.comcast.net. [73.25.156.94]) by smtp.gmail.com with ESMTPSA id kv19sm21560902pjb.22.2020.10.11.19.09.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 11 Oct 2020 19:09:23 -0700 (PDT) From: Rob Clark To: dri-devel@lists.freedesktop.org Cc: Daniel Vetter , Rob Clark , Jordan Crouse , Rob Clark , Sean Paul , David Airlie , linux-arm-msm@vger.kernel.org (open list:DRM DRIVER FOR MSM ADRENO GPU), freedreno@lists.freedesktop.org (open list:DRM DRIVER FOR MSM ADRENO GPU), linux-kernel@vger.kernel.org (open list) Subject: [PATCH v2 15/22] drm/msm: Refcount submits Date: Sun, 11 Oct 2020 19:09:42 -0700 Message-Id: <20201012020958.229288-16-robdclark@gmail.com> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20201012020958.229288-1-robdclark@gmail.com> References: <20201012020958.229288-1-robdclark@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org From: Rob Clark Before we remove dev->struct_mutex from the retire path, we have to deal with the situation of a submit retiring before the submit ioctl returns. To deal with this, ring->submits will hold a reference to the submit, which is dropped when the submit is retired. And the submit ioctl path holds it's own ref, which it drops when it is done with the submit. Also, add to submit list *after* getting/pinning bo's, to prevent badness in case the completed fence is corrupted, and retire_worker mistakenly believes the submit is done too early. Signed-off-by: Rob Clark Reviewed-by: Jordan Crouse --- drivers/gpu/drm/msm/msm_drv.h | 1 - drivers/gpu/drm/msm/msm_gem.h | 13 +++++++++++++ drivers/gpu/drm/msm/msm_gem_submit.c | 11 +++++------ drivers/gpu/drm/msm/msm_gpu.c | 21 ++++++++++++++++----- 4 files changed, 34 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h index a17dadd38685..2ef5cff19883 100644 --- a/drivers/gpu/drm/msm/msm_drv.h +++ b/drivers/gpu/drm/msm/msm_drv.h @@ -277,7 +277,6 @@ void msm_unregister_mmu(struct drm_device *dev, struct msm_mmu *mmu); bool msm_use_mmu(struct drm_device *dev); -void msm_gem_submit_free(struct msm_gem_submit *submit); int msm_ioctl_gem_submit(struct drm_device *dev, void *data, struct drm_file *file); diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h index ec01f35ce57b..93ee73c620ed 100644 --- a/drivers/gpu/drm/msm/msm_gem.h +++ b/drivers/gpu/drm/msm/msm_gem.h @@ -211,6 +211,7 @@ void msm_gem_free_work(struct work_struct *work); * lasts for the duration of the submit-ioctl. */ struct msm_gem_submit { + struct kref ref; struct drm_device *dev; struct msm_gpu *gpu; struct msm_gem_address_space *aspace; @@ -247,6 +248,18 @@ struct msm_gem_submit { } bos[]; }; +void __msm_gem_submit_destroy(struct kref *kref); + +static inline void msm_gem_submit_get(struct msm_gem_submit *submit) +{ + kref_get(&submit->ref); +} + +static inline void msm_gem_submit_put(struct msm_gem_submit *submit) +{ + kref_put(&submit->ref, __msm_gem_submit_destroy); +} + /* helper to determine of a buffer in submit should be dumped, used for both * devcoredump and debugfs cmdstream dumping: */ diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c index a91c1b99db97..3151a0ca8904 100644 --- a/drivers/gpu/drm/msm/msm_gem_submit.c +++ b/drivers/gpu/drm/msm/msm_gem_submit.c @@ -42,6 +42,7 @@ static struct msm_gem_submit *submit_create(struct drm_device *dev, if (!submit) return NULL; + kref_init(&submit->ref); submit->dev = dev; submit->aspace = queue->ctx->aspace; submit->gpu = gpu; @@ -60,14 +61,13 @@ static struct msm_gem_submit *submit_create(struct drm_device *dev, return submit; } -void msm_gem_submit_free(struct msm_gem_submit *submit) +void __msm_gem_submit_destroy(struct kref *kref) { + struct msm_gem_submit *submit = + container_of(kref, struct msm_gem_submit, ref); unsigned i; dma_fence_put(submit->fence); - spin_lock(&submit->ring->submit_lock); - list_del(&submit->node); - spin_unlock(&submit->ring->submit_lock); put_pid(submit->pid); msm_submitqueue_put(submit->queue); @@ -841,8 +841,7 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data, submit_cleanup(submit); if (has_ww_ticket) ww_acquire_fini(&submit->ticket); - if (ret) - msm_gem_submit_free(submit); + msm_gem_submit_put(submit); out_unlock: if (ret && (out_fence_fd >= 0)) put_unused_fd(out_fence_fd); diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c index bb904e467b24..18a7948ac437 100644 --- a/drivers/gpu/drm/msm/msm_gpu.c +++ b/drivers/gpu/drm/msm/msm_gpu.c @@ -712,7 +712,12 @@ static void retire_submit(struct msm_gpu *gpu, struct msm_ringbuffer *ring, pm_runtime_mark_last_busy(&gpu->pdev->dev); pm_runtime_put_autosuspend(&gpu->pdev->dev); - msm_gem_submit_free(submit); + + spin_lock(&ring->submit_lock); + list_del(&submit->node); + spin_unlock(&ring->submit_lock); + + msm_gem_submit_put(submit); } static void retire_submits(struct msm_gpu *gpu) @@ -786,10 +791,6 @@ void msm_gpu_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit) submit->seqno = ++ring->seqno; - spin_lock(&ring->submit_lock); - list_add_tail(&submit->node, &ring->submits); - spin_unlock(&ring->submit_lock); - msm_rd_dump_submit(priv->rd, submit, NULL); update_sw_cntrs(gpu); @@ -816,6 +817,16 @@ void msm_gpu_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit) msm_gem_active_get(drm_obj, gpu); } + /* + * ring->submits holds a ref to the submit, to deal with the case + * that a submit completes before msm_ioctl_gem_submit() returns. + */ + msm_gem_submit_get(submit); + + spin_lock(&ring->submit_lock); + list_add_tail(&submit->node, &ring->submits); + spin_unlock(&ring->submit_lock); + gpu->funcs->submit(gpu, submit); priv->lastctx = submit->queue->ctx;