From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by gabe.freedesktop.org (Postfix) with ESMTPS id 36D336EA07 for ; Wed, 2 Dec 2020 11:09:40 +0000 (UTC) Date: Wed, 2 Dec 2020 16:35:23 +0530 From: "Melkaveri, Arjun" Message-ID: <20201202110522.GA8104@arjun-NUC8i7BEH> References: <20201130020302.24843-1-arjun.melkaveri@intel.com> <160673117719.5723.1339994628703260139@build.alporthouse.com> <4cb91c29-13d0-3e6c-0270-aee74a2189bb@linux.intel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <4cb91c29-13d0-3e6c-0270-aee74a2189bb@linux.intel.com> Subject: Re: [igt-dev] [PATCH i-g-t] gem_exec_reloc & gem_shrink: Added gem_require_mappable_ggtt to check mappable aperture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" To: Tvrtko Ursulin Cc: igt-dev@lists.freedesktop.org, tvrtko.ursulin@intel.com, Chris Wilson List-ID: On Mon, Nov 30, 2020 at 12:09:10PM +0000, Tvrtko Ursulin wrote: > > On 30/11/2020 10:12, Chris Wilson wrote: > > 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 > > > > Cc: Tvrtko Ursulin > > > > Signed-off-by: Arjun Melkaveri > > > > --- > > > > 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. > > Okay, so the plan for this one should be to stop probing aperture size and > instead probe the size of the address space in use. This will be ggtt on old > gens and ppgtt on new ones. And to keep object count in check with regards > to available backing store, if required (not sure right now). Let me know if this is okay #define MAX_47b ((1ull << 47)) // address range's most signifcant bit. const uint64_t max = (AT_LEAST_GEN(devid, 8) ? MAX_47b : gem_aperture_size(i915) / 2); instead of checking gem_require_mappable_ggtt in subtest . > > > > > > > > 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. > > Okay so same as above, don't probe aperture but address space size. > > > > > > > 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. > > Yes, I was ignoring that aspect for now and focusing on the aperture size > misuse. But yes, making gem_shrink falsely pass on dg1 is not what we really > want. Probably make it explicitly use system memory objects once that API is > available. > > Regards, > > Tvrtko Any suggestion for this ? can we use gem_require_mappable_ggtt as all tests use size from gem_mappable_aperture_size. or can i modify code something like this #define MAX_32b ((1ull << 32)) alloc_size = AT_LEAST_GEN(intel_get_drm_devid(fd), 8) ? MAX_32b : gem_mappable_aperture_size(); alloc_size = alloc_size /2 Test would probabbly skip if estimated size is more than RAM + swap) -Arjun _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev