Re: [Accel-config] [PATCH v1 1/2] accel-config/test: fix batch result return logic error of sub-task fail

Dave Jiang <[email protected]> Thu, 20 Apr 2023 08:53:05 -0700
Newsgroups dev.linux.lists.accel-config
Message-ID <[email protected]>

On 4/19/23 7:13 PM, Zhu, Tony wrote:
> Dave,
> 
> Sounds it is not necessary. I only consider the situation the failure  err message could be output for all task.

We can deal with that when this is the implementation.
> 
> Seems for muti-descriptions, it is rare the part is pass and another is fail. According to the batch tests on SPR/GNR, if one fail, all will fail.
Yeah so direct return should be ok. Did you encounter an issue or was 
this just by inspection? I think returning rc at the end should be 
sufficient but may not even be needed.

> 
> Regards,
> Tony(zhu, xinzhan)
> 
> -----Original Message-----
> From: Jiang, Dave <[email protected]>
> Sent: Wednesday, April 19, 2023 11:41 PM
> To: Zhu, Tony <[email protected]>; [email protected]; Thomas, Ramesh <[email protected]>; Yu, Fenghua <[email protected]>
> Cc: Zhang, Rex <[email protected]>
> Subject: Re: [Accel-config] [PATCH v1 1/2] accel-config/test: fix batch result return logic error of sub-task fail
> 
> 
> 
> On 4/19/23 12:23 AM, Tony Zhu wrote:
>> It is not correct to return success when there is Sub-task failure.
>> Even there is one Sub-task failure, the return should be not success.
>>
>> Signed-off-by: Tony Zhu <[email protected]>
>> ---
>>    test/dsa.c | 4 ++--
>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/test/dsa.c b/test/dsa.c
>> index da5d2e6..e9cc55a 100644
>> --- a/test/dsa.c
>> +++ b/test/dsa.c
>> @@ -2257,9 +2257,9 @@ int batch_result_verify(struct batch_task *btsk, int bof, int cpfault)
>>    			}
>>    		} else {
>>    			err("Sub-task[%d] failed with stat=%d", i, sub_stat);
>> -			return ACCTEST_STATUS_FAIL;
>> +			rc = ACCTEST_STATUS_FAIL;
> 
> I don't think this change above is necessary. You are directly returning failure when a sub-task failure is detected right? Otherwise you go to the next descriptor and ignore the failure.
> 
>>    		}
>>    	}
>>    
>> -	return ACCTEST_STATUS_OK;
>> +	return rc;
>>    }