ibmvscsi: correct command abort logic
Subsystems (3)
PATCH 1/1 ibmvscsi: correct command abort logic
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.
---
ibmvscsi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/scsi/ibmvscsi/ibmvscsi.c +1 -1
@@ -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);
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
Quoted 25 lines · Robert Jennings
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_limitcount.Signed-off-by Robert Jennings <[email protected]>---
ibmvscsi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/scsi/ibmvscsi/ibmvscsi.c +1
-1@@ -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);
Signature · 3 lines
Brian King Linux on Power Virtualization IBM Linux Technology Center
My patch is wrong and the current code is correct, I wasn't following the logic correctly.
--Rob
Quoted 44 lines · * Brian King ([email protected]) wrote:
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 thefound_evt!= NULL case.-Brian
Quoted 25 lines · Robert Jennings
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_limitcount.Signed-off-by Robert Jennings <[email protected]>---
ibmvscsi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/scsi/ibmvscsi/ibmvscsi.c +1
-1@@ -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);Signature · 3 lines
Brian King Linux on Power Virtualization IBM Linux Technology Center