* Ingo Molnar <[email protected]> wrote:
> ah. That's a real deadlock scenario. Does the patch below fix it? If
> yes then i think this is a candidate for 2.6.17 merging too.
actually, the replacement patch below is better i think - it moves the
ide_lock taking to outside the printing section. That should still be OK
as we dont call other functions from within the section, and it should
also result in slightly more robust printing, as the whole printing code
will be atomic under ide_lock.
Ingo
---------
Subject: [patch, -rc5-mm3] fix IDE deadlock in error reporting code
From: Ingo Molnar <[email protected]>
Michal Piotrowski reported the following validator assert:
hdd: set_drive_speed_status: status=0x51 { DriveReady SeekComplete Error }
hdd: set_drive_speed_status: error=0xb4 { AbortedCommand LastFailedSense=0x0b }
============================
[ BUG: illegal lock usage! ]
----------------------------
illegal {in-hardirq-W} -> {hardirq-on-W} usage.
hdparm/1821 [HC0[0]:SC0[0]:HE1:SE1] takes:
(ide_lock){++..}, at: [<c0268388>] ide_dump_opcode+0x13/0x9b
[...]
stack backtrace:
[<c0104513>] show_trace+0x1b/0x20
[<c01045f1>] dump_stack+0x1f/0x24
[<c013976c>] print_usage_bug+0x1a5/0x1b1
[<c0139e90>] mark_lock+0x2ca/0x4f7
[<c013aa96>] __lockdep_acquire+0x47e/0xaa4
[<c013b536>] lockdep_acquire+0x67/0x7f
[<c030552d>] _spin_lock+0x24/0x32
[<c0268388>] ide_dump_opcode+0x13/0x9b
[<c02688b6>] ide_dump_status+0x4a6/0x4cc
[<c0267ae6>] ide_config_drive_speed+0x32a/0x33a
[<c0262dc5>] piix_tune_chipset+0x2ed/0x2f8
[<c0262e31>] piix_config_drive_xfer_rate+0x61/0xb5
[<c0263a82>] set_using_dma+0x2f/0x60
[<c0263bee>] ide_write_setting+0x4a/0xc3
[<c02647ca>] generic_ide_ioctl+0x8a/0x47f
[<f886003a>] idecd_ioctl+0xfd/0x133 [ide_cd]
[<c01f1fff>] blkdev_driver_ioctl+0x4b/0x5f
[<c01f2783>] blkdev_ioctl+0x770/0x7bd
[<c017dc0d>] block_ioctl+0x1f/0x21
[<c0189353>] do_ioctl+0x27/0x6e
[<c0189604>] vfs_ioctl+0x26a/0x280
[<c0189667>] sys_ioctl+0x4d/0x7e
[<c0305ed2>] sysenter_past_esp+0x63/0xa1
in ide_dump_opcode() takes the ide_lock in an irq-unsafe manner,
i.e. this function expects to be called with irqs disabled. But
ide_dump_ata[pi]_status() doesnt do that - it enables interrupts
specifically. That is a no-no - what guarantees that another IDE
port couldnt generate an IDE interrupt while we are dumping this
error? The fix is to turn the irq-enabling in these functions into
irq-disabling.
Signed-off-by: Ingo Molnar <[email protected]>
---
drivers/ide/ide-lib.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
Index: linux/drivers/ide/ide-lib.c
===================================================================
--- linux.orig/drivers/ide/ide-lib.c
+++ linux/drivers/ide/ide-lib.c
@@ -471,11 +471,9 @@ static void ide_dump_opcode(ide_drive_t
u8 opcode = 0;
int found = 0;
- spin_lock(&ide_lock);
rq = NULL;
if (HWGROUP(drive))
rq = HWGROUP(drive)->rq;
- spin_unlock(&ide_lock);
if (!rq)
return;
if (rq->flags & (REQ_DRIVE_CMD | REQ_DRIVE_TASK)) {
@@ -506,7 +504,7 @@ static u8 ide_dump_ata_status(ide_drive_
unsigned long flags;
u8 err = 0;
- local_irq_set(flags);
+ spin_lock_irqsave(&ide_lock, flags);
printk("%s: %s: status=0x%02x { ", drive->name, msg, stat);
if (stat & BUSY_STAT)
printk("Busy ");
@@ -566,7 +564,8 @@ static u8 ide_dump_ata_status(ide_drive_
printk("\n");
}
ide_dump_opcode(drive);
- local_irq_restore(flags);
+ spin_unlock_irqrestore(&ide_lock, flags);
+
return err;
}
@@ -588,7 +587,9 @@ static u8 ide_dump_atapi_status(ide_driv
status.all = stat;
error.all = 0;
- local_irq_set(flags);
+
+ spin_lock_irqsave(&ide_lock, flags);
+
printk("%s: %s: status=0x%02x { ", drive->name, msg, stat);
if (status.b.bsy)
printk("Busy ");
@@ -614,7 +615,8 @@ static u8 ide_dump_atapi_status(ide_driv
printk("}\n");
}
ide_dump_opcode(drive);
- local_irq_restore(flags);
+ spin_unlock_irqrestore(&ide_lock, flags);
+
return error.all;
}
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
