[Intel-gfx] [PATCH i-g-t 2/4] gem_create: Test huge object creation as a basic test

[Chris Wilson]

On Fri, Mar 31, 2017 at 08:08:28AM +0100, Tvrtko Ursulin wrote:
> 
> On 30/03/2017 18:22, Chris Wilson wrote:
> >On Thu, Mar 30, 2017 at 05:58:07PM +0100, Tvrtko Ursulin wrote:
> >>From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> >>
> >>It is hard to imagine a more basic test than this one.
> >>
> >>Also removed the skip on simulation since I don't know why
> >>would that be needed here.
> >>
> >>Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> >>---
> >> tests/gem_create.c | 10 ++++++----
> >> 1 file changed, 6 insertions(+), 4 deletions(-)
> >>
> >>diff --git a/tests/gem_create.c b/tests/gem_create.c
> >>index de7b82094545..f687b7b40be4 100644
> >>--- a/tests/gem_create.c
> >>+++ b/tests/gem_create.c
> >>@@ -44,6 +44,7 @@
> >> #include <sys/stat.h>
> >> #include <sys/time.h>
> >> #include <getopt.h>
> >>+#include <limits.h>
> >>
> >> #include <drm.h>
> >>
> >>@@ -95,10 +96,13 @@ static void invalid_flag_test(int fd)
> >>
> >> static void invalid_size_test(int fd)
> >> {
> >>-	int handle;
> >>+	uint32_t handle;
> >>
> >> 	handle = __gem_create(fd, 0);
> >> 	igt_assert(!handle);
> >>+
> >>+	handle = __gem_create(fd, INT_MAX * 4096UL + 1);
> >
> >Why is that an invalid size? Invalid huge in terms of API might arguably
> >be 1<<virtual_bits + 1, but otherwise our only limitation is that it
> >has to be >0 and page aligned.
> 
> Because of the comment above the WARN I am removing in "drm/i915:
> Remove user triggerable WARN from i915_gem_object_create". We cannot
> support larger ones with the combination of sg_table data types and
> how we use them (unsigned int nents).

That's an implementation limitation, not an abi one. It is really
important that we do not enshrine kernel internals as expectations,
especially not as a basic test - the expectation is that we will support
massive objects. Having a reality check test to see how far we can get
is useful.

For example, lazy population of pages is an abi feature (I think so, at
least all userspace takes that into account, but I don't know if anyone
is really depending on it -- certainly due to the limitations we have in
place lazy is good, and userspace does try to take advantage of that,
and compensates for it at others, but whether anyone would break because
of a change to population semantics, not sure), so we could reasonably
allocate all pot of sizes and check the kernel doesn't reject them, until
the overflow check at -4095.
 
> >/* Only multiples of page size (4096) are allowed. Check all likely
> > * misalignments from pot boundaries to check validity and possibility
> > * of incorrect overflow.
> > */
> > for (int order = 0; order < 64; order++) {
> > 	uint64_t size = (uint64_t)1 << order;
> >	igt_assert(!__gem_create(fd, size - 1));
> >	igt_assert(!__gem_create(fd, size + 1));
> >	if (order < 12)
> >		igt_assert(!__gem_create(fd, size + 1));
> >}
> >
> >Still enshrines knowlege of PAGE_SIZE into the ABI. Meh.
> 
> We round up for the user transparently which I am actually making
> the other subtest in this file test in a later patch.

Bah, I keep creating objects at too low a level where that rounding
doesn't occur automatically :)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre