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

Volkin, Bradley D
2014-04-23T15:32:45Z

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.

Thanks,
Brad

>
> Will see what you reply on the static initializer comment it 3/3, not
> sure what you meant there.
>
> Thanks,
>
> Tvrtko