Re: [PATCH v3 7/8] drm/nouveau: use DRM_GPUVM_RESV_PROTECTED

["Danilo Krummrich" <dakr@kernel.org>]

On Sun Oct 18, 2026 at 3:32 AM CEST, Matthew Brost wrote:
> On Sat, Oct 17, 2026 at 06:02:26PM -0700, Matthew Brost wrote:
>> On Sat, Oct 17, 2026 at 11:31:06PM +0200, Danilo Krummrich wrote:
>> > On Fri Oct 2, 2026 at 12:06 AM CEST, Matthew Brost wrote:
>> > > @@ -1187,6 +1187,27 @@ bind_validate_region(struct nouveau_job *job)
>> > >  	return 0;
>> > >  }
>> > >  
>> > > +/*
>> > > + * Lock the VM's common dma-resv together with the one of @obj, as needed to
>> > > + * drop what may be the last reference of a &drm_gpuvm_bo.
>> > > + */
>> > > +static void
>> > > +nouveau_uvmm_lock_vm_and_obj(struct nouveau_uvmm *uvmm, struct drm_exec *exec,
>> > > +			     struct drm_gem_object *obj)
>> > > +{
>> > > +	int ret;
>> > > +
>> > > +	drm_exec_init(exec, DRM_EXEC_IGNORE_DUPLICATES, 2);
>> > > +	drm_exec_until_all_locked(exec) {
>> > > +		ret = drm_exec_lock_obj(exec, drm_gpuvm_resv_obj(&uvmm->base));
>> > > +		if (!ret)
>> > > +			ret = drm_exec_lock_obj(exec, obj);
>> > 
>> > What happens if we reach this with ENOMEM? In such a case we may return without
>> 
>> Right, valid issue. Sashiko flagged this too.
>> 
>> Originally, I thought we would need to open-code the WW transaction, but
>> instead I ended up fixing drm_exec [1].
>> 
>> So I think we should merge [1] and run the MSM and Nouveau work in a
>> no-failure mode.
>> 
>> One thing I'd add is an assert(!ret) if Nouveau has support for that
>> (see xe_assert.h for what we do in Xe so our CI can catch things we
>> believe are impossible). I suppose the open-coded version would be a
>> WARN_ON() or WARN_ON_ONCE().
>> 
>> Also likey a BUILD_BUG_ON(2 <= DRM_EXEC_INLINE_OBJECTS).
>> 
>
> Ugh, logic inversion.
>
> Rather:
>
> BUILD_BUG_ON(2 > DRM_EXEC_INLINE_OBJECTS)
>
> Matt
>
>> Let me know if you want me to respin with these changes.

That'd be great, thanks!

Please prefer BUILD_BUG_ON(), I'd like to avoid the need for any WARN_ON() in
this code path.

>> 
>> Matt 
>> 
>> [1] https://patchwork.freedesktop.org/series/175390/
>> 
>> > any locks being held and nouveau would just continue calling into function that
>> > expect locks to be held and hence produced UB, right?
>> > 
>> > (I think there is a similar case in MSM, but it was pre-existing, so I picked all
>> > other patches.)
>> > 
>> > > +		drm_exec_retry_on_contention(exec);
>> > > +		if (drm_WARN_ON(uvmm->base.drm, ret))
>> > > +			break;
>> > > +	}
>> > > +}