2.5.65-ac2 -- hda/ide trouble on ICH4

hda: host protected area => 1
hda: 160836480 sectors (82348 MB) w/1863KiB Cache, CHS=159560/16/63, UDMA(100)
hda: [PTBL] [10011/255/63] hda1 hda2 hda3 hda4 < hda5 hda6 hda7 hda8 hda9 >

and *deadlock*...

in plain 2.5.65 I was seeing strange error messages like:

Mar 19 20:29:55 mondschein kernel: hda: dma_timer_expiry: dma status == 0x24
Mar 19 20:29:55 mondschein kernel: hda: lost interrupt
Mar 19 20:29:55 mondschein kernel: hda: dma_intr: bad DMA status (dma_stat=30)
Mar 19 20:29:55 mondschein kernel: hda: dma_intr: status=0x52 { DriveReady SeekComplete Index }
Mar 19 20:29:55 mondschein kernel:

Quoted 5 lines · Dominik Brodowski

hda: host protected area => 1
hda: 160836480 sectors (82348 MB) w/1863KiB Cache, CHS=159560/16/63, UDMA(100)
hda: [PTBL] [10011/255/63] hda1 hda2 hda3 hda4 < hda5 hda6 hda7 hda8 hda9 >

and *deadlock*...

Where is the lock, what does the NMI oopser show ?

Quoted 7 lines

in plain 2.5.65 I was seeing strange error messages like:

Mar 19 20:29:55 mondschein kernel: hda: dma_timer_expiry: dma status == 0x24
Mar 19 20:29:55 mondschein kernel: hda: lost interrupt
Mar 19 20:29:55 mondschein kernel: hda: dma_intr: bad DMA status (dma_stat=30)
Mar 19 20:29:55 mondschein kernel: hda: dma_intr: status=0x52 { DriveReady SeekComplete Index }
Mar 19 20:29:55 mondschein kernel:

I've seen 3 or 4 reports of this, none of them duplicatable with the same IDE code on 2.4 so far. Which is odd but I don't yet understand what is going on.

Quoted 1 line

Where is the lock, what does the NMI oopser show ?

The lock is directly "below" that line -- and the NMI oopser isn't triggered, AFAICT

Quoted 4 lines · Dominik Brodowski

Quoted 1 line

Where is the lock, what does the NMI oopser show ?

The lock is directly "below" that line -- and the NMI oopser isn't triggered, AFAICT

Anything useful off right-alt scroll-lock etc ?

Quoted 4 lines · Dominik Brodowski

Quoted 1 line

Where is the lock, what does the NMI oopser show ?

The lock is directly "below" that line -- and the NMI oopser isn't triggered, AFAICT

