Re: [PATCH] drm/i915: annote drop_caches debugfs interface with lockdep

[Daniel Vetter]

On Mon, Mar 13, 2017 at 09:01:57AM +0100, Peter Zijlstra wrote:
> On Sun, Mar 12, 2017 at 09:53:40PM +0100, Daniel Vetter wrote:
> 
> > Peter/Ingo,
> > 
> > We want this to validate the i915 shrinker locking in our fast tests
> > without thrashing badly (that takes too long, we can only thrash in
> > the extended runs). Can you pls take a look and if it's ok ack for
> > merging through drm-intel.git?
> 
> Hurm, I was going to rework all that soonish; have a look here:
> 
>  https://lkml.kernel.org/r/20170302134031.GG6536@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
> 
> The immediate problem is that I made the annotation private to mm/
> there, I suppose I could fix that.

Yeah, we'd really like to have that, and even when switched to a
lockdep_map instead of reusing the context stuff the semantic interface
would be the same (and I think we should keep the gfp_flags stuff, in case
someone adds a nesting lockdep map for GFP_IO).

Do you want a topic branch with just this patch (the shrink_all is new so
there will be a conflict and we can't mege it through one tree alone) so
that you can refactor things with i915 included?

Thanks, Daniel

> 
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c | 2 ++
> >  kernel/locking/lockdep.c            | 2 ++
> >  2 files changed, 4 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 82fb005a5e22..fbe761a3f5bd 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -4273,6 +4273,7 @@ i915_drop_caches_set(void *data, u64 val)
> >  	if (val & (DROP_RETIRE | DROP_ACTIVE))
> >  		i915_gem_retire_requests(dev_priv);
> >  
> > +	lockdep_set_current_reclaim_state(GFP_KERNEL);
> >  	if (val & DROP_BOUND)
> >  		i915_gem_shrink(dev_priv, LONG_MAX, I915_SHRINK_BOUND);
> >  
> > @@ -4281,6 +4282,7 @@ i915_drop_caches_set(void *data, u64 val)
> >  
> >  	if (val & DROP_SHRINK_ALL)
> >  		i915_gem_shrink_all(dev_priv);
> > +	lockdep_clear_current_reclaim_state();
> >  
> >  unlock:
> >  	mutex_unlock(&dev->struct_mutex);
> > diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
> > index 12e38c213b70..508cbf31d43e 100644
> > --- a/kernel/locking/lockdep.c
> > +++ b/kernel/locking/lockdep.c
> > @@ -3856,11 +3856,13 @@ void lockdep_set_current_reclaim_state(gfp_t gfp_mask)
> >  {
> >  	current->lockdep_reclaim_gfp = gfp_mask;
> >  }
> > +EXPORT_SYMBOL_GPL(lockdep_set_current_reclaim_state);
> >  
> >  void lockdep_clear_current_reclaim_state(void)
> >  {
> >  	current->lockdep_reclaim_gfp = 0;
> >  }
> > +EXPORT_SYMBOL_GPL(lockdep_clear_current_reclaim_state);
> >  
> >  #ifdef CONFIG_LOCK_STAT
> >  static int
> > -- 
> > 2.11.0
> > 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch