Re: [PATCH 01/10] drm/exec: Add helper to bypass IGNORE_DUPLICATES flag

["Thomas Hellström" <thomas.hellstrom@linux.intel.com>]

On Tue, 2026-07-07 at 14:28 +0200, Christian König wrote:
> On 7/7/26 14:09, Thomas Hellström wrote:
> > On Fri, 2026-07-03 at 18:31 +0200, Natalie Vock wrote:
> > > TTM is about to switch to drm_exec for locking objects
> > > in the LRU list. When we're done processing the object, we want
> > > to
> > > unlock it only if the caller doesn't already hold that lock. If
> > > DRM_EXEC_IGNORE_DUPLICATES is set on the exec object (which
> > > callers
> > > may
> > > require for unrelated reasons), we have no way of knowing whether
> > > the
> > > lock is already held.
> > > 
> > > To remedy this, add a separate helper that forcefully bypasses
> > > the
> > > IGNORE_DUPLICATES flag for only a single locking operation.
> > > 
> > > Signed-off-by: Natalie Vock <natalie.vock@gmx.de>
> > 
> > The first, more complete attempt to try to tackle the exhaustive
> > eviction introduced a drm_exec snapshot ability instead.
> > 
> > https://lists.freedesktop.org/archives/intel-xe/2024-May/035820.html
> > 
> > The idea was that one would want to incrementally lock more buffer
> > objects until the validation succeeded, rather than dropping each
> > single lock after processing the eviction. That's actually what
> > guarantees forward progress. Restoring the snapshot unlocked all
> > locks
> > that we grabbed in the process, and would work also for single
> > locks.
> 
> That's actually not what this patch here tries to solve.
> 
> The problem is rather since we don't remove the BOs from the LRU list
> that we try to evict some which are actually part of our working set.
> 
> So when we lock for eviction we can't ignore duplicates no matter
> what the global flag says.

Ah yes, I see that now when I've gotten to patch 9. But then I think
the commit message is a bit misleading. It talks about unlocking an
already processed object?

Thanks,
Thomas


