From patchwork Sat Nov 14 19:30:08 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rob Clark X-Patchwork-Id: 11905781 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 33D7D697 for ; Sat, 14 Nov 2020 19:28:52 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 1A7642225E for ; Sat, 14 Nov 2020 19:28:52 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="qTrp/L0r" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726290AbgKNT2d (ORCPT ); Sat, 14 Nov 2020 14:28:33 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45252 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726112AbgKNT2d (ORCPT ); Sat, 14 Nov 2020 14:28:33 -0500 Received: from mail-pg1-x544.google.com (mail-pg1-x544.google.com [IPv6:2607:f8b0:4864:20::544]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 56253C0613D1; Sat, 14 Nov 2020 11:28:33 -0800 (PST) Received: by mail-pg1-x544.google.com with SMTP id f18so9506244pgi.8; Sat, 14 Nov 2020 11:28:33 -0800 (PST) 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=CjOmYSFFYsasYX9Sq8MuBrH7p/CFaq4+YCq5TRyIrqw=; b=qTrp/L0r7cSYg6nAdz7tU34ktyvrLgWsW2SE+YBAtmUwyV0JywqFQjiWNXtg5IrVS5 btrrG/lLaqrPeMS236r5oyMKtkOl+OERxtctWe3b5eLsjKDf468UKvgiIGG1VfE96ZVR 5rhiPmTd4I7ZuCr9bDxfhAzbHh9GQOJ7hTluyMB/XXk1NeHw0ltyvhWaQwBHvAnlqjta GZcw5uywbJDz8FAI6KZyaN3sNWaecBYFNhoW1ZN0NbI56Ck3dbCzIZbamxmqmUAN8Dcw +nsGlU9cOKjL7GeAhhoCmbIG8Xmoh81j7I6BT58FEWGm2rnJWTCJilSDDhnm4mhI+2Wv KxbA== 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=CjOmYSFFYsasYX9Sq8MuBrH7p/CFaq4+YCq5TRyIrqw=; b=EytLzA7+Iy6B0WrReowOAis4UegRNtLy6q8xxqKwi5oCknSGcatZZNEDMXICzGpKKB vTw8Aq6K7/kLAT6pOna+cVCdzeB4q6nR7JlCRWbVmHJPFUImdhAbPW7LAG3NK2GcN98/ ScfToGlgyzU9Pebad+tIFbhY6QJFbB9/husM96zDq19Flj/bnfyS8pmXr+z/DRe/IBRj L0bvbmekKbZPwe/slmys63kiN/yV7rbcLOHm2aaFPguIKTZD75NLRZgar+Pnwcfhg9Jj aDLgvDPG2Ykq1JPDCk/a71Ue2cfwim+i1KzYVcECwQJ6N7pStOmvBgdRiiA3SDW3xDFi /E3A== X-Gm-Message-State: AOAM532axNfez8DRsc/wouY3EaeLajf1TGzSEGS5eXEmJdm/26wT/kBG 43Wo3pCvoggujeCf5lETaW4= X-Google-Smtp-Source: ABdhPJyJfQEYepL8PyuaRVxEahLC2pXCHSM5KwJiRwsI2EOamwMmL1JTjjnfJqk9Rk6ERJK6Kijmgg== X-Received: by 2002:a63:3150:: with SMTP id x77mr652816pgx.114.1605382112892; Sat, 14 Nov 2020 11:28:32 -0800 (PST) Received: from localhost (c-73-25-156-94.hsd1.or.comcast.net. [73.25.156.94]) by smtp.gmail.com with ESMTPSA id g8sm2891778pgn.47.2020.11.14.11.28.31 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 14 Nov 2020 11:28:31 -0800 (PST) From: Rob Clark To: dri-devel@lists.freedesktop.org Cc: Rob Clark , Rob Clark , Sean Paul , David Airlie , Daniel Vetter , "Kristian H. Kristensen" , Jordan Crouse , 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 1/3] drm/msm: Protect obj->active_count under obj lock Date: Sat, 14 Nov 2020 11:30:08 -0800 Message-Id: <20201114193010.753355-2-robdclark@gmail.com> X-Mailer: git-send-email 2.28.0 In-Reply-To: <20201114193010.753355-1-robdclark@gmail.com> References: <20201114193010.753355-1-robdclark@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org From: Rob Clark Previously we only held obj lock in the _active_get() path, and relied on atomic_dec_return() to not be racy in the _active_put() path where obj lock was not held. But this is a false sense of security. Unlike obj lifetime refcnt, where you do not expect to *increase* the refcnt after the last put (which would mean that something has gone horribly wrong with the object liveness reference counting), the active_count can increase again from zero. Racing _active_put()s and _active_get()s could leave the obj on the wrong mm list. But in the retire path, immediately after the _active_put(), the _unpin_iova() would acquire obj lock. So just move the locking earlier and rely on that to protect obj->active_count. Fixes: c5c1643cef7a ("drm/msm: Drop struct_mutex from the retire path") Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gem.c | 11 ++++++----- drivers/gpu/drm/msm/msm_gem.h | 5 +++-- drivers/gpu/drm/msm/msm_gpu.c | 10 ++++++---- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c index a9a834bb7794..2795288b0a95 100644 --- a/drivers/gpu/drm/msm/msm_gem.c +++ b/drivers/gpu/drm/msm/msm_gem.c @@ -770,7 +770,7 @@ void msm_gem_active_get(struct drm_gem_object *obj, struct msm_gpu *gpu) WARN_ON(!msm_gem_is_locked(obj)); WARN_ON(msm_obj->madv != MSM_MADV_WILLNEED); - if (!atomic_fetch_inc(&msm_obj->active_count)) { + if (msm_obj->active_count++ == 0) { mutex_lock(&priv->mm_lock); list_del_init(&msm_obj->mm_list); list_add_tail(&msm_obj->mm_list, &gpu->active_list); @@ -784,8 +784,9 @@ void msm_gem_active_put(struct drm_gem_object *obj) struct msm_drm_private *priv = obj->dev->dev_private; might_sleep(); + WARN_ON(!msm_gem_is_locked(obj)); - if (!atomic_dec_return(&msm_obj->active_count)) { + if (--msm_obj->active_count == 0) { mutex_lock(&priv->mm_lock); list_del_init(&msm_obj->mm_list); list_add_tail(&msm_obj->mm_list, &priv->inactive_list); @@ -936,15 +937,15 @@ void msm_gem_free_object(struct drm_gem_object *obj) struct drm_device *dev = obj->dev; struct msm_drm_private *priv = dev->dev_private; - /* object should not be on active list: */ - WARN_ON(is_active(msm_obj)); - mutex_lock(&priv->mm_lock); list_del(&msm_obj->mm_list); mutex_unlock(&priv->mm_lock); msm_gem_lock(obj); + /* object should not be on active list: */ + WARN_ON(is_active(msm_obj)); + put_iova(obj); if (obj->import_attach) { diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h index d79e7019cc88..3355a48a023b 100644 --- a/drivers/gpu/drm/msm/msm_gem.h +++ b/drivers/gpu/drm/msm/msm_gem.h @@ -87,7 +87,7 @@ struct msm_gem_object { char name[32]; /* Identifier to print for the debugfs files */ - atomic_t active_count; + int active_count; }; #define to_msm_bo(x) container_of(x, struct msm_gem_object, base) @@ -185,7 +185,8 @@ msm_gem_is_locked(struct drm_gem_object *obj) static inline bool is_active(struct msm_gem_object *msm_obj) { - return atomic_read(&msm_obj->active_count); + WARN_ON(!msm_gem_is_locked(&msm_obj->base)); + return msm_obj->active_count; } static inline bool is_purgeable(struct msm_gem_object *msm_obj) diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c index b597213a1890..04f7ab4d63ae 100644 --- a/drivers/gpu/drm/msm/msm_gpu.c +++ b/drivers/gpu/drm/msm/msm_gpu.c @@ -717,11 +717,13 @@ static void retire_submit(struct msm_gpu *gpu, struct msm_ringbuffer *ring, stats->alwayson_start, stats->alwayson_end); for (i = 0; i < submit->nr_bos; i++) { - struct msm_gem_object *msm_obj = submit->bos[i].obj; + struct drm_gem_object *obj = &submit->bos[i].obj->base; - msm_gem_active_put(&msm_obj->base); - msm_gem_unpin_iova(&msm_obj->base, submit->aspace); - drm_gem_object_put(&msm_obj->base); + msm_gem_lock(obj); + msm_gem_active_put(obj); + msm_gem_unpin_iova_locked(obj, submit->aspace); + msm_gem_unlock(obj); + drm_gem_object_put(obj); } pm_runtime_mark_last_busy(&gpu->pdev->dev); From patchwork Sat Nov 14 19:30:09 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rob Clark X-Patchwork-Id: 11905783 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 61C08174A for ; Sat, 14 Nov 2020 19:28:52 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 3E6192225E for ; Sat, 14 Nov 2020 19:28:52 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="QqrU7Ij6" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726315AbgKNT2g (ORCPT ); Sat, 14 Nov 2020 14:28:36 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45262 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726231AbgKNT2g (ORCPT ); Sat, 14 Nov 2020 14:28:36 -0500 Received: from mail-pl1-x641.google.com (mail-pl1-x641.google.com [IPv6:2607:f8b0:4864:20::641]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 06944C0613D1; Sat, 14 Nov 2020 11:28:36 -0800 (PST) Received: by mail-pl1-x641.google.com with SMTP id s2so6115917plr.9; Sat, 14 Nov 2020 11:28:36 -0800 (PST) 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=bcNKe/onX36d4xFW2lvtFrnwC6y2tKCelaaYJA86BPE=; b=QqrU7Ij6kSjnsIh5ItWBED4TTHyJ4JAZb+100DLqno8upamEKgWNPOpRGmsVESkFgp wDBx2Qm6pssgIZTpQE5ml4GD1Bo4ZDorIFsLve24/S2IwJFjne27mpWBkSMSGA6B2oa0 +EbZMXc3IX2jEgZ62D3vYKOFFIxO0onLaRKXDemrCFLHqWvXnIT5Dc4Cs/X42W5K68B8 sR2CEL6iWCwD7Kb+D+0lbtYFDbGaJpBRTcZ+iXSJv3gmF5SGzbMrBb1CTRg3XvfDXB+A 8vPmxDv2bHIlQS3eYw++W6SAapW5tImKqaM6TwQqJuZX1Spz05+VpP+DDOzAZ2zGthR+ kbpg== 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=bcNKe/onX36d4xFW2lvtFrnwC6y2tKCelaaYJA86BPE=; b=T6s1ujHG9dco52WsTYElzXr5ohDligCBwgzfohFVoLgZ4/eqZrVD5aszwVqXD7JHjg XSbwu4cw2+tBK+YZicHOS7J5AYimhlJvDWInDKoJz95wRSjZMNWiqotgcLPC8zWk6tey F/FeriQL5d1z9m0x5az8JMKPGbIYsQTOCWyus1nIHg6y3VrR7te+qvfNC9bkqlN/ZxZg 5aBC+RmnkGtyX0gttgqNKX31DDNYx7tW9PlmLQBdK9w1tG2duRlPYFDiX7V74uOQvv8E 1AAim7B7s5QRlyniybrPPLf7kDRWqTcBOpBgKECcqtDhKoZ46bXWNfAv+AgSF7jDBWqQ Atvg== X-Gm-Message-State: AOAM532hB7fEp8sQRyBO4+SyFmqmW8zfo9OWuvBIZIvqYqgR9b4ZqcoS 3UeKeGy066dLVV5j00OrMg9j2ypHBx0= X-Google-Smtp-Source: ABdhPJzjHnH7nVWrBTKR9amjLPHBqTLTlwz9oXNlwf7v7SnZ+SQZxsWKqtf5m2Xz3zMFsbC4IS8iZw== X-Received: by 2002:a17:90a:7343:: with SMTP id j3mr6839137pjs.51.1605382115491; Sat, 14 Nov 2020 11:28:35 -0800 (PST) Received: from localhost (c-73-25-156-94.hsd1.or.comcast.net. [73.25.156.94]) by smtp.gmail.com with ESMTPSA id 3sm13415786pfv.92.2020.11.14.11.28.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 14 Nov 2020 11:28:34 -0800 (PST) From: Rob Clark To: dri-devel@lists.freedesktop.org Cc: Rob Clark , Rob Clark , Sean Paul , David Airlie , Daniel Vetter , 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 2/3] drm/msm/shrinker: We can vmap shrink active_list too Date: Sat, 14 Nov 2020 11:30:09 -0800 Message-Id: <20201114193010.753355-3-robdclark@gmail.com> X-Mailer: git-send-email 2.28.0 In-Reply-To: <20201114193010.753355-1-robdclark@gmail.com> References: <20201114193010.753355-1-robdclark@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org From: Rob Clark Just because a obj is active, if the vmap_count is zero, we can still tear down the vmap. Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gem_shrinker.c | 47 +++++++++++++++++++------- 1 file changed, 35 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_gem_shrinker.c b/drivers/gpu/drm/msm/msm_gem_shrinker.c index 6f4b1355725f..9d51c1eb808d 100644 --- a/drivers/gpu/drm/msm/msm_gem_shrinker.c +++ b/drivers/gpu/drm/msm/msm_gem_shrinker.c @@ -6,6 +6,7 @@ #include "msm_drv.h" #include "msm_gem.h" +#include "msm_gpu.h" #include "msm_gpu_trace.h" static unsigned long @@ -61,17 +62,19 @@ msm_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc) return freed; } -static int -msm_gem_shrinker_vmap(struct notifier_block *nb, unsigned long event, void *ptr) +/* since we don't know any better, lets bail after a few + * and if necessary the shrinker will be invoked again. + * Seems better than unmapping *everything* + */ +static const int vmap_shrink_limit = 15; + +static unsigned +vmap_shrink(struct list_head *mm_list) { - struct msm_drm_private *priv = - container_of(nb, struct msm_drm_private, vmap_notifier); struct msm_gem_object *msm_obj; unsigned unmapped = 0; - mutex_lock(&priv->mm_lock); - - list_for_each_entry(msm_obj, &priv->inactive_list, mm_list) { + list_for_each_entry(msm_obj, mm_list, mm_list) { if (!msm_gem_trylock(&msm_obj->base)) continue; if (is_vunmapable(msm_obj)) { @@ -80,11 +83,31 @@ msm_gem_shrinker_vmap(struct notifier_block *nb, unsigned long event, void *ptr) } msm_gem_unlock(&msm_obj->base); - /* since we don't know any better, lets bail after a few - * and if necessary the shrinker will be invoked again. - * Seems better than unmapping *everything* - */ - if (++unmapped >= 15) + if (++unmapped >= vmap_shrink_limit) + break; + } + + return unmapped; +} + +static int +msm_gem_shrinker_vmap(struct notifier_block *nb, unsigned long event, void *ptr) +{ + struct msm_drm_private *priv = + container_of(nb, struct msm_drm_private, vmap_notifier); + struct list_head *mm_lists[] = { + &priv->inactive_list, + priv->gpu ? &priv->gpu->active_list : NULL, + NULL, + }; + unsigned idx, unmapped = 0; + + mutex_lock(&priv->mm_lock); + + for (idx = 0; mm_lists[idx]; idx++) { + unmapped += vmap_shrink(mm_lists[idx]); + + if (unmapped >= vmap_shrink_limit) break; } From patchwork Sat Nov 14 19:30:10 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rob Clark X-Patchwork-Id: 11905785 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 91E0916C1 for ; Sat, 14 Nov 2020 19:28:52 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 727C62231B for ; Sat, 14 Nov 2020 19:28:52 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="b9JJUGK5" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726325AbgKNT2j (ORCPT ); Sat, 14 Nov 2020 14:28:39 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45272 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726231AbgKNT2i (ORCPT ); Sat, 14 Nov 2020 14:28:38 -0500 Received: from mail-pj1-x1041.google.com (mail-pj1-x1041.google.com [IPv6:2607:f8b0:4864:20::1041]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B2AECC0613D1; Sat, 14 Nov 2020 11:28:38 -0800 (PST) Received: by mail-pj1-x1041.google.com with SMTP id w20so2011356pjh.1; Sat, 14 Nov 2020 11:28:38 -0800 (PST) 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=9hezsjWfUVXluAEwjCPGwpO8neX5yHhAITTPqDFV4Hc=; b=b9JJUGK5EQcoxGFImr6syPvDAS66+zFg8jRWKbuPuCg2nOuwOq51LLSPn7lTbz7ES4 jYCpvK/b90aDHteksvHiPnnARkf5lB9soHRVNVInkLd3HmYOeA+7maXTg/d+t0wufJkO C9146wSbvisv8NL82EJIjrlF6LJuYHqv0KNhadJAWezc8TOWLfztlf9IwRDJFcpzRasX ulhWfgJP6USoLSwL8ee2brNjGv3x+F9AWVbD7MN5hDzdVCrNSi1owlojyyCde8QusGHv OEw37IMpoZZQ2zMBWvYrUcOzucFgXQELWkTVVjmzwQb/GMLxVpLQqTG8wYGOG78aUaVq y+7w== 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=9hezsjWfUVXluAEwjCPGwpO8neX5yHhAITTPqDFV4Hc=; b=qfP70o4DGVN/o7cQmPJMr+0ob0LNr8gVm13iVCPEbKrezZP4tTI+bUJv0+hMBax0H3 qs19kFi0WamgvJl3tfB8DLhiJ3jJ7C5Her+aa0Dj/mFXdKGaoU9cJ/BboucrB+sM+Vcx HUcuZIp80tNSjySARUma9E4Vap6NpT3W29ssZ64e8U/E2yQ36R5NK//1hegs/vzI6AUE 24DqUPJpiFZB2tWmCHTmnX3QotiG6gZVX2U1+EZQDPXPocWicHVb48g5A8OLD3o7OYOw Pe1hvPIvHa8WClCIgn2BZUXUANvYK/9SuIxp0e3YrEuILOjSAld7s4bDLxHhLGPWp70S QuNg== X-Gm-Message-State: AOAM532SWOfceAVcVPZSZv9Erifw5tCqA2W9CYaN1MR77M3Nf7suYtLY 4jLqHzaIBtfxMfUxqnSzyMo= X-Google-Smtp-Source: ABdhPJzo1p9Z7jUL1jsTRNURdpjV+smWTpP/PDEWIwAEUVTl7BBj2+AxfCz8ceYIAi6FLNrYngVqog== X-Received: by 2002:a17:902:b283:b029:d6:b2a7:3913 with SMTP id u3-20020a170902b283b02900d6b2a73913mr6911494plr.54.1605382118251; Sat, 14 Nov 2020 11:28:38 -0800 (PST) Received: from localhost (c-73-25-156-94.hsd1.or.comcast.net. [73.25.156.94]) by smtp.gmail.com with ESMTPSA id k73sm11683356pga.88.2020.11.14.11.28.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 14 Nov 2020 11:28:37 -0800 (PST) From: Rob Clark To: dri-devel@lists.freedesktop.org Cc: Rob Clark , Rob Clark , Sean Paul , David Airlie , Daniel Vetter , 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 3/3] drm/msm/shrinker: Only iterate dontneed objs Date: Sat, 14 Nov 2020 11:30:10 -0800 Message-Id: <20201114193010.753355-4-robdclark@gmail.com> X-Mailer: git-send-email 2.28.0 In-Reply-To: <20201114193010.753355-1-robdclark@gmail.com> References: <20201114193010.753355-1-robdclark@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org From: Rob Clark In situations where the GPU is mostly idle, all or nearly all buffer objects will be in the inactive list. But if the system is under memory pressure (from something other than GPU), we could still get a lot of shrinker calls. Which results in traversing a list of thousands of objs and in the end finding nothing to shrink. Which isn't so efficient. Instead split the inactive_list into two lists, one inactive objs which are shrinkable, and a second one for those that are not. This way we can avoid traversing objs which we know are not shrinker candidates. Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_debugfs.c | 3 ++- drivers/gpu/drm/msm/msm_drv.c | 3 ++- drivers/gpu/drm/msm/msm_drv.h | 8 +++--- drivers/gpu/drm/msm/msm_gem.c | 34 ++++++++++++++++++++------ drivers/gpu/drm/msm/msm_gem_shrinker.c | 7 +++--- 5 files changed, 40 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_debugfs.c b/drivers/gpu/drm/msm/msm_debugfs.c index 64afbed89821..85ad0babc326 100644 --- a/drivers/gpu/drm/msm/msm_debugfs.c +++ b/drivers/gpu/drm/msm/msm_debugfs.c @@ -124,7 +124,8 @@ static int msm_gem_show(struct drm_device *dev, struct seq_file *m) } seq_printf(m, "Inactive Objects:\n"); - msm_gem_describe_objects(&priv->inactive_list, m); + msm_gem_describe_objects(&priv->inactive_dontneed, m); + msm_gem_describe_objects(&priv->inactive_willneed, m); mutex_unlock(&priv->mm_lock); diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 4d808769e6ed..39a54f364aa8 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -465,7 +465,8 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv) priv->wq = alloc_ordered_workqueue("msm", 0); - INIT_LIST_HEAD(&priv->inactive_list); + INIT_LIST_HEAD(&priv->inactive_willneed); + INIT_LIST_HEAD(&priv->inactive_dontneed); mutex_init(&priv->mm_lock); /* Teach lockdep about lock ordering wrt. shrinker: */ diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h index f869ed67b5da..ed18c5bed10f 100644 --- a/drivers/gpu/drm/msm/msm_drv.h +++ b/drivers/gpu/drm/msm/msm_drv.h @@ -175,8 +175,9 @@ struct msm_drm_private { struct msm_perf_state *perf; /* - * List of inactive GEM objects. Every bo is either in the inactive_list - * or gpu->active_list (for the gpu it is active on[1]) + * Lists of inactive GEM objects. Every bo is either in one of the + * inactive lists (depending on whether or not it is shrinkable) or + * gpu->active_list (for the gpu it is active on[1]) * * These lists are protected by mm_lock. If struct_mutex is involved, it * should be aquired prior to mm_lock. One should *not* hold mm_lock in @@ -185,7 +186,8 @@ struct msm_drm_private { * [1] if someone ever added support for the old 2d cores, there could be * more than one gpu object */ - struct list_head inactive_list; + struct list_head inactive_willneed; /* inactive + !shrinkable */ + struct list_head inactive_dontneed; /* inactive + shrinkable */ struct mutex mm_lock; struct workqueue_struct *wq; diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c index 2795288b0a95..de8d2cfada24 100644 --- a/drivers/gpu/drm/msm/msm_gem.c +++ b/drivers/gpu/drm/msm/msm_gem.c @@ -17,6 +17,7 @@ #include "msm_gpu.h" #include "msm_mmu.h" +static void update_inactive(struct msm_gem_object *msm_obj); static dma_addr_t physaddr(struct drm_gem_object *obj) { @@ -678,6 +679,12 @@ int msm_gem_madvise(struct drm_gem_object *obj, unsigned madv) madv = msm_obj->madv; + /* If the obj is inactive, we might need to move it + * between inactive lists + */ + if (msm_obj->active_count == 0) + update_inactive(msm_obj); + msm_gem_unlock(obj); return (madv != __MSM_MADV_PURGED); @@ -781,19 +788,31 @@ void msm_gem_active_get(struct drm_gem_object *obj, struct msm_gpu *gpu) void msm_gem_active_put(struct drm_gem_object *obj) { struct msm_gem_object *msm_obj = to_msm_bo(obj); - struct msm_drm_private *priv = obj->dev->dev_private; might_sleep(); WARN_ON(!msm_gem_is_locked(obj)); if (--msm_obj->active_count == 0) { - mutex_lock(&priv->mm_lock); - list_del_init(&msm_obj->mm_list); - list_add_tail(&msm_obj->mm_list, &priv->inactive_list); - mutex_unlock(&priv->mm_lock); + update_inactive(msm_obj); } } +static void update_inactive(struct msm_gem_object *msm_obj) +{ + struct msm_drm_private *priv = msm_obj->base.dev->dev_private; + + mutex_lock(&priv->mm_lock); + WARN_ON(msm_obj->active_count != 0); + + list_del_init(&msm_obj->mm_list); + if (msm_obj->madv == MSM_MADV_DONTNEED) + list_add_tail(&msm_obj->mm_list, &priv->inactive_willneed); + else + list_add_tail(&msm_obj->mm_list, &priv->inactive_dontneed); + + mutex_unlock(&priv->mm_lock); +} + int msm_gem_cpu_prep(struct drm_gem_object *obj, uint32_t op, ktime_t *timeout) { bool write = !!(op & MSM_PREP_WRITE); @@ -1099,7 +1118,8 @@ static struct drm_gem_object *_msm_gem_new(struct drm_device *dev, } mutex_lock(&priv->mm_lock); - list_add_tail(&msm_obj->mm_list, &priv->inactive_list); + /* Initially obj is idle, obj->madv == WILLNEED: */ + list_add_tail(&msm_obj->mm_list, &priv->inactive_willneed); mutex_unlock(&priv->mm_lock); return obj; @@ -1169,7 +1189,7 @@ struct drm_gem_object *msm_gem_import(struct drm_device *dev, msm_gem_unlock(obj); mutex_lock(&priv->mm_lock); - list_add_tail(&msm_obj->mm_list, &priv->inactive_list); + list_add_tail(&msm_obj->mm_list, &priv->inactive_willneed); mutex_unlock(&priv->mm_lock); return obj; diff --git a/drivers/gpu/drm/msm/msm_gem_shrinker.c b/drivers/gpu/drm/msm/msm_gem_shrinker.c index 9d51c1eb808d..81dfa57b6a0d 100644 --- a/drivers/gpu/drm/msm/msm_gem_shrinker.c +++ b/drivers/gpu/drm/msm/msm_gem_shrinker.c @@ -19,7 +19,7 @@ msm_gem_shrinker_count(struct shrinker *shrinker, struct shrink_control *sc) mutex_lock(&priv->mm_lock); - list_for_each_entry(msm_obj, &priv->inactive_list, mm_list) { + list_for_each_entry(msm_obj, &priv->inactive_dontneed, mm_list) { if (!msm_gem_trylock(&msm_obj->base)) continue; if (is_purgeable(msm_obj)) @@ -42,7 +42,7 @@ msm_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc) mutex_lock(&priv->mm_lock); - list_for_each_entry(msm_obj, &priv->inactive_list, mm_list) { + list_for_each_entry(msm_obj, &priv->inactive_dontneed, mm_list) { if (freed >= sc->nr_to_scan) break; if (!msm_gem_trylock(&msm_obj->base)) @@ -96,7 +96,8 @@ msm_gem_shrinker_vmap(struct notifier_block *nb, unsigned long event, void *ptr) struct msm_drm_private *priv = container_of(nb, struct msm_drm_private, vmap_notifier); struct list_head *mm_lists[] = { - &priv->inactive_list, + &priv->inactive_dontneed, + &priv->inactive_willneed, priv->gpu ? &priv->gpu->active_list : NULL, NULL, };