All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian King <brking@linux.vnet.ibm.com>
To: Robert Jennings <rcj@linux.vnet.ibm.com>
Cc: James.Bottomley@HansenPartnership.com,
	linux-scsi@vger.kernel.org, Santiago Leon <santil@us.ibm.com>
Subject: Re: [PATCH 1/1] ibmvscsi: correct command abort logic
Date: Mon, 08 Dec 2008 08:32:10 -0600	[thread overview]
Message-ID: <493D2FEA.5020507@linux.vnet.ibm.com> (raw)
In-Reply-To: <20081206154139.GA13534@austin.ibm.com>

I don't think this is right. This will inject an oops into the
found_evt == NULL path. Additionally, if the event is still on the
sent list, with this change, we leave it on the sent list. The
way I thought this was supposed to work was if we got back a successful
response to the abort command that implied that everything was cleaned
up for the command we were aborting, which is why we clean up after it
in the found_evt != NULL case.

-Brian

Robert Jennings wrote:
> The logic is swapped when we determine if a command that we're trying
> to abort has already completed.  This will keep us from freeing event
> structures at the correct time, fail to unmap command data properly 
> and throw off our request_limit count.
> 
> Signed-off-by: Robert Jennings <rcjenn@us.ibm.com>
> 
> ---
> 
>  ibmvscsi.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
> index 6cad175..2d9db9b 100644
> --- a/drivers/scsi/ibmvscsi/ibmvscsi.c
> +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
> @@ -1099,7 +1099,7 @@ static int ibmvscsi_eh_abort_handler(struct scsi_cmnd *cmd)
>  		}
>  	}
> 
> -	if (found_evt == NULL) {
> +	if (found_evt != NULL) {
>  		spin_unlock_irqrestore(hostdata->host->host_lock, flags);
>  		sdev_printk(KERN_INFO, cmd->device, "aborted task tag 0x%lx completed\n",
>  			    tsk_mgmt->task_tag);


-- 
Brian King
Linux on Power Virtualization
IBM Linux Technology Center



  reply	other threads:[~2008-12-08 14:33 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-12-06 15:41 [PATCH 1/1] ibmvscsi: correct command abort logic Robert Jennings
2008-12-08 14:32 ` Brian King [this message]
2008-12-08 20:49   ` Robert Jennings

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=493D2FEA.5020507@linux.vnet.ibm.com \
    --to=brking@linux.vnet.ibm.com \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=linux-scsi@vger.kernel.org \
    --cc=rcj@linux.vnet.ibm.com \
    --cc=santil@us.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.