Re: [PATCH 1/3] tests/gem_userptr_blits: Expanded userptr test cases

Daniel Vetter
2014-04-23T17:53:43Z

On Wed, Apr 23, 2014 at 08:32:27AM -0700, Volkin, Bradley D wrote:
> On Wed, Apr 23, 2014 at 06:33:40AM -0700, Tvrtko Ursulin wrote:
> >
> > On 04/18/2014 06:10 PM, Volkin, Bradley D wrote:
> > > On Wed, Mar 19, 2014 at 04:13:04AM -0700, Tvrtko Ursulin wrote:
> > >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > >>
> > >> A set of userptr test cases to support the new feature.
> > >>
> > >> For the eviction and swapping stress testing I have extracted
> > >> some common behaviour from gem_evict_everything and made both
> > >> test cases use it to avoid duplicating the code.
> > >>
> > >> Both unsynchronized and synchronized userptr objects are
> > >> tested but the latter set of tests will be skipped if kernel
> > >> is compiled without MMU_NOTIFIERS.
> > >>
> > >> Also, with 32-bit userspace swapping tests are skipped if
> > >> the system has a lot more RAM than process address space.
> > >> Forking swapping tests are not skipped since they can still
> > >> trigger swapping by cumulative effect.
> > >>
> > >> v2:
> > >> * Fixed dmabuf test.
> > >> * Added test for rejecting read-only.
> > >> * Fixed ioctl detection for latest kernel patch.
> > >>
> > >> v3:
> > >> * Updated copy() for Gen8+.
> > >> * Fixed ioctl detection on kernels without MMU_NOTIFIERs.
> > >>
> > >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > >
> > > A number of the comments I made on patch 3 apply here as well.
> > > The sizeof(linear) thing is more prevalent in this test, though
> > > it looks like linear is at least used. Other than those comments
> > > this looks good to me.
> >
> > Believe it or not that sizeof(linear) "idiom" I inherited from other
> > blitter tests. Personally I don't care one way or another. But since it
> > makes sense to get rid of it for the benchmark part, perhaps I should
> > change it here as well to be consistent. How strongly do you feel
> > strongly about this?
>
> I think changing it would be slightly more readable, but if it's
> consistent with other blit tests then I don't feel too strongly
> about it. In fact, consistency with the other tests might be the
> better approach. I'm fine with whichever approach you prefer.

Some of the igt tests are so Gross Hacks that justifying ugliness in new
tests with consistency is ill-advised ;-)

If you find some spare cycles to clean up existing tests that would be
awesome, but I don't mind if they keep being ugly.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch