* [PATCH] megaraid2 bad queuecommand return
@ 2004-11-19 9:01 Jens Axboe
0 siblings, 0 replies; 3+ messages in thread
From: Jens Axboe @ 2004-11-19 9:01 UTC (permalink / raw)
To: linux-scsi, James Bottomley
Hi,
If the command is already completed, megaraid_queue_command() must
return 0.
Signed-off-by: Jens Axboe <axboe@suse.de>
===== drivers/scsi/megaraid/megaraid_mbox.c 1.9 vs edited =====
--- 1.9/drivers/scsi/megaraid/megaraid_mbox.c 2004-10-08 17:12:09 +02:00
+++ edited/drivers/scsi/megaraid/megaraid_mbox.c 2004-11-18 23:12:10 +01:00
@@ -1623,6 +1623,7 @@ megaraid_queue_command(struct scsi_cmnd
if (!scb) { // command already completed
done(scp);
+ return 0;
}
return if_busy;
--
Jens Axboe
^ permalink raw reply [flat|nested] 3+ messages in thread
* RE: [PATCH] megaraid2 bad queuecommand return
@ 2004-11-19 14:40 Mukker, Atul
2004-11-19 15:04 ` Jens Axboe
0 siblings, 1 reply; 3+ messages in thread
From: Mukker, Atul @ 2004-11-19 14:40 UTC (permalink / raw)
To: 'Jens Axboe', 'linux-scsi@vger.kernel.org',
'James Bottomley'
Linux/Documentation/scsi/scsi_mid_low_api.txt
"
...
/**
* queuecommand - queue scsi command, invoke 'done' on completion
* @scp: pointer to scsi command object
* @done: function pointer to be invoked on completion
...
*
* Returns 0 on success.
*
* If there's a failure, return either:
*
* SCSI_MLQUEUE_DEVICE_BUSY if the device queue is full, or
* SCSI_MLQUEUE_HOST_BUSY if the entire host queue is full
*
* On both of these returns, the mid-layer will requeue the I/O
...
"
So the driver should set the if_busy flag to SCSI_MLQUEUE_DEVICE_BUSY or
SCSI_MLQUEUE_HOST_BUSY (this is appropriate in this case) instead of setting
it to 1, isn't it?
So, the following seems more appropriate
--- megaraid_mbox.c 2004-09-27 22:14:43.000000000 -0400
+++ megaraid_mbox.c.mod 2004-11-19 12:15:21.776345776 -0500
@@ -1675,7 +1675,7 @@
if (!(scb = megaraid_alloc_scb(adapter, scp))) {
scp->result = (DID_ERROR << 16);
- *busy = 1;
+ *busy = SCSI_MLQUEUE_HOST_BUSY;
return NULL;
}
@@ -1757,7 +1757,7 @@
/* Allocate a SCB and initialize passthru */
if (!(scb = megaraid_alloc_scb(adapter, scp))) {
scp->result = (DID_ERROR << 16);
- *busy = 1;
+ *busy = SCSI_MLQUEUE_HOST_BUSY;
return NULL;
}
@@ -1802,7 +1802,7 @@
*/
if (!(scb = megaraid_alloc_scb(adapter, scp))) {
scp->result = (DID_ERROR << 16);
- *busy = 1;
+ *busy = SCSI_MLQUEUE_HOST_BUSY;
return NULL;
}
ccb = (mbox_ccb_t *)scb->ccb;
@@ -1898,7 +1898,7 @@
*/
if (!(scb = megaraid_alloc_scb(adapter, scp))) {
scp->result = (DID_ERROR << 16);
- *busy = 1;
+ *busy = SCSI_MLQUEUE_HOST_BUSY;
return NULL;
}
@@ -1966,7 +1966,7 @@
// Allocate a SCB and initialize passthru
if (!(scb = megaraid_alloc_scb(adapter, scp))) {
scp->result = (DID_ERROR << 16);
- *busy = 1;
+ *busy = SCSI_MLQUEUE_HOST_BUSY;
return NULL;
}
Thanks
-Atul Mukker
Architect, Drivers and BIOS
LSI Logic Corporation
> -----Original Message-----
> From: Jens Axboe [mailto:axboe@suse.de]
> Sent: Friday, November 19, 2004 4:01 AM
> To: linux-scsi@vger.kernel.org; James Bottomley
> Subject: [PATCH] megaraid2 bad queuecommand return
>
> Hi,
>
> If the command is already completed, megaraid_queue_command()
> must return 0.
>
> Signed-off-by: Jens Axboe <axboe@suse.de>
>
> ===== drivers/scsi/megaraid/megaraid_mbox.c 1.9 vs edited =====
> --- 1.9/drivers/scsi/megaraid/megaraid_mbox.c 2004-10-08
> 17:12:09 +02:00
> +++ edited/drivers/scsi/megaraid/megaraid_mbox.c
> 2004-11-18 23:12:10 +01:00
> @@ -1623,6 +1623,7 @@ megaraid_queue_command(struct scsi_cmnd
>
> if (!scb) { // command already completed
> done(scp);
> + return 0;
> }
>
> return if_busy;
>
> --
> Jens Axboe
>
> -
> To unsubscribe from this list: send the line "unsubscribe
> linux-scsi" in the body of a message to
> majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] megaraid2 bad queuecommand return
2004-11-19 14:40 [PATCH] megaraid2 bad queuecommand return Mukker, Atul
@ 2004-11-19 15:04 ` Jens Axboe
0 siblings, 0 replies; 3+ messages in thread
From: Jens Axboe @ 2004-11-19 15:04 UTC (permalink / raw)
To: Mukker, Atul
Cc: 'linux-scsi@vger.kernel.org', 'James Bottomley'
On Fri, Nov 19 2004, Mukker, Atul wrote:
> Linux/Documentation/scsi/scsi_mid_low_api.txt
>
> "
> ...
> /**
> * queuecommand - queue scsi command, invoke 'done' on completion
> * @scp: pointer to scsi command object
> * @done: function pointer to be invoked on completion
> ...
> *
> * Returns 0 on success.
> *
> * If there's a failure, return either:
> *
> * SCSI_MLQUEUE_DEVICE_BUSY if the device queue is full, or
> * SCSI_MLQUEUE_HOST_BUSY if the entire host queue is full
> *
> * On both of these returns, the mid-layer will requeue the I/O
> ...
> "
>
> So the driver should set the if_busy flag to SCSI_MLQUEUE_DEVICE_BUSY or
> SCSI_MLQUEUE_HOST_BUSY (this is appropriate in this case) instead of setting
> it to 1, isn't it?
>
> So, the following seems more appropriate
>
> --- megaraid_mbox.c 2004-09-27 22:14:43.000000000 -0400
> +++ megaraid_mbox.c.mod 2004-11-19 12:15:21.776345776 -0500
> @@ -1675,7 +1675,7 @@
>
> if (!(scb = megaraid_alloc_scb(adapter, scp))) {
> scp->result = (DID_ERROR << 16);
> - *busy = 1;
> + *busy = SCSI_MLQUEUE_HOST_BUSY;
> return NULL;
> }
>
Nope doesn't work, only if you kill the done() call as well. non-zero
return will always trigger a requeue, which your SCSI done has alread
done. This is the bug.
--
Jens Axboe
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2004-11-19 15:04 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-11-19 14:40 [PATCH] megaraid2 bad queuecommand return Mukker, Atul
2004-11-19 15:04 ` Jens Axboe
-- strict thread matches above, loose matches on Subject: below --
2004-11-19 9:01 Jens Axboe
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).