[Intel-gfx] [PATCH] drm/i915: Don't pin LRC in GGTT when dumping in debugfs

[Chris Wilson]

On Thu, Nov 20, 2014 at 12:09:18PM +0000, Daniel, Thomas wrote:
> > -----Original Message-----
> > From: Chris Wilson [mailto:chris at chris-wilson.co.uk]
> > Sent: Thursday, November 20, 2014 11:41 AM
> > To: Daniel, Thomas
> > Cc: intel-gfx at lists.freedesktop.org; akash goel (akash.goels at gmail.com)
> > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Don't pin LRC in GGTT when
> > dumping in debugfs
> > 
> > On Thu, Nov 20, 2014 at 11:25:57AM +0000, Daniel, Thomas wrote:
> > > > -----Original Message-----
> > > > From: Chris Wilson [mailto:chris at chris-wilson.co.uk]
> > > > Sent: Thursday, November 20, 2014 11:16 AM
> > > > To: Daniel, Thomas
> > > > Cc: intel-gfx at lists.freedesktop.org; akash-goels at gmail.com
> > > > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Don't pin LRC in GGTT
> > > > when dumping in debugfs
> > > >
> > > > On Thu, Nov 20, 2014 at 11:12:05AM +0000, Thomas Daniel wrote:
> > > > > LRC object does not need to be mapped into the GGTT when dumping.
> > > > > Just use pin_pages. A side-effect of this patch is that a compiler
> > > > > warning goes away (not checking return value of
> > i915_gem_obj_ggtt_pin).
> > > >
> > > > Please explain why you need to pin the pages.
> > > I suppose I don't as this is protected by the struct mutex and unpin is called
> > before returning.
> > 
> > The question is: do we need protection against kmalloc and a potential call
> > into the shrinker who may steal the pages from underneath us. Here, we
> > only do a seq_printf() under the lock after get_pages() and that uses a
> > preallocated buffer.
> I don't think so... If a context is in the context_list then the ctx_obj will have a nonzero refcount.  The struct mutex prevents the refcount from changing.

The shrinker only steals pages. Well it may reap the active reference
count as well, which must also be kept in mind, but the ctx_obj should
have the reference from being inside the list and be safe.
 
> Can you identify any situation where the pages may go away?

Anytime you trigger an allocation, the system may reap any objects
pages. It will even steal the dev->struct_mutex. To protect against the
shrinker you have to call pin_pages(). Here, there are no allocations
inside the loop and so you don't need to worry about the shrinker
stealing your pages.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre