On 22/07/2023 18:10, Mike Christie wrote:
As I see, err is initially set to SCSI_DH_OK when declared. Then if we need to retry and 2nd call to scsi_execute_cmd() passes, such that rc == 0, then err still holds the old value. This seems same as pre-existing behaviour - is this proper?
I guess this has been working by accident.
When that happens we end up returning one of the retryable error codes
to dm-multipath. It will then recall us, and before we re-run this
function we will run check_ownership and see that the last call worked.
Hi Mike,
I'd suggest that we fix this up as a prep patch, if you don't mind.
Do you mean just change the description of this patch to reflect it fixes the
second bug? It already is a prep patch. The second rdac patch is built over it.
AFAICS, this patch does not fix the bug where @err may hold a stale
value. However this broken code goes away later in the series.
stable can take the sshdr fix patches without the API change ones if they want.
I just put the sshdr one next to the API change one, so reviewers wouldn't
have to jump back and forth between the front and back of the patchset.
Do you mean move all the sshd hdr patches to a separate patchset?
No, I was just suggesting that we fix the broken code also in a separate
patch in this series, like:
diff --git a/drivers/scsi/device_handler/scsi_dh_rdac.c
b/drivers/scsi/device_handler/scsi_dh_rdac.c
index c5538645057a..5d338f12e68b 100644
--- a/drivers/scsi/device_handler/scsi_dh_rdac.c
+++ b/drivers/scsi/device_handler/scsi_dh_rdac.c
@@ -530,7 +530,7 @@ static void send_mode_select(struct work_struct *work)
container_of(work, struct rdac_controller, ms_work);
struct scsi_device *sdev = ctlr->ms_sdev;
struct rdac_dh_data *h = sdev->handler_data;
- int err = SCSI_DH_OK, retry_cnt = RDAC_RETRY_COUNT;
+ int err, retry_cnt = RDAC_RETRY_COUNT;
struct rdac_queue_data *tmp, *qdata;
LIST_HEAD(list);
unsigned char cdb[MAX_COMMAND_SIZE];
@@ -549,6 +549,7 @@ static void send_mode_select(struct work_struct *work)
spin_unlock(&ctlr->ms_lock);
retry:
+ err = SCSI_DH_OK;
memset(cdb, 0, sizeof(cdb));
data_size = rdac_failover_get(ctlr, &list, cdb);
Thanks,
John
