On Thu, Nov 20, 2014 at 12:09:18PM +0000, Daniel, Thomas wrote: > > -----Original Message----- > > From: Chris Wilson [mailto:chris@xxxxxxxxxxxxxxxxxx] > > Sent: Thursday, November 20, 2014 11:41 AM > > To: Daniel, Thomas > > Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; akash goel (akash.goels@xxxxxxxxx) > > Subject: Re: [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@xxxxxxxxxxxxxxxxxx] > > > > Sent: Thursday, November 20, 2014 11:16 AM > > > > To: Daniel, Thomas > > > > Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; akash-goels@xxxxxxxxx > > > > Subject: Re: [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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
