On 8/14/23 11:18, Bart Van Assche wrote:

On 8/13/23 18:19, Damien Le Moal wrote:

quoted

On 8/12/23 06:35, Bart Van Assche wrote:

quoted

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.

quoted

quoted

+++ 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.

OK. I will let Martin decide that :)
But at the very least, may be mention in the commit message that you also add
the unit tests associated with the change ?

quoted

quoted

+	/*
+	 * 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.

Oops. Missed that one !

Another remark: we'll need this sorting only for devices that are zoned and do
not need zone write locking. For other cases (most cases in fact), we don't and
this could have some performance impact (e.g. fast RAID systems). Is there a way
to have this eh_prepare_resubmit to do nothing for regular devices to avoid that ?

Thanks,

Bart.

-- 
Damien Le Moal
Western Digital Research