From nobody Wed Sep 10 05:43:41 2025 Received: from mail-wm1-f74.google.com (mail-wm1-f74.google.com [209.85.128.74]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id CE9CE34A334 for ; Fri, 5 Sep 2025 12:11:38 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.74 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1757074300; cv=none; b=RiVBpIEoZz7665uT/yxdVnPP+1HcI8O2LWIAn1FywG0XmzWaXbi8nhZ6sf/0wQnX64mfu1fCudTIn0sXQvXbfJMLKZmERib+58pjd3mKCp6/QDbXotaDLlF2k9DCtYoVR/KztHZCr3Qox8d6WEssflZ7zm4h4bFVgd68mzScR6o= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1757074300; c=relaxed/simple; bh=52WoN1Pmy2AcHTRvbpGs0/D/mgk8eznKWIWJ1Q2Pcg4=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Cc:Content-Type; b=j4kPZCKnKoFNnPCNv0vJuSxIBnAZd2P/Mtxlh3OnVx87S+aPTd3KfLaXJ6sm9qKZ1HNtv1aV1mI5DoNelOl3fkyK7133IJzx5fABPw4Vuz6J0b5edYey3gpSIqU/G/PK9Di6nIbCHeTX8tjEbONl5TjapADHn9xTB9xXY5EEHk0= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--aliceryhl.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=zRhkz7SY; arc=none smtp.client-ip=209.85.128.74 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--aliceryhl.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="zRhkz7SY" Received: by mail-wm1-f74.google.com with SMTP id 5b1f17b1804b1-45a15f10f31so20023705e9.0 for ; Fri, 05 Sep 2025 05:11:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1757074297; x=1757679097; darn=vger.kernel.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=F/D7fTGTthHcMtHgbv0id3peMuyGmzlJ6a/8BhjfUeI=; b=zRhkz7SYWR2r67nn2p9xVLJO+chaZS+CNQDTQzbsL+z3o4szefiHwNty1LDcbrIOJ2 fkSLeRERxSmYpSZPt19DYBS7HTbUkYfUHPvN4K5O4Fu7H2FSjqcsR+Aou6u09ZwTmE1i pgwxx5DnUmQPTMvyVepi3fV3X0kC7JTcHx3DutLEYWKkWqpfwgGaida4OVL+BO5BY1wd Ya6d2LQz3BwUJNldqUan3a6MCjJ9+ZEOHq/8VK6InegYMKbLKFoBrqVCCaSFOArDqudA nAdZBegqTq6u/4BR+RENGUmaQRSW8z1kr5zt1xYz1m0Uh1EyCYyJ+hOtdN/5rSmo9vOv kB0A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1757074297; x=1757679097; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=F/D7fTGTthHcMtHgbv0id3peMuyGmzlJ6a/8BhjfUeI=; b=LvHe7FySMAUGmSpaS5qTuYpPtzklK+E1pXZuviQrEugIjoJsFut2Dj+LcH/j8saqBG leObFl4wBZw3+uOyWEd2L1Jv5XLlzT8FXZ7OzipOTIItrWIdPohNxt/sYnJUI3KpW2AC OqAl3YNp0+SsheW/Mzg2KdiEyJpdy/bQaa6GB70P1LMUB5K7pA4Uoy3dL3lI/h5Mv8Fi ONZQblgvNUdUG4Ki+Xhv3alU8D+tGQdk+OMQbnexUrTISQTBEAA7eOJY4UWbdwG9ETxR CZrqBFJFpmLbpK30ODVY1sXKo4/0HzRqKsPnvy3WdAQRLiraQkdNX9q4Q1b3rchLiU/C QLmg== X-Forwarded-Encrypted: i=1; AJvYcCVnHGcm77Aov/RDV/i7SKKN5ae3swSkIW7jf59Hjjg5p05O4VzgFlmHSqr5ZKRbqA0M0FeR40uvaZwBMJU=@vger.kernel.org X-Gm-Message-State: AOJu0Yz1OQSO9lcmX2gDXyXBMyQLgy9JJnB0lniS0P6G4gJ3HEEf0Qd+ /PlmjZTUAoiWk0J0ftzTwznFXnf+2uAcWVMCUjfUhDY5eCnYA5UmiLe8SV45O5cJL9XwaISLGqP XEyw1i5yrDaZcCrA0dg== X-Google-Smtp-Source: AGHT+IHFbrkfCQFcNk6bb2vCkoEy+Jo+99SJ0Tqqy456cPB0dsshgXKqM8xPJSaYDimaeBgCaAT6wFW4fL0Ztw0= X-Received: from wrvl7.prod.google.com ([2002:a5d:5607:0:b0:3dc:3108:9843]) (user=aliceryhl job=prod-delivery.src-stubby-dispatcher) by 2002:adf:e54a:0:b0:3d3:6525:e320 with SMTP id ffacd0b85a97d-3e304081e46mr2546666f8f.29.1757074297183; Fri, 05 Sep 2025 05:11:37 -0700 (PDT) Date: Fri, 05 Sep 2025 12:11:28 +0000 In-Reply-To: <20250905-vmbo-defer-v1-0-7ae1a382b674@google.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20250905-vmbo-defer-v1-0-7ae1a382b674@google.com> X-Developer-Key: i=aliceryhl@google.com; a=openpgp; fpr=49F6C1FAA74960F43A5B86A1EE7A392FDE96209F X-Developer-Signature: v=1; a=openpgp-sha256; l=10160; i=aliceryhl@google.com; h=from:subject:message-id; bh=52WoN1Pmy2AcHTRvbpGs0/D/mgk8eznKWIWJ1Q2Pcg4=; b=owEBbQKS/ZANAwAKAQRYvu5YxjlGAcsmYgBoutN2OiuZdQO4gp5wu1ljn6MvQsNIGoViW/w4U RZyLKQ5AYuJAjMEAAEKAB0WIQSDkqKUTWQHCvFIvbIEWL7uWMY5RgUCaLrTdgAKCRAEWL7uWMY5 RgFjEACvjAx+tNXXFceeuwIqU31O8s/mpB4mbeFsSl4CCDlIN653zQ4ptgdjjAvEqUtyAaVSx5t mhdV25A7NbxJLBHXFKzsc/4gnKep3lg9S6xo9Kw3uiRCVcEYawvnmGb6BqEP1yvG/LJqgquwjR/ JJmO0BFuYbRwezEYGi66bG3Yqnuu34wEoqO4xF8OMPbpX9f1G82DGwnIwynuSHbA/yzmwiaul29 bnCK7b3rtGIdXig3V/rLr07kyumjD2w0hGcgdcaIE9TLdGFOOm4lokPLVAxO/Tr5lLT4+4Ae2hq E+oEpSvHSHfFJcNgvutlVLFxwe5KbbklCHHMr2N8NJQMpcFqYroUoFs6X16Px0tJ20IQLc6ZT9v BHadwmO5rbN+535q3pCHYBGkVskIZkhbP0i1HgtEfhB4sg+EtuaJZrZ4LCNaLCvavn8u2eb6zHh gdZuWFlTJ846eHTOTqJf0B4110waR690rF2A+7yhkNCx5cgYdRH2cVs1ds//Jd8pslFIVyuV9VO ZB2D07DlJn0TzqCNmU/4hvt+P1CraMup3MZT8bQTBdlIlrFux7cQlZkcjSHQ01ZFwsP3X9pRzes cVoaQnEy5Ch6TFov9ywTY2zKWbGfW4TlpdHrAUVcXqg59Y4DPaazD3Gy6F8GvZkWF3kC5uXgB6x Yuz8Dh1/dl4uytA== X-Mailer: b4 0.14.2 Message-ID: <20250905-vmbo-defer-v1-1-7ae1a382b674@google.com> Subject: [PATCH 1/2] drm/gpuvm: add deferred vm_bo cleanup From: Alice Ryhl To: Danilo Krummrich , Matthew Brost , "=?utf-8?q?Thomas_Hellstr=C3=B6m?=" Cc: Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Simona Vetter , Boris Brezillon , Steven Price , Daniel Almeida , Liviu Dudau , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org, Alice Ryhl Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable When using GPUVM in immediate mode, it is necessary to call drm_gpuvm_unlink() from the fence signalling critical path. However, unlink may call drm_gpuvm_bo_put(), which causes some challenges: 1. drm_gpuvm_bo_put() often requires you to take resv locks, which you can't do from the fence signalling critical path. 2. drm_gpuvm_bo_put() calls drm_gem_object_put(), which is often going to be unsafe to call from the fence signalling critical path. To solve these issues, add a deferred version of drm_gpuvm_unlink() that adds the vm_bo to a deferred cleanup list, and then clean it up later. The new methods take the GEMs GPUVA lock internally rather than letting the caller do it because it also needs to perform an operation after releasing the mutex again. This is to prevent freeing the GEM while holding the mutex (more info as comments in the patch). This means that the new methods can only be used with DRM_GPUVM_IMMEDIATE_MODE. Signed-off-by: Alice Ryhl --- drivers/gpu/drm/drm_gpuvm.c | 167 ++++++++++++++++++++++++++++++++++++++++= ++++ include/drm/drm_gpuvm.h | 26 +++++++ 2 files changed, 193 insertions(+) diff --git a/drivers/gpu/drm/drm_gpuvm.c b/drivers/gpu/drm/drm_gpuvm.c index 78a1a4f095095e9379bdf604d583f6c8b9863ccb..849b6c08f83dcba832eda372bd3= ce62b540e144b 100644 --- a/drivers/gpu/drm/drm_gpuvm.c +++ b/drivers/gpu/drm/drm_gpuvm.c @@ -876,6 +876,25 @@ __drm_gpuvm_bo_list_add(struct drm_gpuvm *gpuvm, spinl= ock_t *lock, cond_spin_unlock(lock, !!lock); } =20 +/** + * drm_gpuvm_bo_is_dead() - check whether this vm_bo is scheduled for clea= nup + * @vm_bo: the &drm_gpuvm_bo + * + * When a vm_bo is scheduled for cleanup using the bo_defer list, it is not + * immediately removed from the evict and extobj lists. Therefore, anyone + * iterating these lists should skip entries that are being destroyed. + * + * Checking the refcount without incrementing it is okay as long as the lo= ck + * protecting the evict/extobj list is held for as long as you are using t= he + * vm_bo, because even if the refcount hits zero while you are using it, f= reeing + * the vm_bo requires taking the list's lock. + */ +static bool +drm_gpuvm_bo_is_dead(struct drm_gpuvm_bo *vm_bo) +{ + return !kref_read(&vm_bo->kref); +} + /** * drm_gpuvm_bo_list_add() - insert a vm_bo into the given list * @__vm_bo: the &drm_gpuvm_bo @@ -1081,6 +1100,9 @@ drm_gpuvm_init(struct drm_gpuvm *gpuvm, const char *n= ame, INIT_LIST_HEAD(&gpuvm->evict.list); spin_lock_init(&gpuvm->evict.lock); =20 + INIT_LIST_HEAD(&gpuvm->bo_defer.list); + spin_lock_init(&gpuvm->bo_defer.lock); + kref_init(&gpuvm->kref); =20 gpuvm->name =3D name ? name : "unknown"; @@ -1122,6 +1144,8 @@ drm_gpuvm_fini(struct drm_gpuvm *gpuvm) "Extobj list should be empty.\n"); drm_WARN(gpuvm->drm, !list_empty(&gpuvm->evict.list), "Evict list should be empty.\n"); + drm_WARN(gpuvm->drm, !list_empty(&gpuvm->bo_defer.list), + "VM BO cleanup list should be empty.\n"); =20 drm_gem_object_put(gpuvm->r_obj); } @@ -1217,6 +1241,9 @@ drm_gpuvm_prepare_objects_locked(struct drm_gpuvm *gp= uvm, =20 drm_gpuvm_resv_assert_held(gpuvm); list_for_each_entry(vm_bo, &gpuvm->extobj.list, list.entry.extobj) { + if (drm_gpuvm_bo_is_dead(vm_bo)) + continue; + ret =3D exec_prepare_obj(exec, vm_bo->obj, num_fences); if (ret) break; @@ -1460,6 +1487,9 @@ drm_gpuvm_validate_locked(struct drm_gpuvm *gpuvm, st= ruct drm_exec *exec) =20 list_for_each_entry_safe(vm_bo, next, &gpuvm->evict.list, list.entry.evict) { + if (drm_gpuvm_bo_is_dead(vm_bo)) + continue; + ret =3D ops->vm_bo_validate(vm_bo, exec); if (ret) break; @@ -1560,6 +1590,7 @@ drm_gpuvm_bo_create(struct drm_gpuvm *gpuvm, =20 INIT_LIST_HEAD(&vm_bo->list.entry.extobj); INIT_LIST_HEAD(&vm_bo->list.entry.evict); + INIT_LIST_HEAD(&vm_bo->list.entry.bo_defer); =20 return vm_bo; } @@ -1621,6 +1652,106 @@ drm_gpuvm_bo_put(struct drm_gpuvm_bo *vm_bo) } EXPORT_SYMBOL_GPL(drm_gpuvm_bo_put); =20 +static void +drm_gpuvm_bo_defer_locked(struct kref *kref) +{ + struct drm_gpuvm_bo *vm_bo =3D container_of(kref, struct drm_gpuvm_bo, + kref); + struct drm_gpuvm *gpuvm =3D vm_bo->vm; + + if (!drm_gpuvm_resv_protected(gpuvm)) { + drm_gpuvm_bo_list_del(vm_bo, extobj, true); + drm_gpuvm_bo_list_del(vm_bo, evict, true); + } + + list_del(&vm_bo->list.entry.gem); + mutex_unlock(&vm_bo->obj->gpuva.lock); +} + +/** + * drm_gpuvm_bo_put_deferred() - drop a struct drm_gpuvm_bo reference with + * deferred cleanup + * @vm_bo: the &drm_gpuvm_bo to release the reference of + * + * This releases a reference to @vm_bo. + * + * This might take and release the GEMs GPUVA lock. You should call + * drm_gpuvm_bo_deferred_cleanup() later to complete the cleanup process. + * + * Returns: true if vm_bo is being destroyed, false otherwise. + */ +bool +drm_gpuvm_bo_put_deferred(struct drm_gpuvm_bo *vm_bo) +{ + bool defer; + + drm_WARN_ON(vm_bo->vm->drm, !drm_gpuvm_immediate_mode(vm_bo->vm)); + + if (!vm_bo) + return false; + + defer =3D kref_put_mutex(&vm_bo->kref, drm_gpuvm_bo_defer_locked, + &vm_bo->obj->gpuva.lock); + + /* + * It's important that the GEM stays alive for the duration in which + * drm_gpuvm_bo_defer_locked() holds the mutex, but the instant we add + * the vm_bo to bo_defer, another thread might call + * drm_gpuvm_bo_deferred_cleanup() and put the GEM. For this reason, we + * add the vm_bo to bo_defer after releasing the GEM's mutex. + */ + if (defer) + drm_gpuvm_bo_list_add(vm_bo, bo_defer, true); + + return defer; +} +EXPORT_SYMBOL_GPL(drm_gpuvm_bo_put_deferred); + +/** + * drm_gpuvm_bo_deferred_cleanup() - clean up BOs in the deferred list + * deferred cleanup + * @gpuvm: the VM to clean up + * + * Cleans up &drm_gpuvm_bo instances in the deferred cleanup list. + */ +void +drm_gpuvm_bo_deferred_cleanup(struct drm_gpuvm *gpuvm) +{ + const struct drm_gpuvm_ops *ops =3D gpuvm->ops; + struct drm_gpuvm_bo *vm_bo; + struct drm_gem_object *obj; + LIST_HEAD(bo_defer); + + spin_lock(&gpuvm->bo_defer.lock); + list_replace_init(&gpuvm->bo_defer.list, &bo_defer); + spin_unlock(&gpuvm->bo_defer.lock); + + if (drm_gpuvm_resv_protected(gpuvm)) { + dma_resv_lock(drm_gpuvm_resv(gpuvm), NULL); + list_for_each_entry(vm_bo, &bo_defer, list.entry.bo_defer) { + drm_gpuvm_bo_list_del(vm_bo, extobj, false); + drm_gpuvm_bo_list_del(vm_bo, evict, false); + } + dma_resv_unlock(drm_gpuvm_resv(gpuvm)); + } + + while (!list_empty(&bo_defer)) { + vm_bo =3D list_first_entry(&bo_defer, + struct drm_gpuvm_bo, list.entry.bo_defer); + obj =3D vm_bo->obj; + + list_del(&vm_bo->list.entry.bo_defer); + if (ops && ops->vm_bo_free) + ops->vm_bo_free(vm_bo); + else + kfree(vm_bo); + + drm_gpuvm_put(gpuvm); + drm_gem_object_put(obj); + } +} +EXPORT_SYMBOL_GPL(drm_gpuvm_bo_deferred_cleanup); + static struct drm_gpuvm_bo * __drm_gpuvm_bo_find(struct drm_gpuvm *gpuvm, struct drm_gem_object *obj) @@ -1948,6 +2079,42 @@ drm_gpuva_unlink(struct drm_gpuva *va) } EXPORT_SYMBOL_GPL(drm_gpuva_unlink); =20 +/** + * drm_gpuva_unlink_defer() - unlink a &drm_gpuva with deferred vm_bo clea= nup + * @va: the &drm_gpuva to unlink + * + * Similar to drm_gpuva_unlink(), but uses drm_gpuvm_bo_put_deferred() and= takes + * the lock for the caller. + */ +void +drm_gpuva_unlink_defer(struct drm_gpuva *va) +{ + struct drm_gem_object *obj =3D va->gem.obj; + struct drm_gpuvm_bo *vm_bo =3D va->vm_bo; + + if (unlikely(!obj)) + return; + + drm_WARN_ON(vm_bo->vm->drm, !drm_gpuvm_immediate_mode(vm_bo->vm)); + + mutex_lock(&obj->gpuva.lock); + list_del_init(&va->gem.entry); + + /* + * This is drm_gpuvm_bo_put_deferred() slightly modified since we + * already hold the mutex. It's important that we add the vm_bo to + * bo_defer after releasing the mutex for the same reason as in + * drm_gpuvm_bo_put_deferred(). + */ + if (kref_put(&vm_bo->kref, drm_gpuvm_bo_defer_locked)) + drm_gpuvm_bo_list_add(vm_bo, bo_defer, true); + else + mutex_unlock(&obj->gpuva.lock); + + va->vm_bo =3D NULL; +} +EXPORT_SYMBOL_GPL(drm_gpuva_unlink_defer); + /** * drm_gpuva_find_first() - find the first &drm_gpuva in the given range * @gpuvm: the &drm_gpuvm to search in diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h index 727b8f336fad0d853998e4379cbd374155468e18..1f80389e14312f552a8abc95d12= f52f83beb9be8 100644 --- a/include/drm/drm_gpuvm.h +++ b/include/drm/drm_gpuvm.h @@ -152,6 +152,7 @@ void drm_gpuva_remove(struct drm_gpuva *va); =20 void drm_gpuva_link(struct drm_gpuva *va, struct drm_gpuvm_bo *vm_bo); void drm_gpuva_unlink(struct drm_gpuva *va); +void drm_gpuva_unlink_defer(struct drm_gpuva *va); =20 struct drm_gpuva *drm_gpuva_find(struct drm_gpuvm *gpuvm, u64 addr, u64 range); @@ -331,6 +332,22 @@ struct drm_gpuvm { */ spinlock_t lock; } evict; + + /** + * @bo_defer: structure holding vm_bos that need to be destroyed + */ + struct { + /** + * @bo_defer.list: &list_head storing &drm_gpuvm_bos that need + * to be destroyed + */ + struct list_head list; + + /** + * @bo_defer.lock: spinlock to protect the bo_defer list + */ + spinlock_t lock; + } bo_defer; }; =20 void drm_gpuvm_init(struct drm_gpuvm *gpuvm, const char *name, @@ -714,6 +731,12 @@ struct drm_gpuvm_bo { * &drm_gpuvms evict list. */ struct list_head evict; + + /** + * @list.entry.bo_defer: List entry to attach to + * the &drm_gpuvms bo_defer list. + */ + struct list_head bo_defer; } entry; } list; }; @@ -746,6 +769,9 @@ drm_gpuvm_bo_get(struct drm_gpuvm_bo *vm_bo) =20 bool drm_gpuvm_bo_put(struct drm_gpuvm_bo *vm_bo); =20 +bool drm_gpuvm_bo_put_deferred(struct drm_gpuvm_bo *vm_bo); +void drm_gpuvm_bo_deferred_cleanup(struct drm_gpuvm *gpuvm); + struct drm_gpuvm_bo * drm_gpuvm_bo_find(struct drm_gpuvm *gpuvm, struct drm_gem_object *obj); --=20 2.51.0.355.g5224444f11-goog From nobody Wed Sep 10 05:43:41 2025 Received: from mail-wr1-f73.google.com (mail-wr1-f73.google.com [209.85.221.73]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id EC03234AB04 for ; Fri, 5 Sep 2025 12:11:39 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.73 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1757074301; cv=none; b=bXvSgOkjKOS6yGeXvfwddCrv3yx7O11q8UpCFlxtQKzRuaND3KmTqv3NBoPUDsGGr8aFgQB0TySUWm0MdMuseMDnivnphFFqJJaMxD+B88Q2Ik7pdQmKiIiIQM5425wYLWgDvb6XysgSYvvU3pSi1f4e7U3i28UbTMIiWTK5J7k= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1757074301; c=relaxed/simple; bh=s2GWvmQHUwyjohU1GP99f3t5sldgnhLD+nr53UBS7jk=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Cc:Content-Type; b=GRq/QIL5Xeb70lB9odJOZxcRq0B6rFPKFdNE9y8kdtK4GYUEm9JwwbkABJOPy09uLk5S0xMXyAsjFfWUnPXD8sjzoGbAMVJ8XpH2sjUaoFH2hWMUqiJbzt6gMkd6bfFlG5QwvAVkhFqo05oF/XZWBxlVTnnx2dXPvnFz4UrPmAY= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--aliceryhl.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=WKd3WOvV; arc=none smtp.client-ip=209.85.221.73 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--aliceryhl.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="WKd3WOvV" Received: by mail-wr1-f73.google.com with SMTP id ffacd0b85a97d-3db89e4f443so1250466f8f.1 for ; Fri, 05 Sep 2025 05:11:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1757074298; x=1757679098; darn=vger.kernel.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=rlxAeSfJx82pksd3I0ixRoBKWGI7viD6A795f500XFc=; b=WKd3WOvV/fX9A/edpAjQmhgiha+Y5JvK5wP98yYI4l2HqBBd8Jqb9FQFDiOk+O8dQZ 4pbfz1JywqRx6H9QTMEW7mtjIMe1GhC4Iiu7H+AF2JMXTD6mgpVapjRxSmlxt8uGTgyN 6NIDiP+S5asiVsBMSwu81kH+I6sPTYYB//JVWc0hfAyl52++6iQYtP7UFJYOGsfXORO6 DOauCPvyrpJSOq3L/GYfi0D+llwK5sXV4FF3KQzaEPEpbPgXZF3ssC3R4mXYOXb9/sYt nAJfYm5W1P/kpIUkaWDB8LtOgbAgSAobDdrw7IOjnTxU3mqqaLygE+m/KDrgtCbhVmEC u9WQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1757074298; x=1757679098; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=rlxAeSfJx82pksd3I0ixRoBKWGI7viD6A795f500XFc=; b=C9Y+259DoPhKPV6TJW29w+MvU1LnLo8+i79BggvMLS/B/dJXmhorjDvgzAf1sglRPm OWDHbCfXv47Ub11heUAEJfrvCA3gsjc0Imzh9YWePb7uX06b7blQuIwT/9oLFo0OSFRK RDtMClBsiMkV9lg1OvDXzzTyYRXMqRCp6Kr3Mn0FDDliaPvQeemJnqY/PM1M0UJHqwS9 Cu/wSsn8zQHNlTCTlQqmmpIJAmt2GacFHaI8EygZgjK99xajlVG+UfDMcYetqzO9Q7AU 8XZQNAFgXehpzQq6REloSkaLYstQqOx6OPQY46KrdC0dkjfy+L55PpE9vlFxLYFLAmzl aNOQ== X-Forwarded-Encrypted: i=1; AJvYcCXK87WB2doAQCf1tCOjGpuHRocu1fadaLTWlZcQzMCpI0lelIKeh5JbEwsxDvWKJLFQHPYgWubDWvIqG14=@vger.kernel.org X-Gm-Message-State: AOJu0YzYofXHJdzdbaZuEc0n1FlNqeXF4jUlfnwnUGAQJRA46rWcRo2P VYyqGKp+oP8zK4PuzUsxdYMV4OyTtjmnWO462jjU4hYIQThpHdBczwCW6oC+CCSUNhkD1RU6uLT sk54tWLIHK2foYrrIxw== X-Google-Smtp-Source: AGHT+IG8jYGvzO4xJ5XE6YjDlUyoq7XkUoX06psrVfvh6sWFTCcB4r+O6TwY74/lyIP1l3hTEjNDm+Em6qNSRZg= X-Received: from wrml12.prod.google.com ([2002:adf:e58c:0:b0:3d5:28fd:8822]) (user=aliceryhl job=prod-delivery.src-stubby-dispatcher) by 2002:a05:6000:2008:b0:3e4:e4e:343c with SMTP id ffacd0b85a97d-3e40e4e3a79mr1309181f8f.31.1757074298331; Fri, 05 Sep 2025 05:11:38 -0700 (PDT) Date: Fri, 05 Sep 2025 12:11:29 +0000 In-Reply-To: <20250905-vmbo-defer-v1-0-7ae1a382b674@google.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20250905-vmbo-defer-v1-0-7ae1a382b674@google.com> X-Developer-Key: i=aliceryhl@google.com; a=openpgp; fpr=49F6C1FAA74960F43A5B86A1EE7A392FDE96209F X-Developer-Signature: v=1; a=openpgp-sha256; l=9037; i=aliceryhl@google.com; h=from:subject:message-id; bh=s2GWvmQHUwyjohU1GP99f3t5sldgnhLD+nr53UBS7jk=; b=owEBbQKS/ZANAwAKAQRYvu5YxjlGAcsmYgBoutN24be3/jGNmEoz+LMXQuVQHZUNhQi1vI/sM vYnnqGZy56JAjMEAAEKAB0WIQSDkqKUTWQHCvFIvbIEWL7uWMY5RgUCaLrTdgAKCRAEWL7uWMY5 RnFLD/4rxL4E5zD6NMkjopiwv+rIBm+fmYOSzbi9ShMG5wEXGT4QaQJ+TE+y3VvWuCZOh3JDa+C /QgvOypoW/eu9nRIWFJBqv4IOHI95WV5lkqr7IA0UGuXUY/Zg/Gcy0wWj4xWigAZa6jw/mpNToU +W8LpBpz8qYuUzY/bpjDLndzUDrym7j18IZ8b/zayd8x2Il+/Zjdal4o0CH0UXH/avk+5Fsiiz+ JtLFl8SSJEzVtweaL7x06H7+/yIN5ReqckjUv4ivszj6THkoi/iyCyBOtHu/sFkTY7+gVAS24Ak Jv2cOSuEdMOiJ88XgijiMFOfnw3WWW+ZiiT3Akk+lMiohEPanm7/pcJezQhDIljbkljNTewf7Ny vaQhnVKdAkqr4wh0eXJkAdj9E7JAK0UpyuZ7JpcjGSygEsPS5wVM7gGnWfOWd54RIQqIB5Tybua ZTshg2aOXcrpt8Fac24t45e00wSuhFCK3W6WMfu88bXG0IlJF2mhKNiUrxQ+IwEJekwKQHvhqLo uRmIvBykWbMPCn6fsX2w2CdtN5lYHA4xy1uzcv66t3kBpbDeJQ57/U3EGrZOQGOUj+Rl8EOTjYH tsSLFgPM+0nsr+KOl8x8hfwbfYVKw71Ou6ywZvfBIfFwFD6AyMMvCqv7To3iHeu82gUkEZ3NYS+ ywfT3BM2QDRI5Yg== X-Mailer: b4 0.14.2 Message-ID: <20250905-vmbo-defer-v1-2-7ae1a382b674@google.com> Subject: [PATCH 2/2] panthor: use drm_gpuva_unlink_defer() From: Alice Ryhl To: Danilo Krummrich , Matthew Brost , "=?utf-8?q?Thomas_Hellstr=C3=B6m?=" Cc: Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Simona Vetter , Boris Brezillon , Steven Price , Daniel Almeida , Liviu Dudau , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org, Alice Ryhl Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Instead of manually deferring cleanup of vm_bos, use the new GPUVM infrastructure for doing so. To avoid manual management of vm_bo refcounts, the panthor_vma_link() and panthor_vma_unlink() methods are changed to get and put a vm_bo refcount on the vm_bo. This simplifies the code a lot. I preserved the behavior where panthor_gpuva_sm_step_map() drops the refcount right away rather than letting panthor_vm_cleanup_op_ctx() do it later. Signed-off-by: Alice Ryhl --- drivers/gpu/drm/panthor/panthor_mmu.c | 112 ++++++------------------------= ---- 1 file changed, 18 insertions(+), 94 deletions(-) diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/pantho= r/panthor_mmu.c index 6dec4354e3789d17c5a87fc8de3bc86764b804bc..4922da0b106aec2bdf657ce4c59= 6acb9c63797ce 100644 --- a/drivers/gpu/drm/panthor/panthor_mmu.c +++ b/drivers/gpu/drm/panthor/panthor_mmu.c @@ -181,20 +181,6 @@ struct panthor_vm_op_ctx { u64 range; } va; =20 - /** - * @returned_vmas: List of panthor_vma objects returned after a VM operat= ion. - * - * For unmap operations, this will contain all VMAs that were covered by = the - * specified VA range. - * - * For map operations, this will contain all VMAs that previously mapped = to - * the specified VA range. - * - * Those VMAs, and the resources they point to will be released as part of - * the op_ctx cleanup operation. - */ - struct list_head returned_vmas; - /** @map: Fields specific to a map operation. */ struct { /** @map.vm_bo: Buffer object to map. */ @@ -1081,47 +1067,18 @@ void panthor_vm_free_va(struct panthor_vm *vm, stru= ct drm_mm_node *va_node) mutex_unlock(&vm->mm_lock); } =20 -static void panthor_vm_bo_put(struct drm_gpuvm_bo *vm_bo) +static void panthor_vm_bo_free(struct drm_gpuvm_bo *vm_bo) { struct panthor_gem_object *bo =3D to_panthor_bo(vm_bo->obj); - struct drm_gpuvm *vm =3D vm_bo->vm; - bool unpin; - - /* We must retain the GEM before calling drm_gpuvm_bo_put(), - * otherwise the mutex might be destroyed while we hold it. - * Same goes for the VM, since we take the VM resv lock. - */ - drm_gem_object_get(&bo->base.base); - drm_gpuvm_get(vm); - - /* We take the resv lock to protect against concurrent accesses to the - * gpuvm evicted/extobj lists that are modified in - * drm_gpuvm_bo_destroy(), which is called if drm_gpuvm_bo_put() - * releases sthe last vm_bo reference. - * We take the BO GPUVA list lock to protect the vm_bo removal from the - * GEM vm_bo list. - */ - dma_resv_lock(drm_gpuvm_resv(vm), NULL); - mutex_lock(&bo->base.base.gpuva.lock); - unpin =3D drm_gpuvm_bo_put(vm_bo); - mutex_unlock(&bo->base.base.gpuva.lock); - dma_resv_unlock(drm_gpuvm_resv(vm)); =20 - /* If the vm_bo object was destroyed, release the pin reference that - * was hold by this object. - */ - if (unpin && !drm_gem_is_imported(&bo->base.base)) + if (!drm_gem_is_imported(&bo->base.base)) drm_gem_shmem_unpin(&bo->base); - - drm_gpuvm_put(vm); - drm_gem_object_put(&bo->base.base); + kfree(vm_bo); } =20 static void panthor_vm_cleanup_op_ctx(struct panthor_vm_op_ctx *op_ctx, struct panthor_vm *vm) { - struct panthor_vma *vma, *tmp_vma; - u32 remaining_pt_count =3D op_ctx->rsvd_page_tables.count - op_ctx->rsvd_page_tables.ptr; =20 @@ -1134,16 +1091,12 @@ static void panthor_vm_cleanup_op_ctx(struct pantho= r_vm_op_ctx *op_ctx, kfree(op_ctx->rsvd_page_tables.pages); =20 if (op_ctx->map.vm_bo) - panthor_vm_bo_put(op_ctx->map.vm_bo); + drm_gpuvm_bo_put_deferred(op_ctx->map.vm_bo); =20 for (u32 i =3D 0; i < ARRAY_SIZE(op_ctx->preallocated_vmas); i++) kfree(op_ctx->preallocated_vmas[i]); =20 - list_for_each_entry_safe(vma, tmp_vma, &op_ctx->returned_vmas, node) { - list_del(&vma->node); - panthor_vm_bo_put(vma->base.vm_bo); - kfree(vma); - } + drm_gpuvm_bo_deferred_cleanup(&vm->base); } =20 static struct panthor_vma * @@ -1232,7 +1185,6 @@ static int panthor_vm_prepare_map_op_ctx(struct panth= or_vm_op_ctx *op_ctx, return -EINVAL; =20 memset(op_ctx, 0, sizeof(*op_ctx)); - INIT_LIST_HEAD(&op_ctx->returned_vmas); op_ctx->flags =3D flags; op_ctx->va.range =3D size; op_ctx->va.addr =3D va; @@ -1243,7 +1195,9 @@ static int panthor_vm_prepare_map_op_ctx(struct panth= or_vm_op_ctx *op_ctx, =20 if (!drm_gem_is_imported(&bo->base.base)) { /* Pre-reserve the BO pages, so the map operation doesn't have to - * allocate. + * allocate. This pin is dropped in panthor_vm_bo_free(), so + * once we call drm_gpuvm_bo_create(), GPUVM will take care of + * dropping the pin for us. */ ret =3D drm_gem_shmem_pin(&bo->base); if (ret) @@ -1263,9 +1217,6 @@ static int panthor_vm_prepare_map_op_ctx(struct panth= or_vm_op_ctx *op_ctx, =20 preallocated_vm_bo =3D drm_gpuvm_bo_create(&vm->base, &bo->base.base); if (!preallocated_vm_bo) { - if (!drm_gem_is_imported(&bo->base.base)) - drm_gem_shmem_unpin(&bo->base); - ret =3D -ENOMEM; goto err_cleanup; } @@ -1282,16 +1233,6 @@ static int panthor_vm_prepare_map_op_ctx(struct pant= hor_vm_op_ctx *op_ctx, mutex_unlock(&bo->base.base.gpuva.lock); dma_resv_unlock(panthor_vm_resv(vm)); =20 - /* If the a vm_bo for this combination exists, it already - * retains a pin ref, and we can release the one we took earlier. - * - * If our pre-allocated vm_bo is picked, it now retains the pin ref, - * which will be released in panthor_vm_bo_put(). - */ - if (preallocated_vm_bo !=3D op_ctx->map.vm_bo && - !drm_gem_is_imported(&bo->base.base)) - drm_gem_shmem_unpin(&bo->base); - op_ctx->map.bo_offset =3D offset; =20 /* L1, L2 and L3 page tables. @@ -1339,7 +1280,6 @@ static int panthor_vm_prepare_unmap_op_ctx(struct pan= thor_vm_op_ctx *op_ctx, int ret; =20 memset(op_ctx, 0, sizeof(*op_ctx)); - INIT_LIST_HEAD(&op_ctx->returned_vmas); op_ctx->va.range =3D size; op_ctx->va.addr =3D va; op_ctx->flags =3D DRM_PANTHOR_VM_BIND_OP_TYPE_UNMAP; @@ -1387,7 +1327,6 @@ static void panthor_vm_prepare_sync_only_op_ctx(struc= t panthor_vm_op_ctx *op_ctx struct panthor_vm *vm) { memset(op_ctx, 0, sizeof(*op_ctx)); - INIT_LIST_HEAD(&op_ctx->returned_vmas); op_ctx->flags =3D DRM_PANTHOR_VM_BIND_OP_TYPE_SYNC_ONLY; } =20 @@ -2033,26 +1972,12 @@ static void panthor_vma_link(struct panthor_vm *vm, =20 mutex_lock(&bo->base.base.gpuva.lock); drm_gpuva_link(&vma->base, vm_bo); - drm_WARN_ON(&vm->ptdev->base, drm_gpuvm_bo_put(vm_bo)); mutex_unlock(&bo->base.base.gpuva.lock); } =20 -static void panthor_vma_unlink(struct panthor_vm *vm, - struct panthor_vma *vma) +static void panthor_vma_unlink(struct panthor_vma *vma) { - struct panthor_gem_object *bo =3D to_panthor_bo(vma->base.gem.obj); - struct drm_gpuvm_bo *vm_bo =3D drm_gpuvm_bo_get(vma->base.vm_bo); - - mutex_lock(&bo->base.base.gpuva.lock); - drm_gpuva_unlink(&vma->base); - mutex_unlock(&bo->base.base.gpuva.lock); - - /* drm_gpuva_unlink() release the vm_bo, but we manually retained it - * when entering this function, so we can implement deferred VMA - * destruction. Re-assign it here. - */ - vma->base.vm_bo =3D vm_bo; - list_add_tail(&vma->node, &vm->op_ctx->returned_vmas); + drm_gpuva_unlink_defer(&vma->base); } =20 static void panthor_vma_init(struct panthor_vma *vma, u32 flags) @@ -2084,12 +2009,12 @@ static int panthor_gpuva_sm_step_map(struct drm_gpu= va_op *op, void *priv) if (ret) return ret; =20 - /* Ref owned by the mapping now, clear the obj field so we don't release = the - * pinning/obj ref behind GPUVA's back. - */ drm_gpuva_map(&vm->base, &vma->base, &op->map); panthor_vma_link(vm, vma, op_ctx->map.vm_bo); + + drm_gpuvm_bo_put_deferred(op_ctx->map.vm_bo); op_ctx->map.vm_bo =3D NULL; + return 0; } =20 @@ -2128,16 +2053,14 @@ static int panthor_gpuva_sm_step_remap(struct drm_g= puva_op *op, * owned by the old mapping which will be released when this * mapping is destroyed, we need to grab a ref here. */ - panthor_vma_link(vm, prev_vma, - drm_gpuvm_bo_get(op->remap.unmap->va->vm_bo)); + panthor_vma_link(vm, prev_vma, op->remap.unmap->va->vm_bo); } =20 if (next_vma) { - panthor_vma_link(vm, next_vma, - drm_gpuvm_bo_get(op->remap.unmap->va->vm_bo)); + panthor_vma_link(vm, next_vma, op->remap.unmap->va->vm_bo); } =20 - panthor_vma_unlink(vm, unmap_vma); + panthor_vma_unlink(unmap_vma); return 0; } =20 @@ -2154,12 +2077,13 @@ static int panthor_gpuva_sm_step_unmap(struct drm_g= puva_op *op, return ret; =20 drm_gpuva_unmap(&op->unmap); - panthor_vma_unlink(vm, unmap_vma); + panthor_vma_unlink(unmap_vma); return 0; } =20 static const struct drm_gpuvm_ops panthor_gpuvm_ops =3D { .vm_free =3D panthor_vm_free, + .vm_bo_free =3D panthor_vm_bo_free, .sm_step_map =3D panthor_gpuva_sm_step_map, .sm_step_remap =3D panthor_gpuva_sm_step_remap, .sm_step_unmap =3D panthor_gpuva_sm_step_unmap, --=20 2.51.0.355.g5224444f11-goog