From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jeff Garzik Subject: [PATCH] fix ide-scsi EH locking Date: Fri, 27 May 2005 17:19:45 -0400 Message-ID: <42978EF1.5000703@pobox.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------090503010009000008000405" Return-path: Received: from mail.dvmed.net ([216.237.124.58]:18401 "EHLO mail.dvmed.net") by vger.kernel.org with ESMTP id S262601AbVE0VTz (ORCPT ); Fri, 27 May 2005 17:19:55 -0400 Sender: linux-ide-owner@vger.kernel.org List-Id: linux-ide@vger.kernel.org To: Linux Kernel , SCSI Mailing List , "linux-ide@vger.kernel.org" Cc: James Bottomley , Bartlomiej Zolnierkiewicz , Alan Cox , Dave Jones , Andrew Morton This is a multi-part message in MIME format. --------------090503010009000008000405 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit SCSI's error handling hooks are called inside spin_lock_irqsave(host_lock, flags) ... spin_unlock_irqrestore(host_lock, flags) ide-scsi's SCSI EH functions, which operate inside the above lock, wrap several operations inside spin_lock_irq(ide_lock) ... spin_unlock_irq(ide_lock) Use of the unconditional spin_lock_irq(), as opposed to spin_lock_irqsave(), corrupts the irq context. Attached patch (against latest git) updates ide-scsi to simply use the spin_lock() variant, since we know we are already inside of spin_lock_irqsave(). Patch untested, but at least the code isn't obviously wrong now... --------------090503010009000008000405 Content-Type: text/plain; name="patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="patch" diff --git a/drivers/scsi/ide-scsi.c b/drivers/scsi/ide-scsi.c --- a/drivers/scsi/ide-scsi.c +++ b/drivers/scsi/ide-scsi.c @@ -46,6 +46,7 @@ #include #include #include +#include #include #include @@ -959,7 +960,8 @@ static int idescsi_eh_abort (struct scsi if (test_bit(IDESCSI_LOG_CMD, &scsi->log)) printk (KERN_WARNING "ide-scsi: drive did%s become ready\n", busy?" not":""); - spin_lock_irq(&ide_lock); + /* remember, we are inside spin_lock_irq() already */ + spin_lock(&ide_lock); /* If there is no pc running we're done (our interrupt took care of it) */ if (!scsi->pc) { @@ -985,7 +987,7 @@ static int idescsi_eh_abort (struct scsi } ide_unlock: - spin_unlock_irq(&ide_lock); + spin_unlock(&ide_lock); no_drive: if (test_bit(IDESCSI_LOG_CMD, &scsi->log)) printk (KERN_WARNING "ide-scsi: abort returns %s\n", ret == SUCCESS?"success":"failed"); @@ -1012,7 +1014,8 @@ static int idescsi_eh_reset (struct scsi return FAILED; } - spin_lock_irq(&ide_lock); + /* remember, we are inside spin_lock_irq() already */ + spin_lock(&ide_lock); if (!scsi->pc || (req = scsi->pc->rq) != HWGROUP(drive)->rq || !HWGROUP(drive)->handler) { printk (KERN_WARNING "ide-scsi: No active request in idescsi_eh_reset\n"); @@ -1038,16 +1041,15 @@ static int idescsi_eh_reset (struct scsi HWGROUP(drive)->rq = NULL; HWGROUP(drive)->handler = NULL; HWGROUP(drive)->busy = 1; /* will set this to zero when ide reset finished */ - spin_unlock_irq(&ide_lock); + spin_unlock(&ide_lock); ide_do_reset(drive); /* ide_do_reset starts a polling handler which restarts itself every 50ms until the reset finishes */ do { - set_current_state(TASK_UNINTERRUPTIBLE); spin_unlock_irq(cmd->device->host->host_lock); - schedule_timeout(HZ/20); + msleep(50); spin_lock_irq(cmd->device->host->host_lock); } while ( HWGROUP(drive)->handler ); --------------090503010009000008000405--