linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Liu Ping Fan <kernelfans@gmail.com>
To: linux-scsi@vger.kernel.org
Cc: Robert Jennings <rcj@linux.vnet.ibm.com>,
	Paolo Bonzini <pbonzini@redhat.com>
Subject: [PATCH] scsi: ibmvscsi: protect abort handler from done-scmd in flight
Date: Thu,  5 Jun 2014 14:16:44 +0800	[thread overview]
Message-ID: <1401949004-17725-1-git-send-email-pingfank@linux.vnet.ibm.com> (raw)

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@linux.vnet.ibm.com>
---
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);
-- 
1.8.1.4


             reply	other threads:[~2014-06-05  6:13 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-05  6:16 Liu Ping Fan [this message]
2014-06-05  8:00 ` [PATCH] scsi: ibmvscsi: protect abort handler from done-scmd in flight Paolo Bonzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1401949004-17725-1-git-send-email-pingfank@linux.vnet.ibm.com \
    --to=kernelfans@gmail.com \
    --cc=linux-scsi@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=rcj@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).