On 06/23/05 18:06, Moore, Eric Dean wrote:
> Luben
>
> Ok - So you agree that completing Task management request
> in the same context as the eh threads is the way to go, right?
I think so -- process context.
> Because that is what this LLD is doing. Have you had
> a chance to look at our code?
No, I haven't looked -- I'm swamped.
> That previous patch submitted by Jeff(I guess) was removing code which
> enabled interrupts. My patch was restoring it back to previous
> working code which doesn't hang the driver.
In LLDD yes (since they were never disabled when eh_abort() was called).
In SCSI Core -- the opposite: removing code which disabled ints and then
calling eh_abort() of the LLDD.
> The reason that the previous patch hangs the driver is we are issuing
> the task managment request using interrupt mode, not polling.
> Meaning it requires interrupts to be enabled.
They are! The abort handler is called with ints enabled.
See the core of Jeff's patch here:
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -528,10 +528,8 @@ static int scsi_send_eh_cmnd(struct scsi
* abort a timed out command or not. not sure how
* we should treat them differently anyways.
*/
- spin_lock_irqsave(shost->host_lock, flags);
if (shost->hostt->eh_abort_handler)
shost->hostt->eh_abort_handler(scmd);
- spin_unlock_irqrestore(shost->host_lock, flags);
scmd->request->rq_status = RQ_SCSI_DONE;
scmd->owner = SCSI_OWNER_ERROR_HANDLER;
@@ -737,11 +735,8 @@ static int scsi_eh_get_sense(struct list
**/
static int scsi_try_to_abort_cmd(struct scsi_cmnd *scmd)
{
- unsigned long flags;
- int rtn = FAILED;
-
if (!scmd->device->host->hostt->eh_abort_handler)
- return rtn;
+ return FAILED;
/*
* scsi_done was called just after the command timed out and before
@@ -752,11 +747,7 @@ static int scsi_try_to_abort_cmd(struct
scmd->owner = SCSI_OWNER_LOWLEVEL;
- spin_lock_irqsave(scmd->device->host->host_lock, flags);
- rtn = scmd->device->host->hostt->eh_abort_handler(scmd);
- spin_unlock_irqrestore(scmd->device->host->host_lock, flags);
-
- return rtn;
+ return scmd->device->host->hostt->eh_abort_handler(scmd);
}
/**
So the LLDD is free to sleep in eh_abort(). If it needs to do
something with ints disabled, it should disable them itself.
Luben
-
: 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
