[igt-dev] [PATCH i-g-t] gem_exec_reloc & gem_shrink: Added gem_require_mappable_ggtt to check mappable aperture

[Chris Wilson]

Quoting Tvrtko Ursulin (2020-11-30 10:04:17)
> 
> On 30/11/2020 02:03, Arjun Melkaveri wrote:
> > Added gem_require_mappable_ggtt to check mappable aperture.
> > This is to avoid any test crash that might happen
> > if mappable aperture is not avilable.
> > 
> > Cc: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> > Signed-off-by: Arjun Melkaveri <arjun.melkaveri at intel.com>
> > ---
> >   tests/i915/gem_exec_reloc.c | 3 +++
> >   tests/i915/gem_shrink.c     | 2 ++
> >   2 files changed, 5 insertions(+)
> > 
> > diff --git a/tests/i915/gem_exec_reloc.c b/tests/i915/gem_exec_reloc.c
> > index 8dcb24a6..596b1157 100644
> > --- a/tests/i915/gem_exec_reloc.c
> > +++ b/tests/i915/gem_exec_reloc.c
> > @@ -1217,6 +1217,7 @@ igt_main
> >                       if (!(f->flags & NORELOC)) {
> >                               igt_subtest_f("%srange%s",
> >                                             f->basic ? "basic-" : "", f->name) {
> > +                                     gem_require_mappable_ggtt(fd);
> 
>  From the code and commit message it is not immediately obvious to me 
> why basic_range needs the aperture, Chris? Answer to that will drive the 
> solution.

It does not.

> 
> >                                       igt_while_interruptible(f->flags & INTERRUPTIBLE)
> >                                               basic_range(fd, f->flags);
> >                               }
> > @@ -1264,6 +1265,7 @@ igt_main
> >       }
> >   
> >       igt_subtest_with_dynamic("basic-spin") {
> > +
> 
> !
> 
> >               __for_each_physical_engine(fd, e) {
> >                       igt_dynamic_f("%s", e->name)
> >                               active_spin(fd, e->flags);
> > @@ -1278,6 +1280,7 @@ igt_main
> >       }
> >   
> >       igt_subtest_with_dynamic("basic-many-active") {
> > +             gem_require_mappable_ggtt(fd);
> >               __for_each_physical_engine(fd, e) {
> >                       igt_dynamic_f("%s", e->name)
> >                               many_active(fd, e->flags);
> 
> Same for many_active - maybe tests should use ggtt size and not 
> aperture, with some tweaks to make size/runtime sane?

It does not. This is somebody hacking over a major kernel bug.

> > diff --git a/tests/i915/gem_shrink.c b/tests/i915/gem_shrink.c
> > index dba62c8f..094975af 100644
> > --- a/tests/i915/gem_shrink.c
> > +++ b/tests/i915/gem_shrink.c
> > @@ -432,6 +432,8 @@ igt_main
> >               fd = drm_open_driver(DRIVER_INTEL);
> >               igt_require_gem(fd);
> >   
> > +             gem_require_mappable_ggtt(fd);
> > +
> >               /*
> >                * Spawn enough processes to use all memory, but each only
> >                * uses half the available mappable aperture ~128MiB.
> > 
> 
> And for this one the same I think. Apart from the subtests using 
> mmap-gtt other ones could probably be made work by calculating the 
> working set in a different way. Possibly just use the ggtt size and skip 
> tests which need aperture if no aperture. Chris would that be acceptable 
> or there is a special reason to have N clients with each using an 
> aperture sized amount of memory, to total RAM size?

shrink is system memory limits.
-Chris