[07/22] csiostor: use internal command for LUN reset

Message ID 20200629072021.9864-8-hare@suse.de
State New
Headers show
Series
  • scsi: enable reserved commands for LLDDs
Related show

Commit Message

Hannes Reinecke June 29, 2020, 7:20 a.m.
When issuing a LUN reset we should be allocating an
internal command to avoid overwriting the original command.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/csiostor/csio_scsi.c | 48 +++++++++++++++++++++++----------------
 1 file changed, 29 insertions(+), 19 deletions(-)

Comments

Hannes Reinecke June 29, 2020, 2:59 p.m. | #1
On 6/29/20 3:51 PM, John Garry wrote:
> On 29/06/2020 08:20, Hannes Reinecke wrote:
>> When issuing a LUN reset we should be allocating an
>> internal command to avoid overwriting the original command.
>>
>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>> ---
>>   drivers/scsi/csiostor/csio_scsi.c | 48 
>> +++++++++++++++++++++++----------------
>>   1 file changed, 29 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/scsi/csiostor/csio_scsi.c 
>> b/drivers/scsi/csiostor/csio_scsi.c
>> index 00cf33573136..27001fdcdcac 100644
>> --- a/drivers/scsi/csiostor/csio_scsi.c
>> +++ b/drivers/scsi/csiostor/csio_scsi.c
>> @@ -2057,10 +2057,12 @@ csio_tm_cbfn(struct csio_hw *hw, struct 
>> csio_ioreq *req)
>>   static int
>>   csio_eh_lun_reset_handler(struct scsi_cmnd *cmnd)
>>   {
>> -    struct csio_lnode *ln = shost_priv(cmnd->device->host);
>> +    struct scsi_cmnd *reset_cmnd;
>> +    struct scsi_device *sdev = cmnd->device;
>> +    struct csio_lnode *ln = shost_priv(sdev->host);
>>       struct csio_hw *hw = csio_lnode_to_hw(ln);
>>       struct csio_scsim *scsim = csio_hw_to_scsim(hw);
>> -    struct csio_rnode *rn = (struct csio_rnode 
>> *)(cmnd->device->hostdata);
>> +    struct csio_rnode *rn = (struct csio_rnode *)(sdev->hostdata);
>>       struct csio_ioreq *ioreq = NULL;
>>       struct csio_scsi_qset *sqset;
>>       unsigned long flags;
>> @@ -2073,13 +2075,13 @@ csio_eh_lun_reset_handler(struct scsi_cmnd *cmnd)
>>           goto fail;
>>       csio_dbg(hw, "Request to reset LUN:%llu (ssni:0x%x tgtid:%d)\n",
>> -              cmnd->device->lun, rn->flowid, rn->scsi_id);
>> +              sdev->lun, rn->flowid, rn->scsi_id);
>>       if (!csio_is_lnode_ready(ln)) {
>>           csio_err(hw,
>>                "LUN reset cannot be issued on non-ready"
>>                " local node vnpi:0x%x (LUN:%llu)\n",
>> -             ln->vnp_flowid, cmnd->device->lun);
>> +             ln->vnp_flowid, sdev->lun);
>>           goto fail;
>>       }
>> @@ -2099,17 +2101,22 @@ csio_eh_lun_reset_handler(struct scsi_cmnd *cmnd)
>>           csio_err(hw,
>>                "LUN reset cannot be issued on non-ready"
>>                " remote node ssni:0x%x (LUN:%llu)\n",
>> -             rn->flowid, cmnd->device->lun);
>> +             rn->flowid, sdev->lun);
>>           goto fail;
>>       }
>> +    reset_cmnd = scsi_get_internal_cmd(sdev, DMA_NONE, REQ_NOWAIT);
> 
> out of curiosity, do we use the tag at all or need to allocate a request 
> here?
> 
> it seems that the current code just scribbles on the scmd host scribble 
> parts, and now we use the internal scmd host scribble parts instead - so 
> I'm not sure what we're really improving here.
> 

