On 12/08/2021 07:03, Hannes Reinecke wrote:
Go for it.
I'm not particularly keen on the 'scsi_cmd_to_rq(cmd)->tag' construct,
as this implies that 'scsi_cmd_to_rq()' has to be a define, not a function.
What is the problem exactly?
Having a wrapper for scsi_cmd_to_tag() resolves that.
So which tag does scsi_cmd_to_tag() return? The request tag?
scsi_cmnd.tag? Or, if there is an ata_queued_cmd associated with the
scsi_cmnd, ata_queued_cmd.tag? Maybe it's blindingly obvious, but I
don't see value in introducing possibility of misuse.
Having said that, scsi_cmnd.tag could be removed:
---->8----
[PATCH 1/3] scsi: wd719: Stop using scsi_cmnd.tag
Use scsi_cmd_to_rq(cmd)->tag instead.
Signed-off-by: John Garry <john.garry@xxxxxxxxxx>
---
drivers/scsi/wd719x.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/wd719x.c b/drivers/scsi/wd719x.c
index edc8a139a60d..4f4ebb9c5b09 100644
--- a/drivers/scsi/wd719x.c
+++ b/drivers/scsi/wd719x.c
@@ -467,13 +467,13 @@ static int wd719x_abort(struct scsi_cmnd *cmd)
struct wd719x_scb *scb = scsi_cmd_priv(cmd);
struct wd719x *wd = shost_priv(cmd->device->host);
- dev_info(&wd->pdev->dev, "abort command, tag: %x\n", cmd->tag);
+ dev_info(&wd->pdev->dev, "abort command, tag: %x\n",
scsi_cmd_to_rq(cmd)->tag);
- action = /*cmd->tag ? WD719X_CMD_ABORT_TAG : */WD719X_CMD_ABORT;
+ action = /*scsi_cmd_to_rq(cmd)->tag ? WD719X_CMD_ABORT_TAG :
*/WD719X_CMD_ABORT;
spin_lock_irqsave(wd->sh->host_lock, flags);
result = wd719x_direct_cmd(wd, action, cmd->device->id,
- cmd->device->lun, cmd->tag, scb->phys, 0);
+ cmd->device->lun, scsi_cmd_to_rq(cmd)->tag, scb->phys, 0);
wd719x_finish_cmd(scb, DID_ABORT);
spin_unlock_irqrestore(wd->sh->host_lock, flags);
if (result)
--
[PATCH 2/3] scsi: fnic: Stop setting scsi_cmnd.tag
It is never read. Setting it and the request tag seems dodgy
anyway.
Signed-off-by: John Garry <john.garry@xxxxxxxxxx>
---
drivers/scsi/fnic/fnic_scsi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/fnic/fnic_scsi.c b/drivers/scsi/fnic/fnic_scsi.c
index 0f9cedf78872..f8afbfb468dc 100644
--- a/drivers/scsi/fnic/fnic_scsi.c
+++ b/drivers/scsi/fnic/fnic_scsi.c
@@ -2213,7 +2213,7 @@ fnic_scsi_host_start_tag(struct fnic *fnic, struct
scsi_cmnd *sc)
if (IS_ERR(dummy))
return SCSI_NO_TAG;
- sc->tag = rq->tag = dummy->tag;
+ rq->tag = dummy->tag;
sc->host_scribble = (unsigned char *)dummy;
return dummy->tag;
--
[PATCH 3/3] scsi: Remove scsi_cmnd.tag
There are no users.
Signed-off-by: John Garry <john.garry@xxxxxxxxxx>
---
drivers/scsi/scsi_lib.c | 1 -
include/scsi/scsi_cmnd.h | 1 -
2 files changed, 2 deletions(-)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 9ba1aa7530a9..572673873ddf 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1540,7 +1540,6 @@ static blk_status_t scsi_prepare_cmd(struct
request *req)
scsi_init_command(sdev, cmd);
- cmd->tag = req->tag;
cmd->prot_op = SCSI_PROT_NORMAL;
if (blk_rq_bytes(req))
cmd->sc_data_direction = rq_dma_dir(req);
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index 6c5a1c1c6b1e..eaf04c9a1dfc 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -139,7 +139,6 @@ struct scsi_cmnd {
int flags; /* Command flags */
unsigned long state; /* Command completion state */
- unsigned char tag; /* SCSI-II queued command tag */
unsigned int extra_len; /* length of alignment and padding */
};
--
----8<-----
Bart, feel free to make a similar change - I don't want you to think I'm
hijacking your work.
Thanks!
