[igt-dev] [Intel-gfx] [PATCH i-g-t 1/8] tests/i915/gem_exec_capture: Remove pointless assert

[John Harrison]

On 11/3/2021 06:50, Tvrtko Ursulin wrote:
> On 22/10/2021 00:40, John.C.Harrison at Intel.com wrote:
>> From: John Harrison <John.C.Harrison at Intel.com>
>>
>> The 'many' test ended with an 'assert(count)', presumably meaning to
>> ensure that some objects were actually captured. However, 'count' is
>> the number of objects created not how many were captured. Plus, there
>> is already a 'require(count > 1)' at the start and count is invarient
>> so the final assert is basically pointless.
>>
>> General concensus appears to be that the test should not fail
>> irrespective of how many blobs are captured as low memory situations
>> could cause the capture to be abbreviated. So just remove the
>> pointless assert completely.
>
> Hm the test appears to be using intel_get_avail_ram_mb() to size the 
> working set. Suggesting problems with low memory situations should not 
> apply unless bugs. In which case would a better fix be improving the 
> sizing logic and changing the assert to igt_assert(blobs)?
After fixing the sysfs read code to cope with large files, I don't ever 
see abbreviated captures any more. However, other reviewers objected to 
asserting anything at all about the final count (whether full size, zero 
or whatever) on the grounds that low memory issues *might* still occur. 
And some in quite blunt language as I recall. If you think different, 
feel free to start your own patch set.

John.

>
> Regards,
>
> Tvrtko
>
>> Signed-off-by: John Harrison <John.C.Harrison at Intel.com>
>> ---
>>   tests/i915/gem_exec_capture.c | 1 -
>>   1 file changed, 1 deletion(-)
>>
>> diff --git a/tests/i915/gem_exec_capture.c 
>> b/tests/i915/gem_exec_capture.c
>> index 7e0a8b8ad..53649cdb2 100644
>> --- a/tests/i915/gem_exec_capture.c
>> +++ b/tests/i915/gem_exec_capture.c
>> @@ -524,7 +524,6 @@ static void many(int fd, int dir, uint64_t size, 
>> unsigned int flags)
>>       }
>>       igt_info("Captured %lu %"PRId64"-blobs out of a total of %lu\n",
>>            blobs, size >> 12, count);
>> -    igt_assert(count);
>>         free(error);
>>       free(offsets);
>>