Re: [PATCH v8 3/9] scsi: core: Call .eh_prepare_resubmit() before resubmitting

[Bart Van Assche <bvanassche@acm.org>]

On 8/13/23 18:19, Damien Le Moal wrote:
> On 8/12/23 06:35, Bart Van Assche wrote:
>> Make the error handler call .eh_prepare_resubmit() before resubmitting
> 
> This reads like the eh_prepare_resubmit callback already exists. But you are
> adding it. So you should state that.

Hi Damien,

I will rephrase the patch description.

>> +++ b/drivers/scsi/Makefile.kunit
>> @@ -0,0 +1 @@
>> +obj-$(CONFIG_SCSI_ERROR_TEST) += scsi_error_test.o
> 
> All the above kunit changes (and the test changes below) seem unrelated to what
> the commit message describes. Should these be split into a different patch ?

Some people insist on including unit tests in the same patch as
the patch that introduces the code that is being tested. I can
move the unit test into a separate patch if that is preferred.

>> +	/*
>> +	 * Call .eh_prepare_resubmit for each range of commands with identical
>> +	 * ULD driver pointer.
>> +	 */
>> +	list_for_each_entry_safe(scmd, next, done_q, eh_entry) {
>> +		struct scsi_driver *uld = scsi_cmd_to_driver(scmd);
>> +		struct list_head *prev, uld_cmd_list;
>> +
>> +		while (&next->eh_entry != done_q &&
>> +		       scsi_cmd_to_driver(next) == uld)
>> +			next = list_next_entry(next, eh_entry);
>> +		if (!uld->eh_prepare_resubmit)
>> +			continue;
>> +		prev = scmd->eh_entry.prev;
>> +		list_cut_position(&uld_cmd_list, prev, next->eh_entry.prev);
>> +		uld->eh_prepare_resubmit(&uld_cmd_list);
> 
> Is it guaranteed that all uld implement eh_prepare_resubmit ?

That is not guaranteed. Hence the if (!uld->eh_prepare_resubmit)
test in the above loop.

Thanks,

Bart.