Hi Mike, Thanks for the detailed input. I will apply the below changes and will resubmit the patches. Regards, Muneendra. -----Original Message----- From: Mike Christie [mailto:michael.christie@xxxxxxxxxx] Sent: Thursday, November 5, 2020 9:29 PM To: Muneendra <muneendra.kumar@xxxxxxxxxxxx>; linux-scsi@xxxxxxxxxxxxxxx; hare@xxxxxxx Cc: jsmart2021@xxxxxxxxx; emilne@xxxxxxxxxx; mkumar@xxxxxxxxxx Subject: Re: [PATCH v6 3/4] scsi_transport_fc: Added a new rport state FC_PORTSTATE_MARGINAL On 11/5/20 12:09 AM, Muneendra wrote: > int fc_block_scsi_eh(struct scsi_cmnd *cmnd) > { > struct fc_rport *rport = > starget_to_rport(scsi_target(cmnd->device)); > + int ret = 0; > > if (WARN_ON_ONCE(!rport)) > return FAST_IO_FAIL; > > - return fc_block_rport(rport); > + ret = fc_block_rport(rport); > + /* > + * Clear the SCMD_NORETRIES_ABORT bit if the Port state has > + * changed from marginal to online due to > + * fc_remote_port_delete and fc_remote_port_add > + */ > + if (rport->port_state != FC_PORTSTATE_MARGINAL) > + clear_bit(SCMD_NORETRIES_ABORT, &cmnd->state); > + return ret; > } Hey sorry for the late reply. I was trying to test some things out but am not sure if all drivers work the same. For the code above, what will happen if we have passed that check in the driver, then the driver does the report del and add sequence? Let's say it's initially calling the abort callout, and we passed that check, we then do the del/add seqeuence, what will happen next? Do the fc drivers return success or failure for the abort call. What happens for the other callouts too? If failure, then the eh escalates and when we call the next callout, and we hit the check above and will clear it, so we are ok. If success then we would not get a chance to clear it right? If this is the case, then I think you need to instead go the route where you add the eh cmd completion/decide_disposition callout. You would call it in scmd_eh_abort_handler, scsi_eh_bus_device_reset, etc when we are deciding if we want to retry/fail the command. In this approach you do not need the eh_timed_out changes, since we only seem to care about the port state after the eh callout has completed.
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature
