Re: [PATCH] scsi: ibmvscsi: protect abort handler from done-scmd in flight

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Il 05/06/2014 08:16, Liu Ping Fan ha scritto:
Take the following scene in guest:
seqA: scsi_done() -> gapX (before taking REQ_ATOM_COMPLETE)
seqB: scmd_eh_abort_handler()-> ...-> ibmvscsi_eh_abort_handler()->
      ...->scsi_put_command(scmd)

If seqA is scheduled at gapX, and seqB reclaims scmd. Then when seqA
comes back, it tries to access the scmd when is turned back to mempool.

This patch fixes the race by ensuring when ibmvscsi_eh_abort_handler()
returns, no scsi_done is in flight

Signed-off-by: Liu Ping Fan <pingfank@xxxxxxxxxxxxxxxxxx>
---
When trying to figure the scsi_cmnd in flight issue, I learned from Paolo (thanks).
He showed me the way how virtscsi resolves the race between abort-handler
and scsi_done in flight. And I think that this method is also needed by ibmvscsi.
---
 drivers/scsi/ibmvscsi/ibmvscsi.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
index fa76440..325cef6 100644
--- a/drivers/scsi/ibmvscsi/ibmvscsi.c
+++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
@@ -1828,16 +1828,19 @@ static void ibmvscsi_handle_crq(struct viosrp_crq *crq,

 	if ((crq->status != VIOSRP_OK && crq->status != VIOSRP_OK2) && evt_struct->cmnd)
 		evt_struct->cmnd->result = DID_ERROR << 16;
-	if (evt_struct->done)
-		evt_struct->done(evt_struct);
-	else
-		dev_err(hostdata->dev, "returned done() is NULL; not running it!\n");

 	/*
 	 * Lock the host_lock before messing with these structures, since we
 	 * are running in a task context
+	 * Also, this lock helps ibmvscsi_eh_abort_handler() to shield the
+	 * scsi_done() in flight.
 	 */
 	spin_lock_irqsave(evt_struct->hostdata->host->host_lock, flags);
+	if (evt_struct->done)
+		evt_struct->done(evt_struct);
+	else
+		dev_err(hostdata->dev, "returned done() is NULL; not running it!\n");
+
 	list_del(&evt_struct->list);
 	free_event_struct(&evt_struct->hostdata->pool, evt_struct);
 	spin_unlock_irqrestore(evt_struct->hostdata->host->host_lock, flags);


I think this is not necessary because ibmvscsi places TMFs and commands in the same queue; virtio-scsi instead uses two different queues.

So ibmvscsi_handle_crq processes all completed requests, which naturally serializes the processing of the TMF and the command.

Paolo
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]
  Powered by Linux