> 
> Regards,
> Christian.
> 
> > 
> > Thanks,
> > Thomas
> > 
> > 
> > > ---
> > >  drivers/gpu/drm/drm_exec.c | 52
> > > ++++++++++++++++++++++++++++++++++--
> > > ----------
> > >  include/drm/drm_exec.h     |  2 ++
> > >  2 files changed, 41 insertions(+), 13 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_exec.c
> > > b/drivers/gpu/drm/drm_exec.c
> > > index 7988f5e7d56a3..91de6b4d29df8 100644
> > > --- a/drivers/gpu/drm/drm_exec.c
> > > +++ b/drivers/gpu/drm/drm_exec.c
> > > @@ -190,18 +190,9 @@ static int drm_exec_lock_contended(struct
> > > drm_exec *exec)
> > >  	return ret;
> > >  }
> > >  
> > > -/**
> > > - * drm_exec_lock_obj - lock a GEM object for use
> > > - * @exec: the drm_exec object with the state
> > > - * @obj: the GEM object to lock
> > > - *
> > > - * Lock a GEM object for use and grab a reference to it.
> > > - *
> > > - * Returns: -EDEADLK if a contention is detected, -EALREADY when
> > > object is
> > > - * already locked (can be suppressed by setting the
> > > DRM_EXEC_IGNORE_DUPLICATES
> > > - * flag), -ENOMEM when memory allocation failed and zero for
> > > success.
> > > - */
> > > -int drm_exec_lock_obj(struct drm_exec *exec, struct
> > > drm_gem_object
> > > *obj)
> > > +static int __drm_exec_lock_obj(struct drm_exec *exec,
> > > +			       struct drm_gem_object *obj,
> > > +			       bool always_report_duplicates)
> > >  {
> > >  	int ret;
> > >  
> > > @@ -226,7 +217,7 @@ int drm_exec_lock_obj(struct drm_exec *exec,
> > > struct drm_gem_object *obj)
> > >  		return -EDEADLK;
> > >  	}
> > >  
> > > -	if (unlikely(ret == -EALREADY) &&
> > > +	if (unlikely(ret == -EALREADY) &&
> > > !always_report_duplicates
> > > &&
> > >  	    exec->flags & DRM_EXEC_IGNORE_DUPLICATES)
> > >  		return 0;
> > >  
> > > @@ -243,8 +234,43 @@ int drm_exec_lock_obj(struct drm_exec *exec,
> > > struct drm_gem_object *obj)
> > >  	dma_resv_unlock(obj->resv);
> > >  	return ret;
> > >  }
> > > +
> > > +/**
> > > + * drm_exec_lock_obj - lock a GEM object for use
> > > + * @exec: the drm_exec object with the state
> > > + * @obj: the GEM object to lock
> > > + *
> > > + * Lock a GEM object for use and grab a reference to it.
> > > + *
> > > + * Returns: -EDEADLK if a contention is detected, -EALREADY when
> > > object is
> > > + * already locked (can be suppressed by setting the
> > > DRM_EXEC_IGNORE_DUPLICATES
> > > + * flag), -ENOMEM when memory allocation failed and zero for
> > > success.
> > > + */
> > > +int drm_exec_lock_obj(struct drm_exec *exec, struct
> > > drm_gem_object
> > > *obj)
> > > +{
> > > +	return __drm_exec_lock_obj(exec, obj, false);
> > > +}
> > >  EXPORT_SYMBOL(drm_exec_lock_obj);
> > >  
> > > +/**
> > > + * drm_exec_lock_obj_report_dup - lock a GEM object for use, but
> > > always report duplicates
> > > + * @exec: the drm_exec object with the state
> > > + * @obj: the GEM object to lock
> > > + *
> > > + * Like drm_exec_lock_obj, lock a GEM object for use and grab a
> > > reference to it.
> > > + * Unlike drm_exec_lock_obj, DRM_EXEC_IGNORE_DUPLICATES is
> > > ignored
> > > and duplicates are
> > > + * always reported.
> > > + *
> > > + * Returns: -EDEADLK if a contention is detected, -EALREADY when
> > > object is
> > > + * already locked, -ENOMEM when memory allocation failed and
> > > zero
> > > for success.
> > > + */
> > > +int drm_exec_lock_obj_report_dup(struct drm_exec *exec,
> > > +				 struct drm_gem_object *obj)
> > > +{
> > > +	return __drm_exec_lock_obj(exec, obj, false);
> > > +}
> > > +EXPORT_SYMBOL(drm_exec_lock_obj_report_dup);
> > > +
> > >  /**
> > >   * drm_exec_unlock_obj - unlock a GEM object in this exec
> > > context
> > >   * @exec: the drm_exec object with the state
> > > diff --git a/include/drm/drm_exec.h b/include/drm/drm_exec.h
> > > index 8725ba92ff916..ff80dd2b72240 100644
> > > --- a/include/drm/drm_exec.h
> > > +++ b/include/drm/drm_exec.h
> > > @@ -176,6 +176,8 @@ void drm_exec_init(struct drm_exec *exec, u32
> > > flags, unsigned nr);
> > >  void drm_exec_fini(struct drm_exec *exec);
> > >  bool drm_exec_cleanup(struct drm_exec *exec);
> > >  int drm_exec_lock_obj(struct drm_exec *exec, struct
> > > drm_gem_object
> > > *obj);
> > > +int drm_exec_lock_obj_report_dup(struct drm_exec *exec,
> > > +				 struct drm_gem_object *obj);
> > >  void drm_exec_unlock_obj(struct drm_exec *exec, struct
> > > drm_gem_object *obj);
> > >  int drm_exec_prepare_obj(struct drm_exec *exec, struct
> > > drm_gem_object *obj,
> > >  			 unsigned int num_fences);