Re: [PATCH] Re: sleeping in scsi EH

Brian King
2005-05-27T14:11:05Z

[-- Attachment #1: Type: text/plain, Size: 879 bytes --]

Jeff Garzik wrote:
> Christoph Hellwig wrote:
>
>>On Fri, May 27, 2005 at 04:37:29AM -0400, Jeff Garzik wrote:
>>
>>
>>>Christoph Hellwig wrote:
>>>
>>>
>>>>You can sleep in them. You must however release the host lock and enable
>>>>irqs first and reverse that before returning. The error handlers don't
>>>>need the host lock, but we're stuck with the unfortunate calling convention
>>>>for now.
>>>
>>>Why are we stuck with this calling convention, when everyone who cares
>>>circumvents it?
>>
>>
>>Because no one found the time to do a full transition yet. If you want to
>>update all scsi drivers feel free. One patch per method please.
>
>

Jeff,

Please add the following patch to your eh_abort locking patch for the ipr driver.
It fixes a race condition your patch would introduce. Thanks.

Brian

--
Brian King
eServer Storage I/O
IBM Linux Technology Center

[-- Attachment #2: ipr_abort_locking.patch --]
[-- Type: text/plain, Size: 1671 bytes --]

Signed-off-by: Brian King <brking@us.ibm.com>
---

linux-2.6.12-rc5-bjking1/drivers/scsi/ipr.c | 17 +++++------------
1 files changed, 5 insertions(+), 12 deletions(-)

diff -puN drivers/scsi/ipr.c~ipr_abort_locking drivers/scsi/ipr.c
--- linux-2.6.12-rc5/drivers/scsi/ipr.c~ipr_abort_locking 2005-05-27 08:57:49.000000000 -0500
+++ linux-2.6.12-rc5-bjking1/drivers/scsi/ipr.c 2005-05-27 09:00:48.000000000 -0500
@@ -3069,6 +3069,11 @@ static int ipr_cancel_op(struct scsi_cmn
ioa_cfg = (struct ipr_ioa_cfg *)scsi_cmd->device->host->hostdata;
res = scsi_cmd->device->hostdata;

+ /* If we are currently going through reset/reload, return failed. This will force the
+ mid-layer to call ipr_eh_host_reset, which will then go to sleep and wait for the
+ reset to complete */
+ if (ioa_cfg->in_reset_reload || ioa_cfg->ioa_is_dead)
+ return FAILED;
if (!res || (!ipr_is_gscsi(res) && !ipr_is_vset_device(res)))
return FAILED;

@@ -3119,22 +3124,10 @@ static int ipr_cancel_op(struct scsi_cmn
**/
static int ipr_eh_abort(struct scsi_cmnd * scsi_cmd)
{
- struct ipr_ioa_cfg *ioa_cfg;
unsigned long flags;
int rc;

ENTER;
- ioa_cfg = (struct ipr_ioa_cfg *) scsi_cmd->device->host->hostdata;
-
- /* If we are currently going through reset/reload, return failed. This will force the
- mid-layer to call ipr_eh_host_reset, which will then go to sleep and wait for the
- reset to complete */
- if (ioa_cfg->in_reset_reload)
- return FAILED;
- if (ioa_cfg->ioa_is_dead)
- return FAILED;
- if (!scsi_cmd->device->hostdata)
- return FAILED;

spin_lock_irqsave(scsi_cmd->device->host->host_lock, flags);
rc = ipr_cancel_op(scsi_cmd);
_