From: Elias Oltmanns <eo@nebensachen.de>
To: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>,
linux-ide@vger.kernel.org, linux-kernel@vger.kernel.org,
Randy Dunlap <randy.dunlap@oracle.com>
Subject: [PATCH 1/4 v2] IDE: Fix HDIO_DRIVE_RESET handling
Date: Wed, 25 Jun 2008 13:27:32 +0200 [thread overview]
Message-ID: <87ej6ln49n.fsf_-_@denkblock.local> (raw)
In-Reply-To: <87k5gdn4fq.fsf@denkblock.local> (Elias Oltmanns's message of "Wed, 25 Jun 2008 13:23:53 +0200")
Currently, the code path executing an HDIO_DRIVE_RESET ioctl is broken
in various ways. Most importantly, it is treated as an out of band
request in an illegal way which may very likely lead to system lock ups.
Use the drive's request queue to avoid this problem (and fix a locking
issue for free along the way).
Signed-off-by: Elias Oltmanns <eo@nebensachen.de>
---
drivers/ide/ide-io.c | 23 ++++++++++++++++++++++-
drivers/ide/ide-iops.c | 18 +++++++++++++-----
drivers/ide/ide.c | 41 +++++++++++++++--------------------------
include/linux/ide.h | 6 ++++++
4 files changed, 56 insertions(+), 32 deletions(-)
diff --git a/drivers/ide/ide-io.c b/drivers/ide/ide-io.c
index 2805774..2b33c12 100644
--- a/drivers/ide/ide-io.c
+++ b/drivers/ide/ide-io.c
@@ -766,6 +766,18 @@ static ide_startstop_t execute_drive_cmd (ide_drive_t *drive,
return ide_stopped;
}
+static ide_startstop_t ide_special_rq(ide_drive_t *drive, struct request *rq)
+{
+ switch (rq->cmd[0]) {
+ case REQ_DRIVE_RESET:
+ return ide_do_reset(drive);
+ default:
+ blk_dump_rq_flags(rq, "ide_special_rq - bad request");
+ ide_end_request(drive, 0, 0);
+ return ide_stopped;
+ }
+}
+
static void ide_check_pm_state(ide_drive_t *drive, struct request *rq)
{
struct request_pm_state *pm = rq->data;
@@ -869,7 +881,16 @@ static ide_startstop_t start_request (ide_drive_t *drive, struct request *rq)
pm->pm_step == ide_pm_state_completed)
ide_complete_pm_request(drive, rq);
return startstop;
- }
+ } else if (!rq->rq_disk && blk_special_request(rq))
+ /*
+ * TODO: Once all ULDs have been modified to
+ * check for specific op codes rather than
+ * blindly accepting any special request, the
+ * check for ->rq_disk above may be replaced
+ * by a more suitable mechanism or even
+ * dropped entirely.
+ */
+ return ide_special_rq(drive, rq);
drv = *(ide_driver_t **)rq->rq_disk->private_data;
return drv->do_request(drive, rq, block);
diff --git a/drivers/ide/ide-iops.c b/drivers/ide/ide-iops.c
index 66e39e5..80e782b 100644
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -905,6 +905,14 @@ void ide_execute_pkt_cmd(ide_drive_t *drive)
}
EXPORT_SYMBOL_GPL(ide_execute_pkt_cmd);
+static inline void ide_complete_drive_reset(ide_drive_t *drive)
+{
+ struct request *rq = drive->hwif->hwgroup->rq;
+
+ if (rq && blk_special_request(rq) && rq->cmd[0] == REQ_DRIVE_RESET)
+ ide_end_request(drive, 1, 0);
+}
+
/* needed below */
static ide_startstop_t do_reset1 (ide_drive_t *, int);
@@ -940,7 +948,7 @@ static ide_startstop_t atapi_reset_pollfunc (ide_drive_t *drive)
}
/* done polling */
hwgroup->polling = 0;
- hwgroup->resetting = 0;
+ ide_complete_drive_reset(drive);
return ide_stopped;
}
@@ -961,7 +969,7 @@ static ide_startstop_t reset_pollfunc (ide_drive_t *drive)
if (port_ops->reset_poll(drive)) {
printk(KERN_ERR "%s: host reset_poll failure for %s.\n",
hwif->name, drive->name);
- return ide_stopped;
+ goto out;
}
}
@@ -1004,7 +1012,8 @@ static ide_startstop_t reset_pollfunc (ide_drive_t *drive)
}
}
hwgroup->polling = 0; /* done polling */
- hwgroup->resetting = 0; /* done reset attempt */
+out:
+ ide_complete_drive_reset(drive);
return ide_stopped;
}
@@ -1090,7 +1099,6 @@ static ide_startstop_t do_reset1 (ide_drive_t *drive, int do_not_try_atapi)
/* For an ATAPI device, first try an ATAPI SRST. */
if (drive->media != ide_disk && !do_not_try_atapi) {
- hwgroup->resetting = 1;
pre_reset(drive);
SELECT_DRIVE(drive);
udelay (20);
@@ -1112,10 +1120,10 @@ static ide_startstop_t do_reset1 (ide_drive_t *drive, int do_not_try_atapi)
if (io_ports->ctl_addr == 0) {
spin_unlock_irqrestore(&ide_lock, flags);
+ ide_complete_drive_reset(drive);
return ide_stopped;
}
- hwgroup->resetting = 1;
/*
* Note that we also set nIEN while resetting the device,
* to mask unwanted interrupts from the interface during the reset.
diff --git a/drivers/ide/ide.c b/drivers/ide/ide.c
index b0739c9..dbedb02 100644
--- a/drivers/ide/ide.c
+++ b/drivers/ide/ide.c
@@ -529,6 +529,19 @@ static int generic_ide_resume(struct device *dev)
return err;
}
+static void generic_drive_reset(ide_drive_t *drive)
+{
+ struct request *rq;
+
+ rq = blk_get_request(drive->queue, READ, __GFP_WAIT);
+ rq->cmd_type = REQ_TYPE_SPECIAL;
+ rq->cmd_len = 1;
+ rq->cmd[0] = REQ_DRIVE_RESET;
+ rq->cmd_flags |= REQ_SOFTBARRIER;
+ blk_execute_rq(drive->queue, NULL, rq, 1);
+ blk_put_request(rq);
+}
+
int generic_ide_ioctl(ide_drive_t *drive, struct file *file, struct block_device *bdev,
unsigned int cmd, unsigned long arg)
{
@@ -603,33 +616,9 @@ int generic_ide_ioctl(ide_drive_t *drive, struct file *file, struct block_device
if (!capable(CAP_SYS_ADMIN))
return -EACCES;
- /*
- * Abort the current command on the
- * group if there is one, taking
- * care not to allow anything else
- * to be queued and to die on the
- * spot if we miss one somehow
- */
-
- spin_lock_irqsave(&ide_lock, flags);
-
- if (HWGROUP(drive)->resetting) {
- spin_unlock_irqrestore(&ide_lock, flags);
- return -EBUSY;
- }
-
- ide_abort(drive, "drive reset");
-
- BUG_ON(HWGROUP(drive)->handler);
-
- /* Ensure nothing gets queued after we
- drop the lock. Reset will clear the busy */
-
- HWGROUP(drive)->busy = 1;
- spin_unlock_irqrestore(&ide_lock, flags);
- (void) ide_do_reset(drive);
-
+ generic_drive_reset(drive);
return 0;
+
case HDIO_GET_BUSSTATE:
if (!capable(CAP_SYS_ADMIN))
return -EACCES;
diff --git a/include/linux/ide.h b/include/linux/ide.h
index 5ddae9b..8fbe838 100644
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -139,6 +139,12 @@ struct ide_io_ports {
#define WAIT_MIN_SLEEP (2*HZ/100) /* 20msec - minimum sleep time */
/*
+ * Op codes for special requests to be handled by ide_special_rq().
+ * Values should be in the range of 0x20 to 0x3f.
+ */
+#define REQ_DRIVE_RESET 0x20
+
+/*
* Check for an interrupt and acknowledge the interrupt status
*/
struct hwif_s;
next prev parent reply other threads:[~2008-06-25 11:27 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-06-18 23:35 [PATCH] IDE: Fix HDIO_DRIVE_RESET handling Elias Oltmanns
2008-06-18 23:41 ` Elias Oltmanns
2008-06-19 20:47 ` Bartlomiej Zolnierkiewicz
2008-06-22 23:23 ` Elias Oltmanns
2008-06-22 23:28 ` Elias Oltmanns
2008-06-23 7:47 ` Elias Oltmanns
2008-06-23 22:47 ` Bartlomiej Zolnierkiewicz
2008-06-23 9:16 ` Alan Cox
2008-06-24 7:02 ` Elias Oltmanns
2008-06-24 9:10 ` Alan Cox
2008-06-23 22:41 ` Bartlomiej Zolnierkiewicz
2008-06-24 7:12 ` Elias Oltmanns
2008-06-22 23:32 ` [PATCH 2/4] IDE: Remove unused code Elias Oltmanns
2008-06-22 23:35 ` [PATCH 3/4] Update documentation of HDIO_DRIVE_RESET ioctl Elias Oltmanns
2008-06-22 23:38 ` [PATCH 4/4] IDE: Report errors during drive reset back to user space Elias Oltmanns
2008-06-23 9:18 ` [PATCH] IDE: Fix HDIO_DRIVE_RESET handling Alan Cox
2008-06-23 22:41 ` Bartlomiej Zolnierkiewicz
2008-06-24 7:23 ` Elias Oltmanns
2008-06-24 11:06 ` Bartlomiej Zolnierkiewicz
2008-06-24 12:32 ` Alan Cox
2008-06-24 13:21 ` Bartlomiej Zolnierkiewicz
2008-06-24 13:35 ` Alan Cox
2008-06-24 14:19 ` Bartlomiej Zolnierkiewicz
2008-06-24 14:33 ` Bartlomiej Zolnierkiewicz
2008-06-25 11:23 ` Elias Oltmanns
2008-06-25 11:27 ` Elias Oltmanns [this message]
2008-06-25 11:28 ` [PATCH 2/4 v2] IDE: Remove unused code Elias Oltmanns
2008-06-25 11:29 ` [PATCH 3/4 v2] Update documentation of HDIO_DRIVE_RESET ioctl Elias Oltmanns
2008-06-25 11:30 ` [PATCH 4/4 v2] IDE: Report errors during drive reset back to user space Elias Oltmanns
2008-06-25 20:24 ` [PATCH] IDE: Fix HDIO_DRIVE_RESET handling Bartlomiej Zolnierkiewicz
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87ej6ln49n.fsf_-_@denkblock.local \
--to=eo@nebensachen.de \
--cc=alan@lxorguk.ukuu.org.uk \
--cc=bzolnier@gmail.com \
--cc=linux-ide@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=randy.dunlap@oracle.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).