Primary goal is to register the TMF with the block layer to make it 
aware that some command is outstanding.
Probably not that crucial, so I'll be dropping it for the next round.

Cheers,

Hannes

Patch

diff --git a/drivers/scsi/csiostor/csio_scsi.c b/drivers/scsi/csiostor/csio_scsi.c
index 00cf33573136..27001fdcdcac 100644
--- a/drivers/scsi/csiostor/csio_scsi.c
+++ b/drivers/scsi/csiostor/csio_scsi.c
@@ -2057,10 +2057,12 @@  csio_tm_cbfn(struct csio_hw *hw, struct csio_ioreq *req)
 static int
 csio_eh_lun_reset_handler(struct scsi_cmnd *cmnd)
 {
-	struct csio_lnode *ln = shost_priv(cmnd->device->host);
+	struct scsi_cmnd *reset_cmnd;
+	struct scsi_device *sdev = cmnd->device;
+	struct csio_lnode *ln = shost_priv(sdev->host);
 	struct csio_hw *hw = csio_lnode_to_hw(ln);
 	struct csio_scsim *scsim = csio_hw_to_scsim(hw);
-	struct csio_rnode *rn = (struct csio_rnode *)(cmnd->device->hostdata);
+	struct csio_rnode *rn = (struct csio_rnode *)(sdev->hostdata);
 	struct csio_ioreq *ioreq = NULL;
 	struct csio_scsi_qset *sqset;
 	unsigned long flags;
@@ -2073,13 +2075,13 @@  csio_eh_lun_reset_handler(struct scsi_cmnd *cmnd)
 		goto fail;
 
 	csio_dbg(hw, "Request to reset LUN:%llu (ssni:0x%x tgtid:%d)\n",
-		      cmnd->device->lun, rn->flowid, rn->scsi_id);
+		      sdev->lun, rn->flowid, rn->scsi_id);
 
 	if (!csio_is_lnode_ready(ln)) {
 		csio_err(hw,
 			 "LUN reset cannot be issued on non-ready"
 			 " local node vnpi:0x%x (LUN:%llu)\n",
-			 ln->vnp_flowid, cmnd->device->lun);
+			 ln->vnp_flowid, sdev->lun);
 		goto fail;
 	}
 
@@ -2099,17 +2101,22 @@  csio_eh_lun_reset_handler(struct scsi_cmnd *cmnd)
 		csio_err(hw,
 			 "LUN reset cannot be issued on non-ready"
 			 " remote node ssni:0x%x (LUN:%llu)\n",
-			 rn->flowid, cmnd->device->lun);
+			 rn->flowid, sdev->lun);
 		goto fail;
 	}
 
+	reset_cmnd = scsi_get_internal_cmd(sdev, DMA_NONE, REQ_NOWAIT);
+	if (!reset_cmnd) {
+		csio_err(hw, "No free TMF request\n");
+		goto fail;
+	}
 	/* Get a free ioreq structure - SM is already set to uninit */
 	ioreq = csio_get_scsi_ioreq_lock(hw, scsim);
 
 	if (!ioreq) {
 		csio_err(hw, "Out of IO request elements. Active # :%d\n",
 			 scsim->stats.n_active);
-		goto fail;
+		goto fail_ret_cmnd;
 	}
 
 	sqset			= &hw->sqset[ln->portid][smp_processor_id()];
@@ -2119,11 +2126,11 @@  csio_eh_lun_reset_handler(struct scsi_cmnd *cmnd)
 	ioreq->iq_idx		= sqset->iq_idx;
 	ioreq->eq_idx		= sqset->eq_idx;
 
-	csio_scsi_cmnd(ioreq)	= cmnd;
-	cmnd->host_scribble	= (unsigned char *)ioreq;
-	cmnd->SCp.Status	= 0;
+	csio_scsi_cmnd(ioreq)	= reset_cmnd;
+	reset_cmnd->host_scribble	= (unsigned char *)ioreq;
+	reset_cmnd->SCp.Status	= 0;
 