Anything useful off right-alt scroll-lock etc ?

	while (!list_empty(&list)) {
		ide_drive_t *drive = list_entry(list.next, ide_drive_t,
list);
		list_del_init(&drive->list);
		if (drive->present)
			ata_attach(drive);

Quoted 6 lines · Dominik Brodowski

	while (!list_empty(&list)) {
		ide_drive_t *drive = list_entry(list.next, ide_drive_t,
list);
		list_del_init(&drive->list);
		if (drive->present)
			ata_attach(drive);

Can you boot and printk the drive name out each iteration see if the list is hosed somewhere.

printk("%4s\n", drive->name) prints out "hdd" all the time.

hda is an ATA disk drive
hdb is empty
hdc is an ATAPI CD/DVD-ROM drive
hdd is an ATAPI CD-ROM CD-R/RW drive

Quoted 6 lines

printk("%4s\n", drive->name) prints out "hdd" all the time.

hda is an ATA disk drive
hdb is empty
hdc is an ATAPI CD/DVD-ROM drive
hdd is an ATAPI CD-ROM CD-R/RW drive

This gets weirder by the minute, and I still can't get it to happen here annoyingly.

The list thats breaking is a private list. We delete the drive from that list, if its present (it may be an empty bay) we then use ata_attach to add it to a device list (or back to ata_unused).

I find it hard to believe something like this is a compiler bug, but right now I don't see how stuff is reappearing on the list.

Quoted 14 lines · Alan Cox

Quoted 6 lines

printk("%4s\n", drive->name) prints out "hdd" all the time.

hda is an ATA disk drive
hdb is empty
hdc is an ATAPI CD/DVD-ROM drive
hdd is an ATAPI CD-ROM CD-R/RW drive

This gets weirder by the minute, and I still can't get it to happen here annoyingly.

The list thats breaking is a private list. We delete the drive from that list, if its present (it may be an empty bay) we then use ata_attach to add it to a device list (or back to ata_unused).

Quoted 2 lines

I find it hard to believe something like this is a compiler bug, but right now I don't see how stuff is reappearing on the list.

Just got it to boot :) -- the while(!list_empty...) { list_entry ... looks suspicious. Might be better to use list_for_each_safe() which is designed exactly for this purpouse. I'm currently recompiling 2.5.65-bk-current-as-of-yesterday with the attached patch. Let's see whether it works with this kernel, too...

Quoted 5 lines · Dominik Brodowski

Just got it to boot :) -- the while(!list_empty...) { list_entry ... looks suspicious. Might be better to use list_for_each_safe() which is designed exactly for this purpouse. I'm currently recompiling 2.5.65-bk-current-as-of-yesterday with the attached patch. Let's see whether it works with this kernel, too...

Yes, it also works with 2.5.65-bkX.

Hi !

I tried linux-2.5.65-ac3 with ide-disk and ide-scsi drivers both built in the Kernel.
Two ide-disks attached to ide0, two CDROMS attached to ide1.
Im using the sis5513-PCI-IDE-Driver, but configuring it out makes no difference.

Yes, it also works with 2.5.65-bkX.

i can't reproduce the hang but it seems that drives without driver can get both in ata_unused and idedefault_driver.drives and lists go nuts. It kills ata_unused and uses idedefault_driver.drives only, boots fine here. I'd guess you have ide-cd as module, and the two drives handled by it couse the trouble - first joins the lists second couses the loop.

Quoted 6 lines · Alexander Atanasov

i can't reproduce the hang but it seems that drives without driver can get both in ata_unused and idedefault_driver.drives and lists go nuts. It kills ata_unused and uses idedefault_driver.drives only, boots fine here. I'd guess you have ide-cd as module, and the two drives handled by it couse the trouble - first joins the lists second couses the loop.

We need to know the difference between the two really so I would much rather ensure we don't end up on both lists at once (which is a bug) than lose a list

I don't understand, what's the difference and how the list is lost?
ata_unused used to hold all drives that were not claimed by any driver, now idedefault_driver claims all that drives, all drives go in the .list

The bug is there, and waiting to explode, keeping both lists would mean to add one more list head in ide_drive_t, is that the fix you want?

Quoted 5 lines · Alexander Atanasov

I don't understand, what's the difference and how the list is lost?
ata_unused used to hold all drives that were not claimed by any driver, now idedefault_driver claims all that drives, all drives go in the .list

ata_unused -> unattached device slots, new hotplug discoveries

idedefault_driver -> attached/known devices with no driver other list -> driven by that driver

Quoted 3 lines

The bug is there, and waiting to explode, keeping both lists would mean to add one more list head in ide_drive_t, is that the fix you want?

I don't see where stuff is ending up on both lists yet. I've not had time to look hard at it though

Hello, Alan!

Quoted 8 lines · On 24 Mar 2003 17:40:08 +0000 Alan Cox <[email protected]> wrote:

Quoted 5 lines · Alexander Atanasov

I don't understand, what's the difference and how the list is lost?
ata_unused used to hold all drives that were not claimed by any driver, now idedefault_driver claims all that drives, all drives go in the .list

ata_unused -> unattached device slots, new hotplug discoveries

	Ok. 

Quoted 10 lines

idedefault_driver -> attached/known devices with no driver other list -> driven by that driver

Quoted 3 lines

The bug is there, and waiting to explode, keeping both lists would mean to add one more list head in ide_drive_t, is that the fix you want?

I don't see where stuff is ending up on both lists yet. I've not had time to look hard at it though

	It happens this way:
	ide_register_driver -> ata_attach -> idedefault_driver.attach -> ide_register_subdriver -> list_add(&driver->list, &driver->drives) ->

return to ata_attach -> list_add_tail(&drive->list, &ata_unused);

--
have fun,
alex
-
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/

This is one thing all of you don't get about hotplug.

You are not allowed on PATA, only SATA.

The BIOS and setup on the HBA's need a kick to come alive. There are basic hooks that do not permit post boot hotplug in PATA.

Hello, Alan!

Quoted 8 lines · On 24 Mar 2003 17:40:08 +0000 Alan Cox <[email protected]> wrote:

Quoted 5 lines · Alexander Atanasov

I don't understand, what's the difference and how the list is lost?
ata_unused used to hold all drives that were not claimed by any driver, now idedefault_driver claims all that drives, all drives go in the .list

ata_unused -> unattached device slots, new hotplug discoveries

	Ok.

Quoted 7 lines

idedefault_driver -> attached/known devices with no driver other list -> driven by that driver

Quoted 3 lines

The bug is there, and waiting to explode, keeping both lists would mean to add one more list head in ide_drive_t, is that the fix you want?

Quoted 3 lines

I don't see where stuff is ending up on both lists yet. I've not had time to look hard at it though

	It happens this way:
	ide_register_driver -> ata_attach -> idedefault_driver.attach -> ide_register_subdriver -> list_add(&driver->list, &driver->drives) ->

return to ata_attach -> list_add_tail(&drive->list, &ata_unused);

--
have fun,
alex

Quoted 23 lines · Alexander Atanasov

Hello, Alan!

Quoted 8 lines · On 24 Mar 2003 17:40:08 +0000 Alan Cox <[email protected]> wrote:

Quoted 5 lines · Alexander Atanasov

I don't understand, what's the difference and how the list is lost?
ata_unused used to hold all drives that were not claimed by any driver, now idedefault_driver claims all that drives, all drives go in the .list

ata_unused -> unattached device slots, new hotplug discoveries

	Ok.

Quoted 7 lines

idedefault_driver -> attached/known devices with no driver other list -> driven by that driver

Quoted 3 lines

The bug is there, and waiting to explode, keeping both lists would mean to add one more list head in ide_drive_t, is that the fix you want?

Quoted 7 lines

Quoted 3 lines

I don't see where stuff is ending up on both lists yet. I've not had time to look hard at it though

	It happens this way:
	ide_register_driver -> ata_attach -> idedefault_driver.attach -> ide_register_subdriver -> list_add(&driver->list, &driver->drives) ->

return to ata_attach -> list_add_tail(&drive->list, &ata_unused);

Exactly.

Alan, if we want to keep ata_unused, we should remove list_add_tail(%drive->list, &ata_unused) from ata_attach() and perhaps use (after fixing it to handle idedefault_driver) ide_replace_subdriver() for driver switching for drives owned by idedefault_driver.

BTW in ide_register_driver() we don't use ide_drives lock to protect

    drive->list changes, why?

--
bzolnier

Quoted 3 lines

--
have fun,
alex

Hi Alan,
I found just changing ide_do_reset() to wait till completion can
handle the handler race. can this be enough?

Quoted 3 lines · rain.wang

Hi Alan,
I found just changing ide_do_reset() to wait till completion can
handle the handler race. can this be enough?

This is buggy for a number of reasons. Firstly, how do you make sure that someone else doesn't race with your hwif_data manipulation? This looks very suspect. By far the worst problem is that you are assuming that ide_do_reset() can sleep, when in fact it cannot (just follow the various paths into ide_do_request()). You even grab the ide_lock _and_ disable interrupts yourself prior calling wait_for_completion(), this is incredibly broken.

--
Jens Axboe

Quoted 3 lines · Alan Cox

I've seen 3 or 4 reports of this, none of them duplicatable with the same IDE code on 2.4 so far. Which is odd but I don't yet understand what is going on.

/me neither, unfortunately :-(

Previously callers called it with masked_irq=0 and disabling/enabling hwif->irq code wasn't executed, now ide_do_request() is called with masked_irq=IDE_NO_IRQ=-1 so this code is executed for sure.

Quoted 3 lines · Bartlomiej Zolnierkiewicz

Previously callers called it with masked_irq=0 and disabling/enabling hwif->irq code wasn't executed, now ide_do_request() is called with masked_irq=IDE_NO_IRQ=-1 so this code is executed for sure.

You are right - I botched the simplification of that. The logic is actually cleaner than I did with a bit more thought - IDE_NO_IRQ can go away and we should be using hwif->irq as the argument.

Quoted 4 lines · Dominik Brodowski

Quoted 3 lines · Alan Cox

I've seen 3 or 4 reports of this, none of them duplicatable with the same IDE code on 2.4 so far. Which is odd but I don't yet understand what is going on.

Quoted 1 line

/me neither, unfortunately :-(

Alan, I can trigger the same dma_intr bug under 2.4.21-pre5-ac3 but not 2.4.20-ac2 (VIA vt8235 + WD800JB so it is not controller/disk related).

Dominik could you try attached patch with vanilla 2.5.65?
It reverts previous logic of handling masked_irq argument of ide_do_request().

BTW 2.5.64-ac4 deadlocks for me the same way Dominik has described.