On Mon, Jun 08, 2026 at 06:54:28PM -0700, Sami Tolvanen wrote: > On Mon, Jun 8, 2026 at 5:33 PM Sami Tolvanen <[email protected]> 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
