[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <DCNDGFE7RR5Q.X3PCDW0KIX89@kernel.org>
Date: Mon, 08 Sep 2025 13:11:32 +0200
From: "Danilo Krummrich" <dakr@...nel.org>
To: "Boris Brezillon" <boris.brezillon@...labora.com>
Cc: "Alice Ryhl" <aliceryhl@...gle.com>, "Matthew Brost"
<matthew.brost@...el.com>, Thomas Hellström
<thomas.hellstrom@...ux.intel.com>, "Maarten Lankhorst"
<maarten.lankhorst@...ux.intel.com>, "Maxime Ripard" <mripard@...nel.org>,
"Thomas Zimmermann" <tzimmermann@...e.de>, "David Airlie"
<airlied@...il.com>, "Simona Vetter" <simona@...ll.ch>, "Steven Price"
<steven.price@....com>, "Daniel Almeida" <daniel.almeida@...labora.com>,
"Liviu Dudau" <liviu.dudau@....com>, <dri-devel@...ts.freedesktop.org>,
<linux-kernel@...r.kernel.org>, <rust-for-linux@...r.kernel.org>
Subject: Re: [PATCH 1/2] drm/gpuvm: add deferred vm_bo cleanup
On Mon Sep 8, 2025 at 12:20 PM CEST, Boris Brezillon wrote:
> I'm not following. Yes there's going to be a
> drm_gpuva_unlink_defer_put() that skips the
>
> va->vm_bo = NULL;
> drm_gpuvm_bo_put(vm_bo);
>
> and adds the gpuva to a list for deferred destruction. But I'm not
> seeing where the leak is. Once the gpuva has been put in this list,
> there should be no existing component referring to this object, and it's
> going to be destroyed or recycled, but not re-used as-is.
I'm saying exactly what you say: "has to be a special unlink function" ->
drm_gpuva_unlink_defer_put(). :)
>> Yeah, we really want to avoid that.
>
> I think I agree that we want to avoid it, but I'm not too sure about
> the solution that involves playing with vm_bo::kref. I'm particularly
> worried by the fact drivers can iterate the evict/extobj lists
> directly, and can thus see objects scheduled for destruction. I know
> there's a gpuvm_bo_is_dead() helper, and drivers should be aware of the
> risks, but I don't feel comfortable about this.
No, drivers can't iterate the evict/extobj lists directly; or at least this is
not intended by GPUVM's API and if drivers do so, this is considered peeking
into GPUVM internals, so drivers are on their own anyways.
Iterators, such as for_each_vm_bo_in_list() are not exposed to drivers.
> And since we've mentioned the possibility of having to support
> gpuva destruction deferral too, I'm wondering it wouldn't be cleaner
> to just go for this approach from the start (gpuva owns a ref to a
> vm_bo, which gets released when the gpuva object is released).
Powered by blists - more mailing lists