On Thu, Dec 10, 2015 at 09:04:23PM +0000, Chris Wilson wrote: > On Thu, Dec 10, 2015 at 05:24:42PM +0000, Dave Gordon wrote: > > On 10/12/15 13:29, Chris Wilson wrote: > > >On Wed, Dec 09, 2015 at 03:52:51PM +0000, Dave Gordon wrote: > > >>In various places, a single page of a (regular) GEM object is mapped into > > >>CPU address space and updated. In each such case, either the page or the > > >>the object should be marked dirty, to ensure that the modifications are > > >>not discarded if the object is evicted under memory pressure. > > >> > > >>The typical sequence is: > > >> va = kmap_atomic(i915_gem_object_get_page(obj, pageno)); > > >> *(va+offset) = ... > > >> kunmap_atomic(va); > > >> > > >>Here we introduce i915_gem_object_get_dirty_page(), which performs the > > >>same operation as i915_gem_object_get_page() but with the side-effect > > >>of marking the returned page dirty in the pagecache. This will ensure > > >>that if the object is subsequently evicted (due to memory pressure), > > >>the changes are written to backing store rather than discarded. > > >> > > >>Note that it works only for regular (shmfs-backed) GEM objects, but (at > > >>least for now) those are the only ones that are updated in this way -- > > >>the objects in question are contexts and batchbuffers, which are always > > >>shmfs-backed. > > >> > > >>A separate patch deals with the case where whole objects are (or may > > >>be) dirtied. > > >> > > >>Signed-off-by: Dave Gordon <david.s.gordon@xxxxxxxxx> > > >>Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > > > > > >I like this. There are places were we do both obj->dirty and > > >set_page_dirty(), but this so much more clearly shows what is going on. > > >All of these locations should be infrequent (or at least have patches to > > >make them so), so moving the call out-of-line will also be a benefit. > > > > I think there was only one place that both called set_page_dirty() > > AND set obj->dirty, which was in populate_lr_context(). You'll see > > that I've eliminated both in favour of a call to get_dirty_page() :) > > It was things like all GPU objects have obj->dirty set, so really the > importance of using get_dirty_page() in the relocations and context > pinning is for documentation. Which is a very good reason, nevertheless. Hm, I think if you force a fault on relocs and then shrink everything really hard before actually managing to submit the batch you could provoke this into a proper bug. one-in-a-billion perhaps ;-) -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
