Re: [PATCH 3/3] tests/gem_userptr_benchmark: Benchmarking userptr surfaces and impact

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 04/23/2014 06:17 PM, Volkin, Bradley D wrote:
[snip]
>> +static int gem_userptr(int fd, void *ptr, int size, int read_only, uint32_t *handle)
>> +{
>> +	struct local_i915_gem_userptr userptr;
>> +	int ret;
>> +
>> +	userptr.user_ptr = (uintptr_t)ptr;
>> +	userptr.user_size = size;
>> +	userptr.flags = userptr_flags;
>> +	if (read_only)
>> +		userptr.flags |= LOCAL_I915_USERPTR_READ_ONLY;
>> +
>> +	ret = drmIoctl(fd, LOCAL_IOCTL_I915_GEM_USERPTR, &userptr);
>> +	if (ret)
>> +		ret = errno;
>> +	igt_skip_on_f(ret == ENODEV &&
>> +		      (userptr_flags & LOCAL_I915_USERPTR_UNSYNCHRONIZED) == 0,
>> +		      "Skipping, synchronized mappings with no kernel CONFIG_MMU_NOTIFIER?");
>
> I missed it the first time around, but the condition here doesn't
> match the other test; it's missing the '&& !read_only'. It looks
> like read_only will always be 0 in this test though, so probably
> not an issue.
>
> Reviewed-by: Brad Volkin <bradley.d.volkin@intel.com>

Good catch! It does not matter in the benchmark but I've sent a respin 
for consistency and clarity.

Thanks,

Tvrtko