-	cmnd->SCp.Message	= FCP_TMF_LUN_RESET;
+	reset_cmnd->SCp.Message	= FCP_TMF_LUN_RESET;
 	ioreq->tmo		= CSIO_SCSI_LUNRST_TMO_MS / 1000;
 
 	/*
@@ -2140,7 +2147,7 @@  csio_eh_lun_reset_handler(struct scsi_cmnd *cmnd)
 	sld.level = CSIO_LEV_LUN;
 	sld.lnode = ioreq->lnode;
 	sld.rnode = ioreq->rnode;
-	sld.oslun = cmnd->device->lun;
+	sld.oslun = sdev->lun;
 
 	spin_lock_irqsave(&hw->lock, flags);
 	/* Kick off TM SM on the ioreq */
@@ -2156,14 +2163,14 @@  csio_eh_lun_reset_handler(struct scsi_cmnd *cmnd)
 	csio_dbg(hw, "Waiting max %d secs for LUN reset completion\n",
 		    count * (CSIO_SCSI_TM_POLL_MS / 1000));
 	/* Wait for completion */
-	while ((((struct scsi_cmnd *)csio_scsi_cmnd(ioreq)) == cmnd)
+	while ((((struct scsi_cmnd *)csio_scsi_cmnd(ioreq)) == reset_cmnd)
 								&& count--)
 		msleep(CSIO_SCSI_TM_POLL_MS);
 
 	/* LUN reset timed-out */
-	if (((struct scsi_cmnd *)csio_scsi_cmnd(ioreq)) == cmnd) {
+	if (((struct scsi_cmnd *)csio_scsi_cmnd(ioreq)) == reset_cmnd) {
 		csio_err(hw, "LUN reset (%d:%llu) timed out\n",
-			 cmnd->device->id, cmnd->device->lun);
+			 sdev->id, sdev->lun);
 
 		spin_lock_irq(&hw->lock);
 		csio_scsi_drvcleanup(ioreq);
@@ -2174,11 +2181,12 @@  csio_eh_lun_reset_handler(struct scsi_cmnd *cmnd)
 	}
 
 	/* LUN reset returned, check cached status */
-	if (cmnd->SCp.Status != FW_SUCCESS) {
+	if (reset_cmnd->SCp.Status != FW_SUCCESS) {
 		csio_err(hw, "LUN reset failed (%d:%llu), status: %d\n",
-			 cmnd->device->id, cmnd->device->lun, cmnd->SCp.Status);
-		goto fail;
+			 sdev->id, sdev->lun, reset_cmnd->SCp.Status);
+		goto fail_ret_cmnd;
 	}
+	scsi_put_internal_cmd(reset_cmnd);
 
 	/* LUN reset succeeded, Start aborting affected I/Os */
 	/*
@@ -2196,7 +2204,7 @@  csio_eh_lun_reset_handler(struct scsi_cmnd *cmnd)
 	if (retval != 0) {
 		csio_err(hw,
 			 "Attempt to abort I/Os during LUN reset of %llu"
-			 " returned %d\n", cmnd->device->lun, retval);
+			 " returned %d\n", sdev->lun, retval);
 		/* Return I/Os back to active_q */
 		spin_lock_irq(&hw->lock);
 		list_splice_tail_init(&local_q, &scsim->active_q);
@@ -2207,12 +2215,14 @@  csio_eh_lun_reset_handler(struct scsi_cmnd *cmnd)
 	CSIO_INC_STATS(rn, n_lun_rst);
 
 	csio_info(hw, "LUN reset occurred (%d:%llu)\n",
-		  cmnd->device->id, cmnd->device->lun);
+		  sdev->id, sdev->lun);
 
 	return SUCCESS;
 
 fail_ret_ioreq:
 	csio_put_scsi_ioreq_lock(hw, scsim, ioreq);
+fail_ret_cmnd:
+	scsi_put_internal_cmd(reset_cmnd);
 fail:
 	CSIO_INC_STATS(rn, n_lun_rst_fail);
 	return FAILED;