From nobody Sat Jun 13 09:18:59 2026 Received: from bali.collaboradmins.com (bali.collaboradmins.com [148.251.105.195]) (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 C888F3D3324; Fri, 8 May 2026 10:41:14 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=148.251.105.195 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778236876; cv=none; b=Fd71SCjeu+YfIqcP2OlHS2339KU7qbpElw5JpTNogvfp65rDKHPkCO3gC/BXIih39DYF2JzC32X+8OdcHRWluPz5TGGYlfdTifceKV0Fzx4hHN2Sn0keSSj+eQjzO/6g+PHaRzDF9qcnnc5HqnjJUAsnqqv23LaSXpIzz7Bdxvw= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778236876; c=relaxed/simple; bh=CCjbAOv+AUfwV71P61bambB9KPgJkBGpAkVfSqu3IzE=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=Hm/D6kd8WPjidNrsUMmU9dvmYbog86v9re2UntAhDphpqxF3bA+Iu/H5PMSjmKB0+uCDJcBrFAlvMNKJxmvZ+kIDVaXjYfzWd6WJjyXaitfEpV8mCvRfOb2gJ16g4DVtI99d/ZtOXS0I+4w6qo8JWZDmL8fzHuKszya0cm/ByeI= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=collabora.com; spf=pass smtp.mailfrom=collabora.com; dkim=pass (2048-bit key) header.d=collabora.com header.i=@collabora.com header.b=LS4wB9Ow; arc=none smtp.client-ip=148.251.105.195 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=collabora.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=collabora.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=collabora.com header.i=@collabora.com header.b="LS4wB9Ow" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1778236873; bh=CCjbAOv+AUfwV71P61bambB9KPgJkBGpAkVfSqu3IzE=; h=From:Date:Subject:References:In-Reply-To:To:Cc:From; b=LS4wB9OwTbq8MAUejBMyJj3E3GtQ9Qa9GzIbZZAv3gl5VxR5tJ5/k2BeAF+VWaLFY 7skauOxzS+4Vfo8bYcwjriU9GTtP/DDH0OXJ8zQ/9NOjTpkQ9B0d+YxDVGkMAqXycC 5nPiIKp1zWIA9XG5tulH8huQZDVTSppp2wq36r1yAcIhrO4j6DmNKWrA6/UbVC+yqz BISTiKWXkiZcsMBmxLEA7BEf3ImfH/qcNDXQ5HFO0MeWgrKXqI2N8sMme9c9X0VlIY /JbXlVnI7eGXAxuC7zPSatJR5Ho0ZFpE45pUewJt1h5aj+vIYMcKuyIBQmG1pY7xRP X6s1DmrUqW3JA== Received: from [100.64.0.11] (unknown [100.64.0.11]) (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) (Authenticated sender: bbrezillon) by bali.collaboradmins.com (Postfix) with ESMTPSA id 8A3B717E138B; Fri, 8 May 2026 12:41:12 +0200 (CEST) From: Boris Brezillon Date: Fri, 08 May 2026 12:40:47 +0200 Subject: [PATCH v2 1/4] drm/panthor: Don't use the racy drm_gem_lru_remove() helper Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Message-Id: <20260508-panthor-shrinker-fixes-v2-1-39cdb7d577c9@collabora.com> References: <20260508-panthor-shrinker-fixes-v2-0-39cdb7d577c9@collabora.com> In-Reply-To: <20260508-panthor-shrinker-fixes-v2-0-39cdb7d577c9@collabora.com> To: Steven Price , Liviu Dudau , Boris Brezillon Cc: Dmitry Osipenko , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Simona Vetter , Akash Goel , Chia-I Wu , Rob Clark , Dmitry Baryshkov , Abhinav Kumar , Jessica Zhang , Sean Paul , Marijn Suijten , linux-arm-msm@vger.kernel.org, freedreno@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org X-Mailer: b4 0.14.3 X-Developer-Signature: v=1; a=ed25519-sha256; t=1778236871; l=3640; i=boris.brezillon@collabora.com; s=20260429; h=from:subject:message-id; bh=CCjbAOv+AUfwV71P61bambB9KPgJkBGpAkVfSqu3IzE=; b=S+kY1FBPu22WvLC7UmxPfpUmHF0PXd4jJKCwO+GBDLneBrvYhv2/fwn+tybBr1jVPdusToIko rYY2lkNnIDrDklIlf8CddnL8TyJks/V1TNctT6p+avkhkliKvorufOQ X-Developer-Key: i=boris.brezillon@collabora.com; a=ed25519; pk=eN+ORdOgQY7d5U+0kA8h5bf67XdD8bhKbjD/TCHexSY= drm_gem_lru_remove() stores drm_gem_object::lru in a local variable that's then dereferenced to acquire the LRU lock. Because this assignment is done without the LRU lock held, it can race with drm_gem_lru_scan() where drm_gem_object::lru is temporarily assigned a stack-allcated LRU that goes away when leaving the function. By the time we dereference this local lru variable, the object might already be gone. It feels like drm_gem_lru_remove() was never meant to be used this way, because there's no easy way we can avoid this race unless we defer the locking to the caller. Let's add an explicit LRU for unreclaimable BOs instead, and have all BOs added to this LRU at creation time. Fixes: fb42964e2a76 ("drm/panthor: Add a GEM shrinker") Reported-by: Chia-I Wu Closes: https://gitlab.freedesktop.org/panfrost/linux/-/work_items/86 Reviewed-by: Chia-I Wu Reviewed-by: Steven Price Reviewed-by: Liviu Dudau Signed-off-by: Boris Brezillon --- drivers/gpu/drm/panthor/panthor_device.h | 10 ++++++++++ drivers/gpu/drm/panthor/panthor_gem.c | 5 ++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/pan= thor/panthor_device.h index 4e4607bca7cc..dcdce75b683b 100644 --- a/drivers/gpu/drm/panthor/panthor_device.h +++ b/drivers/gpu/drm/panthor/panthor_device.h @@ -190,6 +190,16 @@ struct panthor_device { /** @reclaim.lock: Lock protecting all LRUs */ struct mutex lock; =20 + /** + * @reclaim.unreclaimable: unreclaimable BOs + * + * Either the BO is unreclaimable because it has no pages allocated, + * or it's unreclaimable because pages are pinned. + * + * All BOs start in this list at creation time. + */ + struct drm_gem_lru unreclaimable; + /** * @reclaim.unused: BOs with unused pages * diff --git a/drivers/gpu/drm/panthor/panthor_gem.c b/drivers/gpu/drm/pantho= r/panthor_gem.c index 13295d7a593d..8e31740126e7 100644 --- a/drivers/gpu/drm/panthor/panthor_gem.c +++ b/drivers/gpu/drm/panthor/panthor_gem.c @@ -204,7 +204,7 @@ void panthor_gem_update_reclaim_state_locked(struct pan= thor_gem_object *bo, drm_gem_lru_move_tail(&ptdev->reclaim.gpu_mapped_shared, &bo->base); break; case PANTHOR_GEM_UNRECLAIMABLE: - drm_gem_lru_remove(&bo->base); + drm_gem_lru_move_tail(&ptdev->reclaim.unreclaimable, &bo->base); break; default: drm_WARN(&ptdev->base, true, "invalid GEM reclaim state (%d)\n", new_sta= te); @@ -994,6 +994,7 @@ static struct panthor_gem_object * panthor_gem_create(struct drm_device *dev, size_t size, uint32_t flags, struct panthor_vm *exclusive_vm, u32 usage_flags) { + struct panthor_device *ptdev =3D container_of(dev, struct panthor_device,= base); struct panthor_gem_object *bo; int ret; =20 @@ -1026,6 +1027,7 @@ panthor_gem_create(struct drm_device *dev, size_t siz= e, uint32_t flags, } =20 panthor_gem_debugfs_set_usage_flags(bo, usage_flags); + drm_gem_lru_move_tail(&ptdev->reclaim.unreclaimable, &bo->base); return bo; =20 err_put: @@ -1551,6 +1553,7 @@ int panthor_gem_shrinker_init(struct panthor_device *= ptdev) return ret; =20 INIT_LIST_HEAD(&ptdev->reclaim.vms); + drm_gem_lru_init(&ptdev->reclaim.unreclaimable, &ptdev->reclaim.lock); drm_gem_lru_init(&ptdev->reclaim.unused, &ptdev->reclaim.lock); drm_gem_lru_init(&ptdev->reclaim.mmapped, &ptdev->reclaim.lock); drm_gem_lru_init(&ptdev->reclaim.gpu_mapped_shared, &ptdev->reclaim.lock); --=20 2.54.0 From nobody Sat Jun 13 09:18:59 2026 Received: from bali.collaboradmins.com (bali.collaboradmins.com [148.251.105.195]) (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 C03473D349C; Fri, 8 May 2026 10:41:15 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=148.251.105.195 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778236877; cv=none; b=EDG4wZ0CgDIi5BDqMbdB+Zdvir1ldJOsG1wglYGAL4rzSN+OBIQMM64gOW96s1yqXbwDiuQpJ5w4Ev01uJATmh9STwrrRJkccvJBEQVVfVz1Ad8HCwEPH+Q5SbSir3G0nHyIvxPWHErUreYP8gf9bLiQhkAWJw4NCllk7DuCGfQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778236877; c=relaxed/simple; bh=0SZ027itVmKWLKnDx2ABO2cRND/xTdZFsyJoY99ik1Y=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=r3Ni3xwZ1rIcmf7vkKYs50CzoVm5i0CfLeAcnDfnJLnDy+UcKs9tfGhbA9mwQk/PAfbh8b9Dy/gn+eIxBUsVZDLMQAfi1CRjuqX/5MoZ1Ua0ZN0QCz+YV6a0gURki2u5OSPDA1fdfndrsHVRKtoxkth4Ts2yXt+o9RoqgZ0woec= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=collabora.com; spf=pass smtp.mailfrom=collabora.com; dkim=pass (2048-bit key) header.d=collabora.com header.i=@collabora.com header.b=moiQufS+; arc=none smtp.client-ip=148.251.105.195 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=collabora.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=collabora.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=collabora.com header.i=@collabora.com header.b="moiQufS+" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1778236874; bh=0SZ027itVmKWLKnDx2ABO2cRND/xTdZFsyJoY99ik1Y=; h=From:Date:Subject:References:In-Reply-To:To:Cc:From; b=moiQufS+pP62dMZQMxsjJd+oVLV+35b9sbpN7yApjREtAf99HbU/baope+GzT0tL4 gE+dzy67JkURm3GGqH/lDE4LkOU9jflsmpOIceshq6HyU4Eq1A+jIIe5pK/IvgAAlO f+/JqeGFs6RyDuXIFfWbwP6RNZr4mcvKlGePeOZbioVS05rBwmU6XPk3vsCsw/hVAT eHLix+rRF3Bv1ZPd+wpzh0wF8RI8DYavpoEjA1fNSM0Vs3oVfnrAYszo6nWSpYwAGt vEC8MvJN7gnzS7QYivGb2b7pz/61STNo+0Z5zcfVHpc3WRx8XkKX6oGYzyUSXVt5x/ oEhY1zEi3RsEA== Received: from [100.64.0.11] (unknown [100.64.0.11]) (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) (Authenticated sender: bbrezillon) by bali.collaboradmins.com (Postfix) with ESMTPSA id 69D6617E14DD; Fri, 8 May 2026 12:41:13 +0200 (CEST) From: Boris Brezillon Date: Fri, 08 May 2026 12:40:48 +0200 Subject: [PATCH v2 2/4] drm/gem: Fix a race between drm_gem_lru_scan() and drm_gem_object_release() Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Message-Id: <20260508-panthor-shrinker-fixes-v2-2-39cdb7d577c9@collabora.com> References: <20260508-panthor-shrinker-fixes-v2-0-39cdb7d577c9@collabora.com> In-Reply-To: <20260508-panthor-shrinker-fixes-v2-0-39cdb7d577c9@collabora.com> To: Steven Price , Liviu Dudau , Boris Brezillon Cc: Dmitry Osipenko , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Simona Vetter , Akash Goel , Chia-I Wu , Rob Clark , Dmitry Baryshkov , Abhinav Kumar , Jessica Zhang , Sean Paul , Marijn Suijten , linux-arm-msm@vger.kernel.org, freedreno@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org X-Mailer: b4 0.14.3 X-Developer-Signature: v=1; a=ed25519-sha256; t=1778236871; l=3930; i=boris.brezillon@collabora.com; s=20260429; h=from:subject:message-id; bh=0SZ027itVmKWLKnDx2ABO2cRND/xTdZFsyJoY99ik1Y=; b=WJffySdN1YIdiEbDmRDkc7kl5DHu70qjBa5OH8jQMMSnX3dqQ46JdxMyQwb1tZUbblBv61mm6 ue0lG6MYqNKCN/HyHsB/lsjZ2HHKuLxFDn2Rws7yxASZFsnVlTVS6/a X-Developer-Key: i=boris.brezillon@collabora.com; a=ed25519; pk=eN+ORdOgQY7d5U+0kA8h5bf67XdD8bhKbjD/TCHexSY= The following race can currently happen: | Thread 0 in `drm_gem_lru_scan` | Thread 1 in `drm_gem_objec= t_release` | | - | - = | | move obj1 with refcount=3D=3D0 to `still_in_lru` | = | | move obj2 with refcount!=3D0 to `still_in_lru` | = | | mutex_unlock | = | | shrink obj2 | = | | | lru =3D obj1->lru; // `sti= ll_in_lru` | | mutex_lock | = | | move obj1 back to the original lru | = | | mutex_unlock | = | | return | = | | | dereference `still_in_lru`= | Move the drm_gem_lru_move_tail_locked() after the kref_get_unless_zero() check so that we don't end up with a vanishing LRU when we hit drm_gem_object_release(). We also need to remove the skipped object from its LRU, otherwise we'll keep hitting it on subsequent loop iterations until it's actually removed from the list in the drm_gem_release(). Fixes: e7c2af13f811 ("drm/gem: Add LRU/shrinker helper") Reported-by: Chia-I Wu Closes: https://gitlab.freedesktop.org/panfrost/linux/-/work_items/86 Signed-off-by: Boris Brezillon Reviewed-by: Liviu Dudau --- drivers/gpu/drm/drm_gem.c | 34 ++++++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index fca42949eb2b..0e087c770883 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -1573,11 +1573,31 @@ drm_gem_lru_remove(struct drm_gem_object *obj) { struct drm_gem_lru *lru =3D obj->lru; =20 + /* + * We do the lru !=3D NULL check without the lru->lock held, which + * means we might end up with a stale lru value by the time the + * lock is acquired. + * + * This is deemed safe because: + * 1. the LRU is assumed to outlive any GEM object it was attached + * (LRUs are usually bound to a drm_device). So even if obj->lru + * has become NULL, it still point to a valid object that can + * safely be dereferenced to get the lock. + * + * 2. all LRUs a GEM object might be attached to must share the same + * lock (lock that's usually part of the driver-specific device + * object), so taking the lock on the 'old' LRU is equivalent + * to taking it on the new one (if any) + */ if (!lru) return; =20 mutex_lock(lru->lock); - drm_gem_lru_remove_locked(obj); + /* Check a second time with the lock held to make sure we're not racing + * with another drm_gem_lru_remove[_locked]() call. + */ + if (obj->lru) + drm_gem_lru_remove_locked(obj); mutex_unlock(lru->lock); } EXPORT_SYMBOL(drm_gem_lru_remove); @@ -1660,15 +1680,17 @@ drm_gem_lru_scan(struct drm_gem_lru *lru, if (!obj) break; =20 - drm_gem_lru_move_tail_locked(&still_in_lru, obj); - /* * If it's in the process of being freed, gem_object->free() - * may be blocked on lock waiting to remove it. So just - * skip it. + * may be blocked on lock waiting to remove it. So just remove + * it from its current LRU and skip it. */ - if (!kref_get_unless_zero(&obj->refcount)) + if (!kref_get_unless_zero(&obj->refcount)) { + drm_gem_lru_remove_locked(obj); continue; + } + + drm_gem_lru_move_tail_locked(&still_in_lru, obj); =20 /* * Now that we own a reference, we can drop the lock for the --=20 2.54.0 From nobody Sat Jun 13 09:18:59 2026 Received: from bali.collaboradmins.com (bali.collaboradmins.com [148.251.105.195]) (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 A2BF83D3CFB; Fri, 8 May 2026 10:41:16 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=148.251.105.195 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778236878; cv=none; b=KWrNacdSfvu+Kvz4QfFYmy8zq/w3QApRB5aJA4HefaH4apecqhkdz/x9cwqqjGAxo6w4tR6Cdnv5NEO92D1GxvNlO/+xZZh2LEApHDAUnPpwAFo367krPZ4egljgcMsypszQ4TA8R92Paw5SczdVKCuRmNusnDCyiEH42NRXSQI= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778236878; c=relaxed/simple; bh=hQnI7G5tnzx0UqTxLwoGWdFSLtG2RIqZJo8XhGgWE3A=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=ohqkrAgv+S4Hg0N2ndEjNeW/N2aDdOEXqdCbutE9OLQRzJqKb5+KdwpDLYQuw4jmxpkBtvdFmsbs999sHyYjfYfI3+CmAAyJV6eTgIbglj9cd4iys8W4GOq6rqZNkZ55YzZDVjqJ12TPGgkZlfKCzNY3kbBMhXZiSPmk6c+uXh8= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=collabora.com; spf=pass smtp.mailfrom=collabora.com; dkim=pass (2048-bit key) header.d=collabora.com header.i=@collabora.com header.b=cy0cRK7l; arc=none smtp.client-ip=148.251.105.195 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=collabora.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=collabora.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=collabora.com header.i=@collabora.com header.b="cy0cRK7l" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1778236875; bh=hQnI7G5tnzx0UqTxLwoGWdFSLtG2RIqZJo8XhGgWE3A=; h=From:Date:Subject:References:In-Reply-To:To:Cc:From; b=cy0cRK7lmQO97SFZj3t1I0RcJnZJErPviKcnClJP3sXabvXzMc30zYYjQYQzajMsx ldTct1+hMGyrgtDz0Cdrhg2fuF/yKquMtlVIOSWv73Q6fFOaQSEmMUl8ovgkO8Cb3/ wzsXIZDHacTtBGy/+CIXRqlywMT7yFZN2lXp5f9m90Z4+wlDvJaCg92ERoC8Ny3Y71 1+n2HM7/Jcdmez8Ut5W3GhtmnKCuS8RmS1YaLFBDQSeSpbEcQk2HQVf+vEv0mu7dGf deddqx7NnLIasdeI399xxXpVwQfpgWQZQhmlSKGvDekm18n73K4yOzLwqd3mY68JQy rkK2ZofcyxjxQ== Received: from [100.64.0.11] (unknown [100.64.0.11]) (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) (Authenticated sender: bbrezillon) by bali.collaboradmins.com (Postfix) with ESMTPSA id 4C16117E1502; Fri, 8 May 2026 12:41:14 +0200 (CEST) From: Boris Brezillon Date: Fri, 08 May 2026 12:40:49 +0200 Subject: [PATCH v2 3/4] drm/gem: Stop exposing the racy/unsafe drm_gem_lru_remove() helper Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Message-Id: <20260508-panthor-shrinker-fixes-v2-3-39cdb7d577c9@collabora.com> References: <20260508-panthor-shrinker-fixes-v2-0-39cdb7d577c9@collabora.com> In-Reply-To: <20260508-panthor-shrinker-fixes-v2-0-39cdb7d577c9@collabora.com> To: Steven Price , Liviu Dudau , Boris Brezillon Cc: Dmitry Osipenko , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Simona Vetter , Akash Goel , Chia-I Wu , Rob Clark , Dmitry Baryshkov , Abhinav Kumar , Jessica Zhang , Sean Paul , Marijn Suijten , linux-arm-msm@vger.kernel.org, freedreno@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org X-Mailer: b4 0.14.3 X-Developer-Signature: v=1; a=ed25519-sha256; t=1778236871; l=4996; i=boris.brezillon@collabora.com; s=20260429; h=from:subject:message-id; bh=hQnI7G5tnzx0UqTxLwoGWdFSLtG2RIqZJo8XhGgWE3A=; b=5v335GdSDcWVQpzkJt3Y7m7Fp8dFuO9rkqc/hK8ocl671AImHIk3W5mqYh+6MK1R3NelbtDO3 6zXi5Gt+JYbD9sQ6JZT3nZaW9mMXNwRL9Wzfd4SIjz20Nk5YvWs6pNr X-Developer-Key: i=boris.brezillon@collabora.com; a=ed25519; pk=eN+ORdOgQY7d5U+0kA8h5bf67XdD8bhKbjD/TCHexSY= The only place where it's safe to call drm_gem_lru_remove() is when we know the drm_gem_object::lru field can't be concurrently updated, which we know is the case when the drm_gem_object is destroyed. Rather than trying to make that safe, let's kill the function and inline its content in drm_gem_object_release(). Signed-off-by: Boris Brezillon Reviewed-by: Liviu Dudau --- drivers/gpu/drm/drm_gem.c | 90 ++++++++++++++++++++-----------------------= ---- include/drm/drm_gem.h | 1 - 2 files changed, 39 insertions(+), 52 deletions(-) diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 0e087c770883..c85a39b8b163 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -1108,6 +1108,15 @@ drm_gem_release(struct drm_device *dev, struct drm_f= ile *file_private) idr_destroy(&file_private->object_idr); } =20 +static void +drm_gem_lru_remove_locked(struct drm_gem_object *obj) +{ + obj->lru->count -=3D obj->size >> PAGE_SHIFT; + WARN_ON(obj->lru->count < 0); + list_del(&obj->lru_node); + obj->lru =3D NULL; +} + /** * drm_gem_object_release - release GEM buffer object resources * @obj: GEM buffer object @@ -1118,13 +1127,42 @@ drm_gem_release(struct drm_device *dev, struct drm_= file *file_private) void drm_gem_object_release(struct drm_gem_object *obj) { + struct drm_gem_lru *lru; + if (obj->filp) fput(obj->filp); =20 drm_gem_private_object_fini(obj); =20 drm_gem_free_mmap_offset(obj); - drm_gem_lru_remove(obj); + + /* + * We do the lru !=3D NULL check without the lru->lock held, which + * means we might end up with a stale lru value by the time the + * lock is acquired. + * + * This is deemed safe because: + * 1. the LRU is assumed to outlive any GEM object it was attached + * (LRUs are usually bound to a drm_device). So even if obj->lru + * has become NULL, it still point to a valid object that can + * safely be dereferenced to get the lock. + * + * 2. all LRUs a GEM object might be attached to must share the same + * lock (lock that's usually part of the driver-specific device + * object), so taking the lock on the 'old' LRU is equivalent + * to taking it on the new one (if any) + */ + lru =3D obj->lru; + if (lru) { + guard(mutex)(lru->lock); + + /* Check a second time with the lock held to make sure we're + * not racing with the drm_gem_lru_remove_locked() call in + * drm_gem_lru_scan(). + */ + if (obj->lru) + drm_gem_lru_remove_locked(obj); + } } EXPORT_SYMBOL(drm_gem_object_release); =20 @@ -1552,56 +1590,6 @@ drm_gem_lru_init(struct drm_gem_lru *lru, struct mut= ex *lock) } EXPORT_SYMBOL(drm_gem_lru_init); =20 -static void -drm_gem_lru_remove_locked(struct drm_gem_object *obj) -{ - obj->lru->count -=3D obj->size >> PAGE_SHIFT; - WARN_ON(obj->lru->count < 0); - list_del(&obj->lru_node); - obj->lru =3D NULL; -} - -/** - * drm_gem_lru_remove - remove object from whatever LRU it is in - * - * If the object is currently in any LRU, remove it. - * - * @obj: The GEM object to remove from current LRU - */ -void -drm_gem_lru_remove(struct drm_gem_object *obj) -{ - struct drm_gem_lru *lru =3D obj->lru; - - /* - * We do the lru !=3D NULL check without the lru->lock held, which - * means we might end up with a stale lru value by the time the - * lock is acquired. - * - * This is deemed safe because: - * 1. the LRU is assumed to outlive any GEM object it was attached - * (LRUs are usually bound to a drm_device). So even if obj->lru - * has become NULL, it still point to a valid object that can - * safely be dereferenced to get the lock. - * - * 2. all LRUs a GEM object might be attached to must share the same - * lock (lock that's usually part of the driver-specific device - * object), so taking the lock on the 'old' LRU is equivalent - * to taking it on the new one (if any) - */ - if (!lru) - return; - - mutex_lock(lru->lock); - /* Check a second time with the lock held to make sure we're not racing - * with another drm_gem_lru_remove[_locked]() call. - */ - if (obj->lru) - drm_gem_lru_remove_locked(obj); - mutex_unlock(lru->lock); -} -EXPORT_SYMBOL(drm_gem_lru_remove); - /** * drm_gem_lru_move_tail_locked - move the object to the tail of the LRU * diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h index 86f5846154f7..d527df98d142 100644 --- a/include/drm/drm_gem.h +++ b/include/drm/drm_gem.h @@ -611,7 +611,6 @@ int drm_gem_dumb_map_offset(struct drm_file *file, stru= ct drm_device *dev, u32 handle, u64 *offset); =20 void drm_gem_lru_init(struct drm_gem_lru *lru, struct mutex *lock); -void drm_gem_lru_remove(struct drm_gem_object *obj); void drm_gem_lru_move_tail_locked(struct drm_gem_lru *lru, struct drm_gem_= object *obj); void drm_gem_lru_move_tail(struct drm_gem_lru *lru, struct drm_gem_object = *obj); unsigned long --=20 2.54.0 From nobody Sat Jun 13 09:18:59 2026 Received: from bali.collaboradmins.com (bali.collaboradmins.com [148.251.105.195]) (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 6F26C3D3D1D; Fri, 8 May 2026 10:41:17 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=148.251.105.195 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778236881; cv=none; b=ZdOCE2UJN5fhHlthU2QK1Gznl3LIIoIhzWyWPKXeEo4kTFp4tNLNzCTc9H4sz8NKRvUW6Q3pSnQvp1mHV9jKNPtdU3h+3zPBeqDAWTj+bbMO4SqfZe5+KU8Mp4S+jntnF3sQSlucKTDX7cO5+T2CptVOe+nDM/lqtdCV7WnmJaw= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778236881; c=relaxed/simple; bh=yFgR2hISu97FtZxkMWezkPJa03YxIP5P51pDzQLo/UA=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=ZjYeVcg/XzgSSKI+P1fXW+3icMMCfSiKZg7jQjVii+/Cb5Hug2ekFRQEd79gii636h1Vcws9zO66um/Rocno4BcLvIrA8gkbap83BfpRp+olw0UwSS3JLWqSZ+BlXNr/Y2QI2vnvv38TPyj5lX0+G0O9oluMhSHxHhXbq8ZYInA= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=collabora.com; spf=pass smtp.mailfrom=collabora.com; dkim=pass (2048-bit key) header.d=collabora.com header.i=@collabora.com header.b=gdWMcJXz; arc=none smtp.client-ip=148.251.105.195 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=collabora.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=collabora.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=collabora.com header.i=@collabora.com header.b="gdWMcJXz" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1778236875; bh=yFgR2hISu97FtZxkMWezkPJa03YxIP5P51pDzQLo/UA=; h=From:Date:Subject:References:In-Reply-To:To:Cc:From; b=gdWMcJXzKo8U4bYX2WaVKKhiMACYDuExrDRDehbQeiD2XbcDoK2ez/XGTjcwmV91y jTZuEsbtpMsYCfsW+MoW7CVoHgX3MUHJzMtiEJUavhPy74aYYJv7kiJN2iBLHRYSkv xaOENT0PeCUX03KMv02YxkYcQjN+ete2+riNpXH6lSoEOyYBg0j50Y28EbB1fH5ol1 r9StEBSzWOGJxD4vdMdNHuaDCOeftO1cYwJR3ztzPm1qBxk+lrBlQ3KWUijaI+u7U2 7jxgS7WeVh5IAimqR/BO1NjurGyIuyjczzjhY/uSiPOs5hAc4iDwU5upsfHEvFQWkW dv+Orew3KChxg== Received: from [100.64.0.11] (unknown [100.64.0.11]) (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) (Authenticated sender: bbrezillon) by bali.collaboradmins.com (Postfix) with ESMTPSA id 2BA9B17E150A; Fri, 8 May 2026 12:41:15 +0200 (CEST) From: Boris Brezillon Date: Fri, 08 May 2026 12:40:50 +0200 Subject: [PATCH v2 4/4] drm/gem: Make the GEM LRU lock part of drm_device Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Message-Id: <20260508-panthor-shrinker-fixes-v2-4-39cdb7d577c9@collabora.com> References: <20260508-panthor-shrinker-fixes-v2-0-39cdb7d577c9@collabora.com> In-Reply-To: <20260508-panthor-shrinker-fixes-v2-0-39cdb7d577c9@collabora.com> To: Steven Price , Liviu Dudau , Boris Brezillon Cc: Dmitry Osipenko , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Simona Vetter , Akash Goel , Chia-I Wu , Rob Clark , Dmitry Baryshkov , Abhinav Kumar , Jessica Zhang , Sean Paul , Marijn Suijten , linux-arm-msm@vger.kernel.org, freedreno@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org X-Mailer: b4 0.14.3 X-Developer-Signature: v=1; a=ed25519-sha256; t=1778236871; l=25597; i=boris.brezillon@collabora.com; s=20260429; h=from:subject:message-id; bh=yFgR2hISu97FtZxkMWezkPJa03YxIP5P51pDzQLo/UA=; b=55j0S9OYo5m0Iw2FYPr8mSh05HTfQZ5CXAJvl7KBL+OwfS3TVCJExnfJXBjtaz61tCmzFEw2l rY8Yxxkhu04A84DVk5o5lqVJ9UaT5bzPS622wb5pUFOsJEhbUB15GVH X-Developer-Key: i=boris.brezillon@collabora.com; a=ed25519; pk=eN+ORdOgQY7d5U+0kA8h5bf67XdD8bhKbjD/TCHexSY= Recently, a few races have been discovered in the GEM LRU logic, all of them caused by the fact the LRU lock is accessed through gem->lru->lock, and that lock itself also protects changes to gem->lru, leading to situations where gem->lru needs to first be accessed without the lock held, to then get the lru to access the lock through and finally take the lock and do the expected operation. Currently, the two drivers making use of this API declare device-wide locks, and there's no clue that we will ever have a driver that wants different pools of LRUs protected by different locks under the same drm_device. So we're better off moving this lock to drm_device and always locking it through obj->dev->gem_lru_mutex, or directly through dev->gem_lru_mutex. If anyone ever needs more fine-grained locking, this can be revisited to pass some drm_gem_lru_pool object represent the pool of LRUs under a specific lock, but for now, the per-device lock seems to be enough. Signed-off-by: Boris Brezillon Reviewed-by: Liviu Dudau Reviewed-by: Rob Clark --- drivers/gpu/drm/drm_drv.c | 2 ++ drivers/gpu/drm/drm_gem.c | 49 ++++++++--------------------= ---- drivers/gpu/drm/msm/msm_drv.c | 11 ++++--- drivers/gpu/drm/msm/msm_drv.h | 7 ----- drivers/gpu/drm/msm/msm_gem.c | 32 ++++++++++----------- drivers/gpu/drm/msm/msm_gem_shrinker.c | 4 +-- drivers/gpu/drm/msm/msm_gem_submit.c | 6 ++-- drivers/gpu/drm/msm/msm_gem_vma.c | 12 ++++---- drivers/gpu/drm/msm/msm_ringbuffer.c | 6 ++-- drivers/gpu/drm/panthor/panthor_device.h | 3 -- drivers/gpu/drm/panthor/panthor_gem.c | 21 ++++++-------- drivers/gpu/drm/panthor/panthor_mmu.c | 29 ++++++++++--------- include/drm/drm_device.h | 7 +++++ include/drm/drm_gem.h | 20 ++++++------- 14 files changed, 88 insertions(+), 121 deletions(-) diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 985c283cf59f..675675480da4 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -697,6 +697,7 @@ static void drm_dev_init_release(struct drm_device *dev= , void *res) mutex_destroy(&dev->master_mutex); mutex_destroy(&dev->clientlist_mutex); mutex_destroy(&dev->filelist_mutex); + mutex_destroy(&dev->gem_lru_mutex); } =20 static int drm_dev_init(struct drm_device *dev, @@ -738,6 +739,7 @@ static int drm_dev_init(struct drm_device *dev, INIT_LIST_HEAD(&dev->vblank_event_list); =20 spin_lock_init(&dev->event_lock); + mutex_init(&dev->gem_lru_mutex); mutex_init(&dev->filelist_mutex); mutex_init(&dev->clientlist_mutex); mutex_init(&dev->master_mutex); diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index c85a39b8b163..a0e6668e93f2 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -1127,8 +1127,6 @@ drm_gem_lru_remove_locked(struct drm_gem_object *obj) void drm_gem_object_release(struct drm_gem_object *obj) { - struct drm_gem_lru *lru; - if (obj->filp) fput(obj->filp); =20 @@ -1136,30 +1134,7 @@ drm_gem_object_release(struct drm_gem_object *obj) =20 drm_gem_free_mmap_offset(obj); =20 - /* - * We do the lru !=3D NULL check without the lru->lock held, which - * means we might end up with a stale lru value by the time the - * lock is acquired. - * - * This is deemed safe because: - * 1. the LRU is assumed to outlive any GEM object it was attached - * (LRUs are usually bound to a drm_device). So even if obj->lru - * has become NULL, it still point to a valid object that can - * safely be dereferenced to get the lock. - * - * 2. all LRUs a GEM object might be attached to must share the same - * lock (lock that's usually part of the driver-specific device - * object), so taking the lock on the 'old' LRU is equivalent - * to taking it on the new one (if any) - */ - lru =3D obj->lru; - if (lru) { - guard(mutex)(lru->lock); - - /* Check a second time with the lock held to make sure we're - * not racing with the drm_gem_lru_remove_locked() call in - * drm_gem_lru_scan(). - */ + scoped_guard(mutex, &obj->dev->gem_lru_mutex) { if (obj->lru) drm_gem_lru_remove_locked(obj); } @@ -1582,9 +1557,8 @@ EXPORT_SYMBOL(drm_gem_unlock_reservations); * @lock: The lock protecting the LRU */ void -drm_gem_lru_init(struct drm_gem_lru *lru, struct mutex *lock) +drm_gem_lru_init(struct drm_gem_lru *lru) { - lru->lock =3D lock; lru->count =3D 0; INIT_LIST_HEAD(&lru->list); } @@ -1601,7 +1575,7 @@ EXPORT_SYMBOL(drm_gem_lru_init); void drm_gem_lru_move_tail_locked(struct drm_gem_lru *lru, struct drm_gem_objec= t *obj) { - lockdep_assert_held_once(lru->lock); + lockdep_assert_held_once(&obj->dev->gem_lru_mutex); =20 if (obj->lru) drm_gem_lru_remove_locked(obj); @@ -1625,9 +1599,9 @@ EXPORT_SYMBOL(drm_gem_lru_move_tail_locked); void drm_gem_lru_move_tail(struct drm_gem_lru *lru, struct drm_gem_object *obj) { - mutex_lock(lru->lock); + mutex_lock(&obj->dev->gem_lru_mutex); drm_gem_lru_move_tail_locked(lru, obj); - mutex_unlock(lru->lock); + mutex_unlock(&obj->dev->gem_lru_mutex); } EXPORT_SYMBOL(drm_gem_lru_move_tail); =20 @@ -1648,7 +1622,8 @@ EXPORT_SYMBOL(drm_gem_lru_move_tail); * @ticket: Optional ww_acquire_ctx context to use for locking */ unsigned long -drm_gem_lru_scan(struct drm_gem_lru *lru, +drm_gem_lru_scan(struct drm_device *dev, + struct drm_gem_lru *lru, unsigned int nr_to_scan, unsigned long *remaining, bool (*shrink)(struct drm_gem_object *obj, struct ww_acquire_ctx *ticke= t), @@ -1658,9 +1633,9 @@ drm_gem_lru_scan(struct drm_gem_lru *lru, struct drm_gem_object *obj; unsigned freed =3D 0; =20 - drm_gem_lru_init(&still_in_lru, lru->lock); + drm_gem_lru_init(&still_in_lru); =20 - mutex_lock(lru->lock); + mutex_lock(&dev->gem_lru_mutex); =20 while (freed < nr_to_scan) { obj =3D list_first_entry_or_null(&lru->list, typeof(*obj), lru_node); @@ -1685,7 +1660,7 @@ drm_gem_lru_scan(struct drm_gem_lru *lru, * rest of the loop body, to reduce contention with other * code paths that need the LRU lock */ - mutex_unlock(lru->lock); + mutex_unlock(&dev->gem_lru_mutex); =20 if (ticket) ww_acquire_init(ticket, &reservation_ww_class); @@ -1729,7 +1704,7 @@ drm_gem_lru_scan(struct drm_gem_lru *lru, =20 tail: drm_gem_object_put(obj); - mutex_lock(lru->lock); + mutex_lock(&dev->gem_lru_mutex); } =20 /* @@ -1741,7 +1716,7 @@ drm_gem_lru_scan(struct drm_gem_lru *lru, list_splice_tail(&still_in_lru.list, &lru->list); lru->count +=3D still_in_lru.count; =20 - mutex_unlock(lru->lock); + mutex_unlock(&dev->gem_lru_mutex); =20 return freed; } diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 195f40e331e5..cc2bcd14b1c2 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -128,11 +128,10 @@ static int msm_drm_init(struct device *dev, const str= uct drm_driver *drv, /* * Initialize the LRUs: */ - mutex_init(&priv->lru.lock); - drm_gem_lru_init(&priv->lru.unbacked, &priv->lru.lock); - drm_gem_lru_init(&priv->lru.pinned, &priv->lru.lock); - drm_gem_lru_init(&priv->lru.willneed, &priv->lru.lock); - drm_gem_lru_init(&priv->lru.dontneed, &priv->lru.lock); + drm_gem_lru_init(&priv->lru.unbacked); + drm_gem_lru_init(&priv->lru.pinned); + drm_gem_lru_init(&priv->lru.willneed); + drm_gem_lru_init(&priv->lru.dontneed); =20 /* Initialize stall-on-fault */ spin_lock_init(&priv->fault_stall_lock); @@ -140,7 +139,7 @@ static int msm_drm_init(struct device *dev, const struc= t drm_driver *drv, =20 /* Teach lockdep about lock ordering wrt. shrinker: */ fs_reclaim_acquire(GFP_KERNEL); - might_lock(&priv->lru.lock); + might_lock(&ddev->gem_lru_mutex); fs_reclaim_release(GFP_KERNEL); =20 if (priv->kms_init) { diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h index 76ac61df0b35..c3fb3205f683 100644 --- a/drivers/gpu/drm/msm/msm_drv.h +++ b/drivers/gpu/drm/msm/msm_drv.h @@ -150,13 +150,6 @@ struct msm_drm_private { * DONTNEED state (ie. can be purged) */ struct drm_gem_lru dontneed; - - /** - * lock: - * - * Protects manipulation of all of the LRUs. - */ - struct mutex lock; } lru; =20 struct notifier_block vmap_notifier; diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c index 2cb3ab04f125..070f5fc4bc17 100644 --- a/drivers/gpu/drm/msm/msm_gem.c +++ b/drivers/gpu/drm/msm/msm_gem.c @@ -177,11 +177,11 @@ static void update_lru_locked(struct drm_gem_object *= obj) =20 static void update_lru(struct drm_gem_object *obj) { - struct msm_drm_private *priv =3D obj->dev->dev_private; + struct drm_device *dev =3D obj->dev; =20 - mutex_lock(&priv->lru.lock); + mutex_lock(&dev->gem_lru_mutex); update_lru_locked(obj); - mutex_unlock(&priv->lru.lock); + mutex_unlock(&dev->gem_lru_mutex); } =20 static struct page **get_pages(struct drm_gem_object *obj) @@ -292,11 +292,11 @@ void msm_gem_pin_obj_locked(struct drm_gem_object *ob= j) =20 static void pin_obj_locked(struct drm_gem_object *obj) { - struct msm_drm_private *priv =3D obj->dev->dev_private; + struct drm_device *dev =3D obj->dev; =20 - mutex_lock(&priv->lru.lock); + mutex_lock(&dev->gem_lru_mutex); msm_gem_pin_obj_locked(obj); - mutex_unlock(&priv->lru.lock); + mutex_unlock(&dev->gem_lru_mutex); } =20 struct page **msm_gem_pin_pages_locked(struct drm_gem_object *obj) @@ -487,16 +487,16 @@ int msm_gem_pin_vma_locked(struct drm_gem_object *obj= , struct drm_gpuva *vma) =20 void msm_gem_unpin_locked(struct drm_gem_object *obj) { - struct msm_drm_private *priv =3D obj->dev->dev_private; + struct drm_device *dev =3D obj->dev; struct msm_gem_object *msm_obj =3D to_msm_bo(obj); =20 msm_gem_assert_locked(obj); =20 - mutex_lock(&priv->lru.lock); + mutex_lock(&dev->gem_lru_mutex); msm_obj->pin_count--; GEM_WARN_ON(msm_obj->pin_count < 0); update_lru_locked(obj); - mutex_unlock(&priv->lru.lock); + mutex_unlock(&dev->gem_lru_mutex); } =20 /* Special unpin path for use in fence-signaling path, avoiding the need @@ -507,10 +507,10 @@ void msm_gem_unpin_locked(struct drm_gem_object *obj) */ void msm_gem_unpin_active(struct drm_gem_object *obj) { - struct msm_drm_private *priv =3D obj->dev->dev_private; + struct drm_device *dev =3D obj->dev; struct msm_gem_object *msm_obj =3D to_msm_bo(obj); =20 - GEM_WARN_ON(!mutex_is_locked(&priv->lru.lock)); + GEM_WARN_ON(!mutex_is_locked(&dev->gem_lru_mutex)); =20 msm_obj->pin_count--; GEM_WARN_ON(msm_obj->pin_count < 0); @@ -797,12 +797,12 @@ void msm_gem_put_vaddr(struct drm_gem_object *obj) */ int msm_gem_madvise(struct drm_gem_object *obj, unsigned madv) { - struct msm_drm_private *priv =3D obj->dev->dev_private; + struct drm_device *dev =3D obj->dev; struct msm_gem_object *msm_obj =3D to_msm_bo(obj); =20 msm_gem_lock(obj); =20 - mutex_lock(&priv->lru.lock); + mutex_lock(&dev->gem_lru_mutex); =20 if (msm_obj->madv !=3D __MSM_MADV_PURGED) msm_obj->madv =3D madv; @@ -814,7 +814,7 @@ int msm_gem_madvise(struct drm_gem_object *obj, unsigne= d madv) */ update_lru_locked(obj); =20 - mutex_unlock(&priv->lru.lock); + mutex_unlock(&dev->gem_lru_mutex); =20 msm_gem_unlock(obj); =20 @@ -839,10 +839,10 @@ void msm_gem_purge(struct drm_gem_object *obj) =20 put_pages(obj); =20 - mutex_lock(&priv->lru.lock); + mutex_lock(&dev->gem_lru_mutex); /* A one-way transition: */ msm_obj->madv =3D __MSM_MADV_PURGED; - mutex_unlock(&priv->lru.lock); + mutex_unlock(&dev->gem_lru_mutex); =20 drm_gem_free_mmap_offset(obj); =20 diff --git a/drivers/gpu/drm/msm/msm_gem_shrinker.c b/drivers/gpu/drm/msm/m= sm_gem_shrinker.c index 31fa51a44f86..c07af9602fee 100644 --- a/drivers/gpu/drm/msm/msm_gem_shrinker.c +++ b/drivers/gpu/drm/msm/msm_gem_shrinker.c @@ -186,7 +186,7 @@ msm_gem_shrinker_scan(struct shrinker *shrinker, struct= shrink_control *sc) if (!stages[i].cond) continue; stages[i].freed =3D - drm_gem_lru_scan(stages[i].lru, nr, + drm_gem_lru_scan(priv->dev, stages[i].lru, nr, &stages[i].remaining, stages[i].shrink, &ticket); @@ -255,7 +255,7 @@ msm_gem_shrinker_vmap(struct notifier_block *nb, unsign= ed long event, void *ptr) unsigned long remaining =3D 0; =20 for (idx =3D 0; lrus[idx] && unmapped < vmap_shrink_limit; idx++) { - unmapped +=3D drm_gem_lru_scan(lrus[idx], + unmapped +=3D drm_gem_lru_scan(priv->dev, lrus[idx], vmap_shrink_limit - unmapped, &remaining, vmap_shrink, diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm= _gem_submit.c index 26ea8a28be47..3c6bc90c3d48 100644 --- a/drivers/gpu/drm/msm/msm_gem_submit.c +++ b/drivers/gpu/drm/msm/msm_gem_submit.c @@ -352,7 +352,7 @@ static int submit_fence_sync(struct msm_gem_submit *sub= mit) =20 static int submit_pin_objects(struct msm_gem_submit *submit) { - struct msm_drm_private *priv =3D submit->dev->dev_private; + struct drm_device *dev =3D submit->dev; int i, ret =3D 0; =20 for (i =3D 0; i < submit->nr_bos; i++) { @@ -381,11 +381,11 @@ static int submit_pin_objects(struct msm_gem_submit *= submit) * get_pages() which could trigger reclaim.. and if we held the LRU lock * could trigger deadlock with the shrinker). */ - mutex_lock(&priv->lru.lock); + mutex_lock(&dev->gem_lru_mutex); for (i =3D 0; i < submit->nr_bos; i++) { msm_gem_pin_obj_locked(submit->bos[i].obj); } - mutex_unlock(&priv->lru.lock); + mutex_unlock(&dev->gem_lru_mutex); =20 submit->bos_pinned =3D true; =20 diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_ge= m_vma.c index 271691ae32c3..3ed05ab0eeef 100644 --- a/drivers/gpu/drm/msm/msm_gem_vma.c +++ b/drivers/gpu/drm/msm/msm_gem_vma.c @@ -702,7 +702,7 @@ static struct dma_fence * msm_vma_job_run(struct drm_sched_job *_job) { struct msm_vm_bind_job *job =3D to_msm_vm_bind_job(_job); - struct msm_drm_private *priv =3D job->vm->drm->dev_private; + struct drm_device *dev =3D job->vm->drm; struct msm_gem_vm *vm =3D to_msm_vm(job->vm); struct drm_gem_object *obj; int ret =3D vm->unusable ? -EINVAL : 0; @@ -745,13 +745,13 @@ msm_vma_job_run(struct drm_sched_job *_job) if (ret) msm_gem_vm_unusable(job->vm); =20 - mutex_lock(&priv->lru.lock); + mutex_lock(&dev->gem_lru_mutex); =20 job_foreach_bo (obj, job) { msm_gem_unpin_active(obj); } =20 - mutex_unlock(&priv->lru.lock); + mutex_unlock(&dev->gem_lru_mutex); =20 /* VM_BIND ops are synchronous, so no fence to wait on: */ return NULL; @@ -1304,7 +1304,7 @@ vm_bind_job_pin_objects(struct msm_vm_bind_job *job) return PTR_ERR(pages); } =20 - struct msm_drm_private *priv =3D job->vm->drm->dev_private; + struct drm_device *dev =3D job->vm->drm; =20 /* * A second loop while holding the LRU lock (a) avoids acquiring/dropping @@ -1313,10 +1313,10 @@ vm_bind_job_pin_objects(struct msm_vm_bind_job *job) * get_pages() which could trigger reclaim.. and if we held the LRU lock * could trigger deadlock with the shrinker). */ - mutex_lock(&priv->lru.lock); + mutex_lock(&dev->gem_lru_mutex); job_foreach_bo (obj, job) msm_gem_pin_obj_locked(obj); - mutex_unlock(&priv->lru.lock); + mutex_unlock(&dev->gem_lru_mutex); =20 job->bos_pinned =3D true; =20 diff --git a/drivers/gpu/drm/msm/msm_ringbuffer.c b/drivers/gpu/drm/msm/msm= _ringbuffer.c index a7dafa7ab4b1..0d14c31bd4e4 100644 --- a/drivers/gpu/drm/msm/msm_ringbuffer.c +++ b/drivers/gpu/drm/msm/msm_ringbuffer.c @@ -16,13 +16,13 @@ static struct dma_fence *msm_job_run(struct drm_sched_j= ob *job) struct msm_gem_submit *submit =3D to_msm_submit(job); struct msm_fence_context *fctx =3D submit->ring->fctx; struct msm_gpu *gpu =3D submit->gpu; - struct msm_drm_private *priv =3D gpu->dev->dev_private; + struct drm_device *dev =3D gpu->dev; unsigned nr_cmds =3D submit->nr_cmds; int i; =20 msm_fence_init(submit->hw_fence, fctx); =20 - mutex_lock(&priv->lru.lock); + mutex_lock(&dev->gem_lru_mutex); =20 for (i =3D 0; i < submit->nr_bos; i++) { struct drm_gem_object *obj =3D submit->bos[i].obj; @@ -32,7 +32,7 @@ static struct dma_fence *msm_job_run(struct drm_sched_job= *job) =20 submit->bos_pinned =3D false; =20 - mutex_unlock(&priv->lru.lock); + mutex_unlock(&dev->gem_lru_mutex); =20 /* TODO move submit path over to using a per-ring lock.. */ mutex_lock(&gpu->lock); diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/pan= thor/panthor_device.h index dcdce75b683b..cc5720312fa9 100644 --- a/drivers/gpu/drm/panthor/panthor_device.h +++ b/drivers/gpu/drm/panthor/panthor_device.h @@ -187,9 +187,6 @@ struct panthor_device { /** @reclaim.shrinker: Shrinker instance */ struct shrinker *shrinker; =20 - /** @reclaim.lock: Lock protecting all LRUs */ - struct mutex lock; - /** * @reclaim.unreclaimable: unreclaimable BOs * diff --git a/drivers/gpu/drm/panthor/panthor_gem.c b/drivers/gpu/drm/pantho= r/panthor_gem.c index 8e31740126e7..450516d55faa 100644 --- a/drivers/gpu/drm/panthor/panthor_gem.c +++ b/drivers/gpu/drm/panthor/panthor_gem.c @@ -1497,13 +1497,13 @@ panthor_gem_shrinker_scan(struct shrinker *shrinker= , struct shrink_control *sc) if (!can_swap()) goto out; =20 - freed +=3D drm_gem_lru_scan(&ptdev->reclaim.unused, + freed +=3D drm_gem_lru_scan(&ptdev->base, &ptdev->reclaim.unused, sc->nr_to_scan - freed, &remaining, panthor_gem_try_evict_no_resv_wait, NULL); if (freed >=3D sc->nr_to_scan) goto out; =20 - freed +=3D drm_gem_lru_scan(&ptdev->reclaim.mmapped, + freed +=3D drm_gem_lru_scan(&ptdev->base, &ptdev->reclaim.mmapped, sc->nr_to_scan - freed, &remaining, panthor_gem_try_evict_no_resv_wait, NULL); if (freed >=3D sc->nr_to_scan) @@ -1517,7 +1517,7 @@ panthor_gem_shrinker_scan(struct shrinker *shrinker, = struct shrink_control *sc) if (freed >=3D sc->nr_to_scan) goto out; =20 - freed +=3D drm_gem_lru_scan(&ptdev->reclaim.gpu_mapped_shared, + freed +=3D drm_gem_lru_scan(&ptdev->base, &ptdev->reclaim.gpu_mapped_shar= ed, sc->nr_to_scan - freed, &remaining, panthor_gem_try_evict, NULL); =20 @@ -1546,22 +1546,17 @@ panthor_gem_shrinker_scan(struct shrinker *shrinker= , struct shrink_control *sc) int panthor_gem_shrinker_init(struct panthor_device *ptdev) { struct shrinker *shrinker; - int ret; - - ret =3D drmm_mutex_init(&ptdev->base, &ptdev->reclaim.lock); - if (ret) - return ret; =20 INIT_LIST_HEAD(&ptdev->reclaim.vms); - drm_gem_lru_init(&ptdev->reclaim.unreclaimable, &ptdev->reclaim.lock); - drm_gem_lru_init(&ptdev->reclaim.unused, &ptdev->reclaim.lock); - drm_gem_lru_init(&ptdev->reclaim.mmapped, &ptdev->reclaim.lock); - drm_gem_lru_init(&ptdev->reclaim.gpu_mapped_shared, &ptdev->reclaim.lock); + drm_gem_lru_init(&ptdev->reclaim.unreclaimable); + drm_gem_lru_init(&ptdev->reclaim.unused); + drm_gem_lru_init(&ptdev->reclaim.mmapped); + drm_gem_lru_init(&ptdev->reclaim.gpu_mapped_shared); ptdev->reclaim.gpu_mapped_count =3D 0; =20 /* Teach lockdep about lock ordering wrt. shrinker: */ fs_reclaim_acquire(GFP_KERNEL); - might_lock(&ptdev->reclaim.lock); + might_lock(&ptdev->base.gem_lru_mutex); fs_reclaim_release(GFP_KERNEL); =20 shrinker =3D shrinker_alloc(0, "drm-panthor-gem"); diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/pantho= r/panthor_mmu.c index 452d0b6d4668..9d4500850561 100644 --- a/drivers/gpu/drm/panthor/panthor_mmu.c +++ b/drivers/gpu/drm/panthor/panthor_mmu.c @@ -715,10 +715,10 @@ int panthor_vm_active(struct panthor_vm *vm) * never became active in the first place will be reclaimed last, but * that's an acceptable trade-off. */ - mutex_lock(&ptdev->reclaim.lock); + mutex_lock(&ptdev->base.gem_lru_mutex); if (vm->reclaim.lru.count) list_move_tail(&vm->reclaim.lru_node, &ptdev->reclaim.vms); - mutex_unlock(&ptdev->reclaim.lock); + mutex_unlock(&ptdev->base.gem_lru_mutex); =20 /* Make sure we don't race with lock/unlock_region() calls * happening around VM bind operations. @@ -1962,9 +1962,9 @@ static void panthor_vm_free(struct drm_gpuvm *gpuvm) struct panthor_vm *vm =3D container_of(gpuvm, struct panthor_vm, base); struct panthor_device *ptdev =3D vm->ptdev; =20 - mutex_lock(&ptdev->reclaim.lock); + mutex_lock(&ptdev->base.gem_lru_mutex); list_del_init(&vm->reclaim.lru_node); - mutex_unlock(&ptdev->reclaim.lock); + mutex_unlock(&ptdev->base.gem_lru_mutex); =20 mutex_lock(&vm->heaps.lock); if (drm_WARN_ON(&ptdev->base, vm->heaps.pool)) @@ -2360,11 +2360,11 @@ void panthor_vm_update_bo_reclaim_lru_locked(struct= panthor_gem_object *bo) drm_WARN_ON(&ptdev->base, vm); vm =3D container_of(vm_bo->vm, struct panthor_vm, base); =20 - mutex_lock(&ptdev->reclaim.lock); + mutex_lock(&ptdev->base.gem_lru_mutex); drm_gem_lru_move_tail_locked(&vm->reclaim.lru, &bo->base); if (list_empty(&vm->reclaim.lru_node)) list_move(&vm->reclaim.lru_node, &ptdev->reclaim.vms); - mutex_unlock(&ptdev->reclaim.lock); + mutex_unlock(&ptdev->base.gem_lru_mutex); } } =20 @@ -2774,7 +2774,7 @@ panthor_vm_create(struct panthor_device *ptdev, bool = for_mcu, vm->kernel_auto_va.start =3D auto_kernel_va_start; vm->kernel_auto_va.end =3D vm->kernel_auto_va.start + auto_kernel_va_size= - 1; =20 - drm_gem_lru_init(&vm->reclaim.lru, &ptdev->reclaim.lock); + drm_gem_lru_init(&vm->reclaim.lru); INIT_LIST_HEAD(&vm->reclaim.lru_node); INIT_LIST_HEAD(&vm->node); INIT_LIST_HEAD(&vm->as.lru_node); @@ -3140,7 +3140,7 @@ panthor_mmu_reclaim_priv_bos(struct panthor_device *p= tdev, LIST_HEAD(remaining_vms); LIST_HEAD(vms); =20 - mutex_lock(&ptdev->reclaim.lock); + mutex_lock(&ptdev->base.gem_lru_mutex); list_splice_init(&ptdev->reclaim.vms, &vms); =20 while (freed < nr_to_scan) { @@ -3156,12 +3156,13 @@ panthor_mmu_reclaim_priv_bos(struct panthor_device = *ptdev, continue; } =20 - mutex_unlock(&ptdev->reclaim.lock); + mutex_unlock(&ptdev->base.gem_lru_mutex); =20 - freed +=3D drm_gem_lru_scan(&vm->reclaim.lru, nr_to_scan - freed, + freed +=3D drm_gem_lru_scan(&ptdev->base, &vm->reclaim.lru, + nr_to_scan - freed, remaining, shrink, NULL); =20 - mutex_lock(&ptdev->reclaim.lock); + mutex_lock(&ptdev->base.gem_lru_mutex); =20 /* If the VM is still in the temporary list, remove it so we * can proceed with the next VM. @@ -3177,11 +3178,11 @@ panthor_mmu_reclaim_priv_bos(struct panthor_device = *ptdev, list_add_tail(&vm->reclaim.lru_node, &remaining_vms); } =20 - mutex_unlock(&ptdev->reclaim.lock); + mutex_unlock(&ptdev->base.gem_lru_mutex); =20 panthor_vm_put(vm); =20 - mutex_lock(&ptdev->reclaim.lock); + mutex_lock(&ptdev->base.gem_lru_mutex); } =20 /* Re-insert VMs with remaining data to reclaim at the beginning of @@ -3192,7 +3193,7 @@ panthor_mmu_reclaim_priv_bos(struct panthor_device *p= tdev, */ list_splice_tail(&vms, &remaining_vms); list_splice(&remaining_vms, &ptdev->reclaim.vms); - mutex_unlock(&ptdev->reclaim.lock); + mutex_unlock(&ptdev->base.gem_lru_mutex); =20 return freed; } diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h index bc78fb77cc27..768a8dae83c5 100644 --- a/include/drm/drm_device.h +++ b/include/drm/drm_device.h @@ -375,6 +375,13 @@ struct drm_device { * Root directory for debugfs files. */ struct dentry *debugfs_root; + + /** + * @gem_lru_mutex: + * + * Lock protecting movement of GEM objects between LRUs. + */ + struct mutex gem_lru_mutex; }; =20 void drm_dev_set_dma_dev(struct drm_device *dev, struct device *dma_dev); diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h index d527df98d142..dd1a9cd559be 100644 --- a/include/drm/drm_gem.h +++ b/include/drm/drm_gem.h @@ -245,17 +245,11 @@ struct drm_gem_object_funcs { * for lockless &shrinker.count_objects, and provides * &drm_gem_lru_scan for driver's &shrinker.scan_objects * implementation. + * + * Any access to this kind of object must be done with + * drm_device::gem_lru_mutex held. */ struct drm_gem_lru { - /** - * @lock: - * - * Lock protecting movement of GEM objects between LRUs. All - * LRUs that the object can move between should be protected - * by the same lock. - */ - struct mutex *lock; - /** * @count: * @@ -453,6 +447,9 @@ struct drm_gem_object { * @lru: * * The current LRU list that the GEM object is on. + * + * Access to this field must be done with drm_device::gem_lru_mutex + * held. */ struct drm_gem_lru *lru; }; @@ -610,11 +607,12 @@ void drm_gem_unlock_reservations(struct drm_gem_objec= t **objs, int count, int drm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev, u32 handle, u64 *offset); =20 -void drm_gem_lru_init(struct drm_gem_lru *lru, struct mutex *lock); +void drm_gem_lru_init(struct drm_gem_lru *lru); void drm_gem_lru_move_tail_locked(struct drm_gem_lru *lru, struct drm_gem_= object *obj); void drm_gem_lru_move_tail(struct drm_gem_lru *lru, struct drm_gem_object = *obj); unsigned long -drm_gem_lru_scan(struct drm_gem_lru *lru, +drm_gem_lru_scan(struct drm_device *dev, + struct drm_gem_lru *lru, unsigned int nr_to_scan, unsigned long *remaining, bool (*shrink)(struct drm_gem_object *obj, struct ww_acquire_ctx *ticke= t), --=20 2.54.0