Hi Bart,
currently, the code path executing an HDIO_DRIVE_RESET ioctl is broken
in various ways. Firstly, ide_abort() is called with the ide_lock held
and may call ide_end_request() further down the line which will try to
grab the same lock again. More importantly though, the whole concept of
aborting an inflight request is flawed -- at least as far as ide_abort()
is concerned.
The patch below tries to address these issues by handling the
HDIO_DRIVE_RESET ioctl in-band. I wonder whether this approach is
acceptable and may be extended to other applications like a modified
version of the disk shock protection patches. Also, I abstained from
using op codes 0x0 to 0x1f since at least part of this range is used by
ULDs like ide-floppy, ide-tape, etc. On the other hand, there really
should be no conflict because those ULDs will always set ->rq_disk thus
preventing ide_special_rq() from snatching away what should be passed on
to ->do_request(). So, what's you're advice on this one? Finally,
shouldn't op codes like REQ_DRIVE_RESET really be declared in a private
header file in drivers/ide/?
Signed-off-by: Elias Oltmanns <eo@xxxxxxxxxxxxxx>
---
drivers/ide/ide-io.c | 16 +++++++++++++++-
drivers/ide/ide.c | 40 ++++++++++++++--------------------------
include/linux/ide.h | 6 ++++++
3 files changed, 35 insertions(+), 27 deletions(-)
diff --git a/drivers/ide/ide-io.c b/drivers/ide/ide-io.c
index 6965253..b3a37b1 100644
--- a/drivers/ide/ide-io.c
+++ b/drivers/ide/ide-io.c
@@ -788,6 +788,19 @@ 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:
+ ide_end_request(drive, 1, 0);
+ 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;
@@ -891,7 +904,8 @@ 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))
+ 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.c b/drivers/ide/ide.c
index c758dcb..4f9c796 100644
--- a/drivers/ide/ide.c
+++ b/drivers/ide/ide.c
@@ -623,6 +623,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, 0, __GFP_WAIT);
+ /* Should we call ide_init_drive_cmd() here? */
+
+ rq->cmd[0] = REQ_DRIVE_RESET;
+ rq->cmd_type = REQ_TYPE_SPECIAL;
+ rq->cmd_flags |= REQ_SOFTBARRIER;
+ ide_do_drive_cmd(drive, rq, ide_end);
+}
+
int generic_ide_ioctl(ide_drive_t *drive, struct file *file, struct block_device *bdev,
unsigned int cmd, unsigned long arg)
{
@@ -697,32 +710,7 @@ 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))
diff --git a/include/linux/ide.h b/include/linux/ide.h
index 9918772..4c3e802 100644
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -842,6 +842,12 @@ int ide_wait_stat(ide_startstop_t *, ide_drive_t *, u8, u8, unsigned long);
extern ide_startstop_t ide_do_reset (ide_drive_t *);
+/*
+ * 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
+
extern void ide_init_drive_cmd (struct request *rq);
/*
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
