All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alice Ryhl <aliceryhl@google.com>
To: Sami Tolvanen <samitolvanen@google.com>
Cc: "Danilo Krummrich" <dakr@kernel.org>,
	"Matthew Brost" <matthew.brost@intel.com>,
	"Thomas Hellström" <thomas.hellstrom@linux.intel.com>,
	"David Airlie" <airlied@gmail.com>,
	"Simona Vetter" <simona@ffwll.ch>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Boqun Feng" <boqun@kernel.org>, "Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Daniel Almeida" <daniel.almeida@collabora.com>,
	"Asahi Lina" <lina+kernel@asahilina.net>,
	dri-devel@lists.freedesktop.org, rust-for-linux@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 2/2] rust: drm: gpuvm: require Send and Sync for GpuVm's shared data
Date: Wed, 10 Jun 2026 07:13:54 +0000	[thread overview]
Message-ID: <aikOstbyTYGuV8xp@google.com> (raw)
In-Reply-To: <CABCJKucVW-FF2sti9gVoKkp-StLMof7kSqisbmUvjyEAaKLuTw@mail.gmail.com>

On Mon, Jun 08, 2026 at 06:54:28PM -0700, Sami Tolvanen wrote:
> On Mon, Jun 8, 2026 at 5:33 PM Sami Tolvanen <samitolvanen@google.com> wrote:
> >
> > GpuVm implements Send and Sync unconditionally, but it shares and drops
> > T::VmBoData and T::Object across threads: obtain() can return ARefs to
> > the same bo on several threads, and deferred_cleanup() drops them on
> > whichever thread calls it. This is unsound unless both are Send + Sync,
> > so bound both impls accordingly.
> 
> Sashiko keeps finding more issues here. For the benefit of those not
> subscribed to dri-devel:
> 
> > This is a pre-existing issue, but does GpuVm also need Send + Sync bounds
> > for T::VaData here?
> >
> > Since GpuVm<T> explicitly bypasses the lack of Send on VaData,
> > UniqueRefGpuVm<T> automatically derives Send. This allows safe code to map
> > a VA containing !Send data on one thread, send the UniqueRefGpuVm<T> to
> > another thread, and unmap the VA there.
> >
> > Unmapping via sm_unmap() returns a GpuVaRemoved<T> which drops the !Send
> > VaData on the wrong thread, potentially causing data races.
> 
> Looking at the code, UniqueRefGpuVm seems like a better place for this
> than GpuVm. However, before I go add another patch to the series, I
> wonder if it would make more sense to just add these bounds to the
> DriverGpuVm trait instead? I'd be happy to hear some non-AI thoughts
> about this too.

Adding bounds on the trait does sound like a good idea for simplicity.
It's not like we're ever going to use GPUVM with non-thread-safe types.

If it's help, note that you can use `where GpuVm<T>: Send` bounds or
similar to "reuse" the implementations elsewhere. For example,
GpuVmBo<T> being Sync requires GpuVm<T> being Sync because of the
GpuVmBo::gpuvm() method, so you can use `where GpuVm<T>: Send` as a
bound to directly express that if you want.

But if we just add bounds to the trait, it may not be necessary.

Alice

      reply	other threads:[~2026-06-10  7:13 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-09  0:32 [PATCH v3 0/2] rust: drm: gpuvm: implement Send and Sync for refcounted handles Sami Tolvanen
2026-06-09  0:32 ` [PATCH v3 1/2] rust: drm: gpuvm: implement Send and Sync for GpuVaAlloc and GpuVmBo Sami Tolvanen
2026-06-09  0:32 ` [PATCH v3 2/2] rust: drm: gpuvm: require Send and Sync for GpuVm's shared data Sami Tolvanen
2026-06-09  0:49   ` sashiko-bot
2026-06-09  1:54   ` Sami Tolvanen
2026-06-10  7:13     ` Alice Ryhl [this message]

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=aikOstbyTYGuV8xp@google.com \
    --to=aliceryhl@google.com \
    --cc=a.hindborg@kernel.org \
    --cc=airlied@gmail.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun@kernel.org \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gary@garyguo.net \
    --cc=lina+kernel@asahilina.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=matthew.brost@intel.com \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=samitolvanen@google.com \
    --cc=simona@ffwll.ch \
    --cc=thomas.hellstrom@linux.intel.com \
    --cc=tmgross@umich.edu \
    /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.