All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alice Ryhl <aliceryhl@google.com>
To: Danilo Krummrich <dakr@kernel.org>
Cc: "Daniel Almeida" <daniel.almeida@collabora.com>,
	"Boris Brezillon" <boris.brezillon@collabora.com>,
	"Janne Grunau" <j@jannau.net>,
	"Matthew Brost" <matthew.brost@intel.com>,
	"Thomas Hellström" <thomas.hellstrom@linux.intel.com>,
	"Lyude Paul" <lyude@redhat.com>,
	"Asahi Lina" <lina+kernel@asahilina.net>,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	rust-for-linux@vger.kernel.org
Subject: Re: [PATCH v4 3/6] rust: gpuvm: add GpuVm::obtain()
Date: Sat, 21 Feb 2026 08:46:21 +0000	[thread overview]
Message-ID: <aZlw3SGmLetHwaTM@google.com> (raw)
In-Reply-To: <DGJX3FI97W1G.371MAMC60FX24@kernel.org>

On Fri, Feb 20, 2026 at 05:08:09PM +0100, Danilo Krummrich wrote:
> On Fri Feb 20, 2026 at 9:16 AM CET, Alice Ryhl wrote:
> >> > +/// A [`GpuVmBo`] object in the GEM list.
> >> > +///
> >> > +/// # Invariants
> >> > +///
> >> > +/// Points at a `drm_gpuvm_bo` that contains a valid `T::VmBoData` and is present in the gem list.
> >> > +pub struct GpuVmBoRegistered<T: DriverGpuVm>(NonNull<GpuVmBo<T>>);
> >> 
> >> I know that I proposed to rename this from GpuVmBoResident to GpuVmBoRegistered
> >> in a drive-by comment on v3.
> >> 
> >> But now that I have a closer look, I think it would be nice to just have GpuVmBo
> >> being the registered one and GpuVmBoAlloc being the pre-allocated one.
> >> 
> >> As it is currently, I think it is bad to ever present a &GpuVmBo to a driver
> >> because it implies that we don't know whether it is a pre-allocated one or a
> >> "normal", registered one. But we do always know.
> >
> > Actually, I think GpuVmBo is already the registered one.
> > GpuVmBoRegistered is just ARef<GpuVmBo<T>>.
> 
> GpuVmBoAlloc<T> dereferences to GpuVmBo<T>, so currently it is not.

I will drop the Deref impl.

> >> For instance, in patch 6 we give out &'op GpuVmBo<T>, but it actually carries
> >> the invariant of being registered.
> >> 
> >> Of course, we could fix this by giving out a &'op GpuVmBoRegistered<T> instead,
> >> but it would be nice to not have drivers be in touch with a type that can be one
> >> or the other.
> >> 
> >> I know that the current GpuVmBo<T> also serves the purpose of storing common
> >> code. Maybe we can make it private, call it GpuVmBoInner<T> and have inline
> >> forwarding methods for GpuVmBo<T> and GpuVmBoAlloc<T>. This is slightly more
> >> overhead in this implementation due to the forwarding methods, but less
> >> ambiguity for drivers, which I think is more important.
> >
> > I think we should keep the current state that GpuVmBo is registered, and
> > only GpuVmBoAlloc is not. That is most useful.
> 
> We seem to agree then: What I want is that from a driver perspective there is
> only GpuVmBo<T> (which is the registered thing) and GpuVmBoAlloc<T> which is the
> pre-allocated thing, i.e.  no separate GpuVmBoRegistered<T> type.

So, should we get rid of GpuVmBoRegistered in favor of ARef<GpuVm<T>>?

Alice

  reply	other threads:[~2026-02-21  8:46 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-30 14:24 [PATCH v4 0/6] Rust GPUVM immediate mode Alice Ryhl
2026-01-30 14:24 ` [PATCH v4 1/6] rust: drm: add base GPUVM immediate mode abstraction Alice Ryhl
2026-02-02 15:15   ` Boris Brezillon
2026-02-19 14:36   ` Danilo Krummrich
2026-02-19 14:41     ` Alice Ryhl
2026-02-19 14:55       ` Danilo Krummrich
2026-01-30 14:24 ` [PATCH v4 2/6] rust: helpers: Add bindings/wrappers for dma_resv_lock Alice Ryhl
2026-02-19 14:40   ` Danilo Krummrich
2026-02-19 14:45     ` Alice Ryhl
2026-01-30 14:24 ` [PATCH v4 3/6] rust: gpuvm: add GpuVm::obtain() Alice Ryhl
2026-02-19 19:22   ` Danilo Krummrich
2026-02-20  8:16     ` Alice Ryhl
2026-02-20 16:08       ` Danilo Krummrich
2026-02-21  8:46         ` Alice Ryhl [this message]
2026-02-21 15:09           ` Danilo Krummrich
2026-02-23  9:15             ` Alice Ryhl
2026-02-23 10:44               ` Danilo Krummrich
2026-02-23 11:22                 ` Alice Ryhl
2026-02-25 15:46                   ` Danilo Krummrich
2026-01-30 14:24 ` [PATCH v4 4/6] rust: gpuvm: add GpuVa struct Alice Ryhl
2026-01-30 14:24 ` [PATCH v4 5/6] rust: gpuvm: add GpuVmCore::sm_unmap() Alice Ryhl
2026-01-30 14:24 ` [PATCH v4 6/6] rust: gpuvm: add GpuVmCore::sm_map() Alice Ryhl
2026-02-06 20:17   ` Deborah Brouwer
2026-02-09  8:17     ` Alice Ryhl

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aZlw3SGmLetHwaTM@google.com \
    --to=aliceryhl@google.com \
    --cc=boris.brezillon@collabora.com \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=j@jannau.net \
    --cc=lina+kernel@asahilina.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lyude@redhat.com \
    --cc=matthew.brost@intel.com \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=thomas.hellstrom@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.