Re: [PATCH] [i-g-t] tests/intel/xe_vm:Reduce n_execs for bind-array-enobufs

["Randhawa, Jagmeet" <jagmeet.randhawa@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 3095 bytes --]


On 10/23/2024 7:12 AM, Cavitt, Jonathan wrote:
> -----Original Message-----
> From: Randhawa, Jagmeet<jagmeet.randhawa@intel.com> 
> Sent: Tuesday, October 22, 2024 1:12 PM
> Cc:igt-dev@lists.freedesktop.org; Cavitt, Jonathan<jonathan.cavitt@intel.com>; Zuo, Alex<alex.zuo@intel.com>; Konieczny, Kamil<kamil.konieczny@intel.com>; Randhawa, Jagmeet<jagmeet.randhawa@intel.com>
> Subject: [PATCH] [i-g-t] tests/intel/xe_vm:Reduce n_execs for bind-array-enobufs
>> The bind-array-enobufs test in xe_vm.c is intended to trigger
>> an -ENOBUFS error by submitting an oversized bind array.
>> After encountering and handling the error, the test reduces
>> n_execs by half and retries the operation. On some environments
>> with stricter resource limits (e.g. simulator),
> Is it "just" simulator this is an issue on?  Because if so, we could probably
> use igt_run_in_simulation() to narrow the scope:
>
> """
> 		xe_cork_fini(&cork);
> -		n_execs = n_execs / 2;
> +		n_execs /= igt_run_in_simulation() ? 4 : 2;
> }
> """
>
> If not, then
> Reviewed-by: Jonathan Cavitt<jonathan.cavitt@intel.com>
> -Jonathan Cavitt

Thank you for your comment and review Jonathan. Oddly when I tested the 
following line

  "n_execs /= igt_run_in_simulation() ? 4 : 2;" The test fails as it was 
before the original patch was sent

so I suspect igt_run_in_simulation() is returning false and we are 
taking the "2" integer in this case.

I believe it is just simulator we are facing this issue on, however I 
think leaving the code as

"n_execs = n_execs / 4;" doesn't do any harm if we run it on any other platform such as hardware
, as it does not affect the basis of the test which is to trigger enobufs error, handle it, and then
run with less n_execs to pass on the next iterations. This patch will just accelerate the test on hardware.
If this works with you, may I ask if I still have your reviewed-by? I can change it and investigate
if you feel strongly about it. Thanks again.

Jagmeet

>
>> halving n_execs isn't sufficient to prevent the ENOBUFS error on the retry.
>> Reducing n_execs further to n_execs / 4 allows the test to pass
>> successfully. This patch modifies the test to reduce n_execs
>> to n_execs / 4 after an ENOBUFS error is handled. This ensures
>> compatibility with environments that have tighter resource
>> constraints while maintaining the test's integrity.
>>
>> Cc: Jonathan Cavitt<jonathan.cavitt@intel.com>
>> Signed-off-by: Jagmeet Randhawa<jagmeet.randhawa@intel.com>
>> ---
>>   tests/intel/xe_vm.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tests/intel/xe_vm.c b/tests/intel/xe_vm.c
>> index 7a8740b69..e78ddd0e5 100644
>> --- a/tests/intel/xe_vm.c
>> +++ b/tests/intel/xe_vm.c
>> @@ -957,7 +957,7 @@ test_bind_array(int fd, struct drm_xe_engine_class_instance *eci, int n_execs,
>>   		xe_cork_end(&cork);
>>   		xe_cork_wait_done(&cork);
>>   		xe_cork_fini(&cork);
>> -		n_execs = n_execs / 2;
>> +		n_execs = n_execs / 4;
>>   	}
>>   
>>   	xe_vm_bind_array(fd, vm, bind_exec_queue, bind_ops, n_execs, sync, 1);
>> -- 
>> 2.34.1
>>
>>

[-- Attachment #2: Type: text/html, Size: 4948 bytes --]