Re: [Intel-gfx] [PATCH i-g-t] i915/api_intel_bb: Only assert objects are unmoved for full-ppgtt

From: Chris Wilson <chris@chris-wilson.co.uk>
To: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
Cc: igt-dev@lists.freedesktop.org, intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH i-g-t] i915/api_intel_bb: Only assert objects are unmoved for full-ppgtt
Date: Thu, 03 Dec 2020 10:51:01 +0000	[thread overview]
Message-ID: <160699266175.2026.13704936004765341238@build.alporthouse.com> (raw)
In-Reply-To: <20201203103423.GA29773@zkempczy-mobl2>

Quoting Zbigniew Kempczyński (2020-12-03 10:34:23)
> On Thu, Dec 03, 2020 at 08:39:31AM +0000, Chris Wilson wrote:
> > If we let an object idle in a shared GTT, it may be evicted by the
> > kernel in favour of another client. Thus, we have to be very careful
> > when asserting that two different executions of the same object will
> > be at the same address. If there's an idle point between the two
> > asserts, it will only be guaranteed to hold for full-ppgtt.
> > 
> > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2754
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> > ---
> >  tests/i915/api_intel_bb.c | 19 +++++++++++++++----
> >  1 file changed, 15 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tests/i915/api_intel_bb.c b/tests/i915/api_intel_bb.c
> > index 0cb3192cb..18814d14d 100644
> > --- a/tests/i915/api_intel_bb.c
> > +++ b/tests/i915/api_intel_bb.c
> > @@ -505,10 +505,21 @@ static void blit(struct buf_ops *bops,
> >       intel_bb_exec(ibb, intel_bb_offset(ibb), flags, true);
> >       check_buf(dst, COLOR_77);
> >  
> > -     poff2_src = intel_bb_get_object_offset(ibb, src->handle);
> > -     poff2_dst = intel_bb_get_object_offset(ibb, dst->handle);
> > -     igt_assert(poff_src == poff2_src);
> > -     igt_assert(poff_dst == poff2_dst);
> > +     /*
> > +      * Since we let the objects idle, if the GTT is shared another client
> > +      * is liable to reuse our offsets for themselves, causing us to have
> > +      * to relocate. We don't expect this to happen as LRU eviction should
> > +      * try to avoid reuse, but we use random eviction instead as it is
> > +      * much quicker! Given that the kernel is *allowed* to relocate objects,
> > +      * we cannot assert that the objects remain in the same location, unless
> > +      * we are in full control of our own GTT.
> > +      */
> > +     if (gem_uses_full_ppgtt(i915)) {
> > +             igt_assert_eq_u64(intel_bb_get_object_offset(ibb, src->handle),
> > +                               poff_src);
> > +             igt_assert_eq_u64(intel_bb_get_object_offset(ibb, dst->handle),
> > +                               poff_dst);
> > +     }
> >  
> >       intel_buf_destroy(src);
> >       intel_buf_destroy(dst);
> > -- 
> > 2.29.2
> > 
> 
> Patch looks ok. BTW is it possible when we're running the test in isolated
> environment (IGT)?

Never say never. The kernel does do things in the background (e.g.
responding to hotplug with fbcon) that may cause the global GTT to be
evicted, and we do some background work for heartbeats and the like (but
they should all be statically placed, so unlikely).

GTT mmapping is another possible cause, since the preference is to keep
the whole object as a single mmapped vma, and so we may move an existing
vma used for the execbuf to a mappable location. (Only if idle at the
time, otherwise we create a new vma for the mmap.)

The devil is in the details, and it's much easier to say that if not
using full-ppgtt, expect relocations.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx