* [PATCH] ide-dma blacklist behaviour broken
@ 2004-10-05 14:20 Jens Axboe
2004-10-05 15:37 ` Christoph Hellwig
2004-10-10 0:42 ` Bartlomiej Zolnierkiewicz
0 siblings, 2 replies; 12+ messages in thread
From: Jens Axboe @ 2004-10-05 14:20 UTC (permalink / raw)
To: Linux Kernel, Bartlomiej Zolnierkiewicz
Hi,
The blacklist stuff is broken. When set_using_dma() calls into
ide_dma_check(), it returns ide_dma_off() for a blacklisted drive. This
of course succeeds, returning success to the caller of ide_dma_check().
Not so good... It then uncondtionally calls ide_dma_on(), which turns on
dma for the drive.
This moves the check to ide_dma_on() so we also catch the buggy
->ide_dma_check() defined by various chipset drivers.
--- drivers/ide/ide-dma.c~ 2004-10-05 16:11:49.631910586 +0200
+++ drivers/ide/ide-dma.c 2004-10-05 16:21:58.828330845 +0200
@@ -354,11 +355,13 @@
struct hd_driveid *id = drive->id;
ide_hwif_t *hwif = HWIF(drive);
- if ((id->capability & 1) && hwif->autodma) {
- /* Consult the list of known "bad" drives */
- if (__ide_dma_bad_drive(drive))
- return __ide_dma_off(drive);
+ /* Consult the list of known "bad" drives */
+ if (__ide_dma_bad_drive(drive)) {
+ __ide_dma_off(drive);
+ return 1;
+ }
+ if ((id->capability & 1) && hwif->autodma) {
/*
* Enable DMA on any drive that has
* UltraDMA (mode 0/1/2/3/4/5/6) enabled
@@ -512,6 +515,9 @@
int __ide_dma_on (ide_drive_t *drive)
{
+ if (__ide_dma_bad_drive(drive))
+ return 1;
+
drive->using_dma = 1;
ide_toggle_bounce(drive, 1);
--
Jens Axboe
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] ide-dma blacklist behaviour broken
2004-10-05 14:20 [PATCH] ide-dma blacklist behaviour broken Jens Axboe
@ 2004-10-05 15:37 ` Christoph Hellwig
2004-10-05 15:46 ` Jens Axboe
2004-10-05 19:30 ` Juri Haberland
2004-10-10 0:42 ` Bartlomiej Zolnierkiewicz
1 sibling, 2 replies; 12+ messages in thread
From: Christoph Hellwig @ 2004-10-05 15:37 UTC (permalink / raw)
To: Jens Axboe; +Cc: Linux Kernel, Bartlomiej Zolnierkiewicz
On Tue, Oct 05, 2004 at 04:20:01PM +0200, Jens Axboe wrote:
> Hi,
>
> The blacklist stuff is broken. When set_using_dma() calls into
> ide_dma_check(), it returns ide_dma_off() for a blacklisted drive. This
> of course succeeds, returning success to the caller of ide_dma_check().
> Not so good... It then uncondtionally calls ide_dma_on(), which turns on
> dma for the drive.
>
> This moves the check to ide_dma_on() so we also catch the buggy
> ->ide_dma_check() defined by various chipset drivers.
Is this a bug introduced in the 2.6.9ish IDE changes or has it been there
for a longer time?
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] ide-dma blacklist behaviour broken
2004-10-05 15:37 ` Christoph Hellwig
@ 2004-10-05 15:46 ` Jens Axboe
2004-10-05 22:46 ` Alan Cox
2004-10-05 19:30 ` Juri Haberland
1 sibling, 1 reply; 12+ messages in thread
From: Jens Axboe @ 2004-10-05 15:46 UTC (permalink / raw)
To: Christoph Hellwig, Linux Kernel, Bartlomiej Zolnierkiewicz
On Tue, Oct 05 2004, Christoph Hellwig wrote:
> On Tue, Oct 05, 2004 at 04:20:01PM +0200, Jens Axboe wrote:
> > Hi,
> >
> > The blacklist stuff is broken. When set_using_dma() calls into
> > ide_dma_check(), it returns ide_dma_off() for a blacklisted drive. This
> > of course succeeds, returning success to the caller of ide_dma_check().
> > Not so good... It then uncondtionally calls ide_dma_on(), which turns on
> > dma for the drive.
> >
> > This moves the check to ide_dma_on() so we also catch the buggy
> > ->ide_dma_check() defined by various chipset drivers.
>
> Is this a bug introduced in the 2.6.9ish IDE changes or has it been there
> for a longer time?
I didn't check, someone just reported today. But looking at eg 2.6.5, it
seems to have the same bug. So it's likely very old.
--
Jens Axboe
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] ide-dma blacklist behaviour broken
2004-10-05 15:46 ` Jens Axboe
@ 2004-10-05 22:46 ` Alan Cox
2004-10-06 5:45 ` Jens Axboe
0 siblings, 1 reply; 12+ messages in thread
From: Alan Cox @ 2004-10-05 22:46 UTC (permalink / raw)
To: Jens Axboe
Cc: Christoph Hellwig, Linux Kernel Mailing List,
Bartlomiej Zolnierkiewicz
On Maw, 2004-10-05 at 16:46, Jens Axboe wrote:
> I didn't check, someone just reported today. But looking at eg 2.6.5, it
> seems to have the same bug. So it's likely very old.
We should actually probably nuke most of the IDE blacklist, much of the
CD-ROM blacklist arose because we DMA rather than PIO'd the ATAPI CDB.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] ide-dma blacklist behaviour broken
2004-10-05 22:46 ` Alan Cox
@ 2004-10-06 5:45 ` Jens Axboe
2004-10-06 13:27 ` Alan Cox
0 siblings, 1 reply; 12+ messages in thread
From: Jens Axboe @ 2004-10-06 5:45 UTC (permalink / raw)
To: Alan Cox
Cc: Christoph Hellwig, Linux Kernel Mailing List,
Bartlomiej Zolnierkiewicz
On Tue, Oct 05 2004, Alan Cox wrote:
> On Maw, 2004-10-05 at 16:46, Jens Axboe wrote:
> > I didn't check, someone just reported today. But looking at eg 2.6.5, it
> > seems to have the same bug. So it's likely very old.
>
> We should actually probably nuke most of the IDE blacklist, much of the
> CD-ROM blacklist arose because we DMA rather than PIO'd the ATAPI CDB.
Hmm? When have we ever done that?
--
Jens Axboe
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] ide-dma blacklist behaviour broken
2004-10-06 5:45 ` Jens Axboe
@ 2004-10-06 13:27 ` Alan Cox
2004-10-06 14:41 ` Jens Axboe
0 siblings, 1 reply; 12+ messages in thread
From: Alan Cox @ 2004-10-06 13:27 UTC (permalink / raw)
To: Jens Axboe
Cc: Christoph Hellwig, Linux Kernel Mailing List,
Bartlomiej Zolnierkiewicz
On Mer, 2004-10-06 at 06:45, Jens Axboe wrote:
> > We should actually probably nuke most of the IDE blacklist, much of the
> > CD-ROM blacklist arose because we DMA rather than PIO'd the ATAPI CDB.
>
> Hmm? When have we ever done that?
2.0, 2.2, 2.4 to about 2.4.18 or so (Khalid Aziz eventually pinned it
down and fixed it).
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] ide-dma blacklist behaviour broken
2004-10-06 13:27 ` Alan Cox
@ 2004-10-06 14:41 ` Jens Axboe
2004-10-06 13:58 ` Alan Cox
0 siblings, 1 reply; 12+ messages in thread
From: Jens Axboe @ 2004-10-06 14:41 UTC (permalink / raw)
To: Alan Cox
Cc: Christoph Hellwig, Linux Kernel Mailing List,
Bartlomiej Zolnierkiewicz
On Wed, Oct 06 2004, Alan Cox wrote:
> On Mer, 2004-10-06 at 06:45, Jens Axboe wrote:
> > > We should actually probably nuke most of the IDE blacklist, much of the
> > > CD-ROM blacklist arose because we DMA rather than PIO'd the ATAPI CDB.
> >
> > Hmm? When have we ever done that?
>
> 2.0, 2.2, 2.4 to about 2.4.18 or so (Khalid Aziz eventually pinned it
> down and fixed it).
Ah, I think you are misreading it. It wasn't the DMA'ing of the atapi
cdb, that was always pio'ed to the drive. But DMA for the command itself
was turned on before the cdb had been transferred.
--
Jens Axboe
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] ide-dma blacklist behaviour broken
2004-10-06 14:41 ` Jens Axboe
@ 2004-10-06 13:58 ` Alan Cox
2004-10-06 15:01 ` Jens Axboe
0 siblings, 1 reply; 12+ messages in thread
From: Alan Cox @ 2004-10-06 13:58 UTC (permalink / raw)
To: Jens Axboe
Cc: Christoph Hellwig, Linux Kernel Mailing List,
Bartlomiej Zolnierkiewicz
On Mer, 2004-10-06 at 15:41, Jens Axboe wrote:
> Ah, I think you are misreading it. It wasn't the DMA'ing of the atapi
> cdb, that was always pio'ed to the drive. But DMA for the command itself
> was turned on before the cdb had been transferred.
Yep. I may have the detail wrong, its a long time ago. That fixed
several CD's that were on the blacklist and most of the others may well
never have been tested.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] ide-dma blacklist behaviour broken
2004-10-06 13:58 ` Alan Cox
@ 2004-10-06 15:01 ` Jens Axboe
0 siblings, 0 replies; 12+ messages in thread
From: Jens Axboe @ 2004-10-06 15:01 UTC (permalink / raw)
To: Alan Cox
Cc: Christoph Hellwig, Linux Kernel Mailing List,
Bartlomiej Zolnierkiewicz
On Wed, Oct 06 2004, Alan Cox wrote:
> On Mer, 2004-10-06 at 15:41, Jens Axboe wrote:
> > Ah, I think you are misreading it. It wasn't the DMA'ing of the atapi
> > cdb, that was always pio'ed to the drive. But DMA for the command itself
> > was turned on before the cdb had been transferred.
>
> Yep. I may have the detail wrong, its a long time ago. That fixed
> several CD's that were on the blacklist and most of the others may well
> never have been tested.
Yeah, it was a very nasty bug. So do you suggest we try and scrap the
atapi drives from the blacklist?
--
Jens Axboe
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] ide-dma blacklist behaviour broken
2004-10-05 15:37 ` Christoph Hellwig
2004-10-05 15:46 ` Jens Axboe
@ 2004-10-05 19:30 ` Juri Haberland
1 sibling, 0 replies; 12+ messages in thread
From: Juri Haberland @ 2004-10-05 19:30 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Linux Kernel, Bartlomiej Zolnierkiewicz, axboe
Christoph Hellwig wrote:
> On Tue, Oct 05, 2004 at 04:20:01PM +0200, Jens Axboe wrote:
>> Hi,
>>
>> The blacklist stuff is broken. When set_using_dma() calls into
>> ide_dma_check(), it returns ide_dma_off() for a blacklisted drive. This
>> of course succeeds, returning success to the caller of ide_dma_check().
>> Not so good... It then uncondtionally calls ide_dma_on(), which turns on
>> dma for the drive.
>>
>> This moves the check to ide_dma_on() so we also catch the buggy
>> ->ide_dma_check() defined by various chipset drivers.
>
> Is this a bug introduced in the 2.6.9ish IDE changes or has it been there
> for a longer time?
Looks like it is also in 2.4.27.
Juri
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] ide-dma blacklist behaviour broken
2004-10-05 14:20 [PATCH] ide-dma blacklist behaviour broken Jens Axboe
2004-10-05 15:37 ` Christoph Hellwig
@ 2004-10-10 0:42 ` Bartlomiej Zolnierkiewicz
2004-10-10 8:14 ` Jens Axboe
1 sibling, 1 reply; 12+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2004-10-10 0:42 UTC (permalink / raw)
To: Jens Axboe; +Cc: Linux Kernel
[ linux-ide MIA ]
On Tuesday 05 October 2004 16:20, Jens Axboe wrote:
> Hi,
>
> The blacklist stuff is broken. When set_using_dma() calls into
> ide_dma_check(), it returns ide_dma_off() for a blacklisted drive. This
> of course succeeds, returning success to the caller of ide_dma_check().
> Not so good... It then uncondtionally calls ide_dma_on(), which turns on
> dma for the drive.
- s/ide_dma_check/->ide_dma_check/
- s/ide_dma_off/__ide_dma_off/
> This moves the check to ide_dma_on() so we also catch the buggy
> ->ide_dma_check() defined by various chipset drivers.
Yep, good catch.
> --- drivers/ide/ide-dma.c~ 2004-10-05 16:11:49.631910586 +0200
> +++ drivers/ide/ide-dma.c 2004-10-05 16:21:58.828330845 +0200
> @@ -354,11 +355,13 @@
> struct hd_driveid *id = drive->id;
> ide_hwif_t *hwif = HWIF(drive);
>
> - if ((id->capability & 1) && hwif->autodma) {
> - /* Consult the list of known "bad" drives */
> - if (__ide_dma_bad_drive(drive))
> - return __ide_dma_off(drive);
> + /* Consult the list of known "bad" drives */
> + if (__ide_dma_bad_drive(drive)) {
> + __ide_dma_off(drive);
> + return 1;
> + }
>
> + if ((id->capability & 1) && hwif->autodma) {
> /*
> * Enable DMA on any drive that has
> * UltraDMA (mode 0/1/2/3/4/5/6) enabled
Is __ide_dma_bad_drive() check still needed?
Doesn't __ide_dma_on() fix handle this now?
> @@ -512,6 +515,9 @@
>
> int __ide_dma_on (ide_drive_t *drive)
> {
> + if (__ide_dma_bad_drive(drive))
> + return 1;
> +
> drive->using_dma = 1;
> ide_toggle_bounce(drive, 1);
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] ide-dma blacklist behaviour broken
2004-10-10 0:42 ` Bartlomiej Zolnierkiewicz
@ 2004-10-10 8:14 ` Jens Axboe
0 siblings, 0 replies; 12+ messages in thread
From: Jens Axboe @ 2004-10-10 8:14 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz; +Cc: Linux Kernel
On Sun, Oct 10 2004, Bartlomiej Zolnierkiewicz wrote:
> > --- drivers/ide/ide-dma.c~ 2004-10-05 16:11:49.631910586 +0200
> > +++ drivers/ide/ide-dma.c 2004-10-05 16:21:58.828330845 +0200
> > @@ -354,11 +355,13 @@
> > struct hd_driveid *id = drive->id;
> > ide_hwif_t *hwif = HWIF(drive);
> >
> > - if ((id->capability & 1) && hwif->autodma) {
> > - /* Consult the list of known "bad" drives */
> > - if (__ide_dma_bad_drive(drive))
> > - return __ide_dma_off(drive);
> > + /* Consult the list of known "bad" drives */
> > + if (__ide_dma_bad_drive(drive)) {
> > + __ide_dma_off(drive);
> > + return 1;
> > + }
> >
> > + if ((id->capability & 1) && hwif->autodma) {
> > /*
> > * Enable DMA on any drive that has
> > * UltraDMA (mode 0/1/2/3/4/5/6) enabled
>
> Is __ide_dma_bad_drive() check still needed?
> Doesn't __ide_dma_on() fix handle this now?
Yeah, we can solely rely on that if we so wish.
--
Jens Axboe
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2004-10-10 8:15 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-10-05 14:20 [PATCH] ide-dma blacklist behaviour broken Jens Axboe
2004-10-05 15:37 ` Christoph Hellwig
2004-10-05 15:46 ` Jens Axboe
2004-10-05 22:46 ` Alan Cox
2004-10-06 5:45 ` Jens Axboe
2004-10-06 13:27 ` Alan Cox
2004-10-06 14:41 ` Jens Axboe
2004-10-06 13:58 ` Alan Cox
2004-10-06 15:01 ` Jens Axboe
2004-10-05 19:30 ` Juri Haberland
2004-10-10 0:42 ` Bartlomiej Zolnierkiewicz
2004-10-10 8:14 ` Jens Axboe
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.