[Intel-gfx] [PATCH 02/18] drm/i915: Flush pages on acquisition

[Matthew Auld]

On Wed, 20 Mar 2019 at 11:48, Chris Wilson <chris at chris-wilson.co.uk> wrote:
>
> Quoting Matthew Auld (2019-03-20 11:41:52)
> > On Tue, 19 Mar 2019 at 11:58, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> > > @@ -2534,6 +2522,14 @@ void __i915_gem_object_set_pages(struct drm_i915_gem_object *obj,
> > >
> > >         lockdep_assert_held(&obj->mm.lock);
> > >
> > > +       /* Make the pages coherent with the GPU (flushing any swapin). */
> > > +       if (obj->cache_dirty) {
> > > +               obj->write_domain = 0;
> > > +               if (i915_gem_object_has_struct_page(obj))
> > > +                       drm_clflush_sg(pages);
> > > +               obj->cache_dirty = false;
> > > +       }
> >
> > Is it worth adding some special casing here for volatile objects, so
> > that we avoid doing the clflush_sg every time we do set_pages for
> > !llc?
> >
> > if (obj->cache_dirty && obj->mm.madvise == WILLNEED)
> >
> > Or is that meh?
>
> No, even for volatile objects we have to be careful with what remains in
> the CPU cache as that may obscure updates to the underlying page. We see
> the very same problem with speculative cacheline loading.
>
> A DONTNEED object should fail before it gets allocated pages :)

I was talking about kernel internal objects, which are marked as
DONTNEED just before we call set_pages(), and for that case it's
surely up to the caller to flush things before they even think of
doing the unpin(since it's volatile).

>
> If it becomes DONTNEED in flight? Haven't considered that case, but I
> think it is best we keep the pages around as we have people waiting for
> them, so we should consider them in use and so only reap them after this
> period of activity.
>
> One agenda I have for local-memory is the per-fd private page pool,
> where we can stuff cache flushed pages for reuse in !llc. However, all
> the testing many, many years ago said that if userspace is doing the
> write thing, such a cache is fruitless.
> -Chris