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

[Tvrtko Ursulin]

On 31/03/2017 11:48, Chris Wilson wrote:
> 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.

We added code to the driver to prevent userspace from attempting 
something. It makes sense to have a test for that, so that one day if it 
gets removed in error the test fails, rather than memory corruption in 
the kernel happens.

We can talk about basic or not basic, but I don't see how the existence 
of such test can be argued against in principle.

Regards,

Tvrtko