On 5/24/22 13:16, Steffen Maier wrote:
On 5/24/22 08:15, Hannes Reinecke wrote:
When sending a TMF we're only concerned with the rport and the LUN ID,
so use struct fc_rport as argument for qedf_initiate_tmf().
Signed-off-by: Hannes Reinecke <hare@xxxxxxxx>
Reviewed-by: Christoph Hellwig <hch@xxxxxx>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@xxxxxxx>
Cc: Saurav Kashyap <skashyap@xxxxxxxxxxx>
---
drivers/scsi/qedf/qedf.h | 3 +-
drivers/scsi/qedf/qedf_io.c | 69 ++++++++++-------------------------
drivers/scsi/qedf/qedf_main.c | 19 +++++-----
3 files changed, 31 insertions(+), 60 deletions(-)
@@ -2432,33 +2425,10 @@ int qedf_initiate_tmf(struct scsi_cmnd
*sc_cmd, u8 tm_flags)
(tm_flags == FCP_TMF_TGT_RESET) ? "TARGET RESET" :
"LUN RESET");
- if (qedf_priv(sc_cmd)->io_req) {
- io_req = qedf_priv(sc_cmd)->io_req;
- ref_cnt = kref_read(&io_req->refcount);
- QEDF_ERR(NULL,
- "orig io_req = %p xid = 0x%x ref_cnt = %d.\n",
- io_req, io_req->xid, ref_cnt);
- }
-
- rval = fc_remote_port_chkready(rport);
- if (rval) {
- QEDF_ERR(NULL, "device_reset rport not ready\n");
- rc = FAILED;
- goto tmf_err;
- }
-
- rc = fc_block_scsi_eh(sc_cmd);
+ rc = fc_block_rport(rport);
diff --git a/drivers/scsi/qedf/qedf_main.c
b/drivers/scsi/qedf/qedf_main.c
index 18dc68d577b6..85ccfbc5cd26 100644
--- a/drivers/scsi/qedf/qedf_main.c
+++ b/drivers/scsi/qedf/qedf_main.c
@@ -773,7 +773,7 @@ static int qedf_eh_abort(struct scsi_cmnd *sc_cmd)
goto drop_rdata_kref;
}
- rc = fc_block_scsi_eh(sc_cmd);
+ rc = fc_block_rport(rport);
if (rc)
goto drop_rdata_kref;
Why replace the fc_block helper here in the abort handler?
Isn't the scope of the abort hander exactly the one scsi_cmnd?
The description of this patch is about changes to TMF (so I understand
the change in qedf_initiate_tmf() above for device or target reset),
i.e. not abort.
Admittedly, it shouldn't be a problem functional-wise, as
fc_block_scsi_eh() delegates internally to fc_block_rport(), but it
looks odd to me nonetheless.
This change seems inconsistent with the other patches in this series.
You don't plan to get rid of fc_block_scsi_eh(), do you?
Oh, later in patch 09 you write:
"Use fc_block_rport() instead of fc_block_scsi_eh() as the latter
will be deprecated."
Why?
Yeah, sorry, that is misleading. I had an earlier patchset where I tried
to phase it out, only to find out that this is not the best of ideas.
So no worries, everything will stay as-is in that area.
Will be modifying the patch (and the description) for the next round.
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@xxxxxxx +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman
