2.4.17 Oops when trying to mount ATAPI CDROM

2.4.17 Oops when trying to mount ATAPI CDROM

[Manuel McLure]

Motherboard: ASUS P55T2P4 motherboard (Intel 430HX chipset)
CPU: AMD K6-III 400MHz
CD-ROM: No-name ATAPI CD-ROM installed as master on IDE channel 1 (hard 
drive is attached as master on IDE channel 0)
Kernel: Official 2.4.17 and Red Hat 2.4.9-21

Whenever I try to access the CD-ROM after boot, I get the following Oops:

Unable to handle kernel NULL pointer dereference at virtual address 
00000063
c0193030
*pde = 00000000
Oops: 0000
CPU:    0
EIP:    0010:[<c0193030>]    Not tainted
Using defaults from ksymoops -t elf32-i386 -a i386
EFLAGS: 00010246
eax: c02b7730   ebx: 00000000   ecx: 00000005   edx: 00000004
esi: 00000005   edi: c02b7730   ebp: 00000001   esp: c0f51ee0
ds: 0018   es: 0018   ss: 0018
Process modprobe (pid: 1180, stackpage=c0f51000)
Stack: c02b7730 00000286 c8db3960 c0189762 c02b7730 c02b7730 00000007 
c02b7730
        c3e03e00 c02b7730 00000134 c8db0a81 c02b7730 c8db3960 00000001 
c8dad000
        00000001 00000015 00000001 c01165d7 c8db38a0 c0ab0000 000067a0 
c0ab2000
Call Trace: [<c8db3960>] [<c0189762>] [<c8db0a81>] [<c8db3960>] 
[<c01165d7>]
    [<c8db38a0>] [<c8dad060>] [<c0106f13>]
Code: 8a 15 63 00 00 00 83 e2 08 75 06 f6 43 6a 02 74 0c be 07 00

>> EIP; c0193030 <config_drive_xfer_rate+d0/110>   <=====
Trace; c8db3960 <[ide-cd]ide_cdrom_driver+0/f>
Trace; c0189762 <ide_register_subdriver+92/100>
Trace; c8db0a81 <[ide-cd]init_module+a1/1af>
Trace; c8db3960 <[ide-cd]ide_cdrom_driver+0/f>
Trace; c01165d7 <sys_init_module+537/5f0>
Trace; c8db38a0 <[ide-cd].LC156+0/d>
Trace; c8dad060 <[ide-cd]__module_license+0/0>
Trace; c0106f13 <system_call+33/40>
Code;  c0193030 <config_drive_xfer_rate+d0/110>
00000000 <_EIP>:
Code;  c0193030 <config_drive_xfer_rate+d0/110>   <=====
    0:   8a 15 63 00 00 00         mov    0x63,%dl   <=====
Code;  c0193036 <config_drive_xfer_rate+d6/110>
    6:   83 e2 08                  and    $0x8,%edx
Code;  c0193039 <config_drive_xfer_rate+d9/110>
    9:   75 06                     jne    11 <_EIP+0x11> c0193041 
<config_drive_xfer_rate+e1/110>
Code;  c019303b <config_drive_xfer_rate+db/110>
    b:   f6 43 6a 02               testb  $0x2,0x6a(%ebx)
Code;  c019303f <config_drive_xfer_rate+df/110>
    f:   74 0c                     je     1d <_EIP+0x1d> c019304d 
<config_drive_xfer_rate+ed/110>
Code;  c0193041 <config_drive_xfer_rate+e1/110>
   11:   be 07 00 00 00            mov    $0x7,%esi

After the Oops, lsmod shows:

ide-cd                 27072   3  (initializing)

and the module is stuck at initializing. 
This CD-ROM worked fine on 2.2.20 as long as I booted with "hdc=noprobe 
hdc=cdrom" (otherwise I'd get "lost interrupt" messages). I have tried 
"ide=nodma" with no effect, as well as trying with the "hdc=" options both 
on and off. Running "hdparm -d0 /dev/hdc" before trying the mount causes 
the same Oops.

Any ideas?

Thanks!
-- 
Manuel A. McLure KE6TAW | ...for in Ulthar, according to an ancient
<manuel@mclure.org>     | and significant law, no man may kill a cat.
<http://www.mclure.org> |             -- H.P. Lovecraft

Re: 2.4.17 Oops when trying to mount ATAPI CDROM

[Andre Hedrick]

Manuel,

Would you be kind enough to be a little more specific on the hardware?
The attached devices bu make model and real vender if known.

Regards,

Andre Hedrick
Linux Disk Certification Project                Linux ATA Development

On Sat, 2 Feb 2002, Manuel McLure wrote:

> Motherboard: ASUS P55T2P4 motherboard (Intel 430HX chipset)
> CPU: AMD K6-III 400MHz
> CD-ROM: No-name ATAPI CD-ROM installed as master on IDE channel 1 (hard 
> drive is attached as master on IDE channel 0)
> Kernel: Official 2.4.17 and Red Hat 2.4.9-21
> 
> Whenever I try to access the CD-ROM after boot, I get the following Oops:
> 
> Unable to handle kernel NULL pointer dereference at virtual address 
> 00000063
> c0193030
> *pde = 00000000
> Oops: 0000
> CPU:    0
> EIP:    0010:[<c0193030>]    Not tainted
> Using defaults from ksymoops -t elf32-i386 -a i386
> EFLAGS: 00010246
> eax: c02b7730   ebx: 00000000   ecx: 00000005   edx: 00000004
> esi: 00000005   edi: c02b7730   ebp: 00000001   esp: c0f51ee0
> ds: 0018   es: 0018   ss: 0018
> Process modprobe (pid: 1180, stackpage=c0f51000)
> Stack: c02b7730 00000286 c8db3960 c0189762 c02b7730 c02b7730 00000007 
> c02b7730
>         c3e03e00 c02b7730 00000134 c8db0a81 c02b7730 c8db3960 00000001 
> c8dad000
>         00000001 00000015 00000001 c01165d7 c8db38a0 c0ab0000 000067a0 
> c0ab2000
> Call Trace: [<c8db3960>] [<c0189762>] [<c8db0a81>] [<c8db3960>] 
> [<c01165d7>]
>     [<c8db38a0>] [<c8dad060>] [<c0106f13>]
> Code: 8a 15 63 00 00 00 83 e2 08 75 06 f6 43 6a 02 74 0c be 07 00
> 
> >> EIP; c0193030 <config_drive_xfer_rate+d0/110>   <=====
> Trace; c8db3960 <[ide-cd]ide_cdrom_driver+0/f>
> Trace; c0189762 <ide_register_subdriver+92/100>
> Trace; c8db0a81 <[ide-cd]init_module+a1/1af>
> Trace; c8db3960 <[ide-cd]ide_cdrom_driver+0/f>
> Trace; c01165d7 <sys_init_module+537/5f0>
> Trace; c8db38a0 <[ide-cd].LC156+0/d>
> Trace; c8dad060 <[ide-cd]__module_license+0/0>
> Trace; c0106f13 <system_call+33/40>
> Code;  c0193030 <config_drive_xfer_rate+d0/110>
> 00000000 <_EIP>:
> Code;  c0193030 <config_drive_xfer_rate+d0/110>   <=====
>     0:   8a 15 63 00 00 00         mov    0x63,%dl   <=====
> Code;  c0193036 <config_drive_xfer_rate+d6/110>
>     6:   83 e2 08                  and    $0x8,%edx
> Code;  c0193039 <config_drive_xfer_rate+d9/110>
>     9:   75 06                     jne    11 <_EIP+0x11> c0193041 
> <config_drive_xfer_rate+e1/110>
> Code;  c019303b <config_drive_xfer_rate+db/110>
>     b:   f6 43 6a 02               testb  $0x2,0x6a(%ebx)
> Code;  c019303f <config_drive_xfer_rate+df/110>
>     f:   74 0c                     je     1d <_EIP+0x1d> c019304d 
> <config_drive_xfer_rate+ed/110>
> Code;  c0193041 <config_drive_xfer_rate+e1/110>
>    11:   be 07 00 00 00            mov    $0x7,%esi
> 
> After the Oops, lsmod shows:
> 
> ide-cd                 27072   3  (initializing)
> 
> and the module is stuck at initializing. 
> This CD-ROM worked fine on 2.2.20 as long as I booted with "hdc=noprobe 
> hdc=cdrom" (otherwise I'd get "lost interrupt" messages). I have tried 
> "ide=nodma" with no effect, as well as trying with the "hdc=" options both 
> on and off. Running "hdparm -d0 /dev/hdc" before trying the mount causes 
> the same Oops.
> 
> Any ideas?
> 
> Thanks!
> -- 
> Manuel A. McLure KE6TAW | ...for in Ulthar, according to an ancient
> <manuel@mclure.org>     | and significant law, no man may kill a cat.
> <http://www.mclure.org> |             -- H.P. Lovecraft
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: 2.4.17 Oops when trying to mount ATAPI CDROM

[Andrew Morton]

Andre Hedrick wrote:
> 
> Manuel,
> 
> Would you be kind enough to be a little more specific on the hardware?
> The attached devices bu make model and real vender if known.
> 
>

There are eight different config_drive_xfer_rate() functions.
And they all basically do this:

	if (id && ...) {
		...
	} else if (id->xxx)

So either

1: The first test for id=NULL is unneeded or

2: id can sometimes be null, which will oops in the way
   Manuel describes.

-

Re: 2.4.17 Oops when trying to mount ATAPI CDROM

[Manuel McLure]

On 2002.02.02 22:04 Andre Hedrick wrote:
> 
> Manuel,
> 
> Would you be kind enough to be a little more specific on the hardware?
> The attached devices bu make model and real vender if known.
kml/

The CD-ROM is detected as a Pioneer CD-ROM ATAPI Model DR-A24X 0104 - I 
haven't opened the case to look at it but I do recall that it is 
definitely a 24X Pioneer ATAPI CDROM.

-- 
Manuel A. McLure KE6TAW | ...for in Ulthar, according to an ancient
<manuel@mclure.org>     | and significant law, no man may kill a cat.
<http://www.mclure.org> |             -- H.P. Lovecraft

Re: 2.4.17 Oops when trying to mount ATAPI CDROM

[Manuel McLure]

On 2002.02.03 10:21 Manuel McLure wrote:
> 
> On 2002.02.02 22:04 Andre Hedrick wrote:
> >
> > Manuel,
> >
> > Would you be kind enough to be a little more specific on the hardware?
> > The attached devices bu make model and real vender if known.
> kml/
> 
> The CD-ROM is detected as a Pioneer CD-ROM ATAPI Model DR-A24X 0104 - I
> haven't opened the case to look at it but I do recall that it is
> definitely a 24X Pioneer ATAPI CDROM.
> 

Some more information - if I boot without "hdc=noprobe hdc=cdrom", I don't 
get the oops whel loading the "ide-cd" module - instead I get

hdc: set_drive_speed_status: status=0x00 { }
hdc: lost interrupt
hdc: ATAPI 20X CD-ROM drive, 128kB Cache, DMA
Uniform CD-ROM driver Revision: 3.12
hdc: lost interrupt
hdc: lost interrupt
hdc: lost interrupt

The module eventually finishes initializing but is not usable due to the 
"lost interrupt"s.

-- 
Manuel A. McLure KE6TAW | ...for in Ulthar, according to an ancient
<manuel@mclure.org>     | and significant law, no man may kill a cat.
<http://www.mclure.org> |             -- H.P. Lovecraft

Re: 2.4.17 Oops when trying to mount ATAPI CDROM

[Jens Axboe]

On Sun, Feb 03 2002, Manuel McLure wrote:
> 
> On 2002.02.03 10:21 Manuel McLure wrote:
> > 
> > On 2002.02.02 22:04 Andre Hedrick wrote:
> > >
> > > Manuel,
> > >
> > > Would you be kind enough to be a little more specific on the hardware?
> > > The attached devices bu make model and real vender if known.
> > kml/
> > 
> > The CD-ROM is detected as a Pioneer CD-ROM ATAPI Model DR-A24X 0104 - I
> > haven't opened the case to look at it but I do recall that it is
> > definitely a 24X Pioneer ATAPI CDROM.
> > 
> 
> Some more information - if I boot without "hdc=noprobe hdc=cdrom", I don't 
> get the oops whel loading the "ide-cd" module - instead I get
> 
> hdc: set_drive_speed_status: status=0x00 { }
> hdc: lost interrupt
> hdc: ATAPI 20X CD-ROM drive, 128kB Cache, DMA
> Uniform CD-ROM driver Revision: 3.12
> hdc: lost interrupt
> hdc: lost interrupt
> hdc: lost interrupt
> 
> The module eventually finishes initializing but is not usable due to the 
> "lost interrupt"s.

That particular Pioneer model has never worked well in Linux. I have on
here that at least sort-of works when noprobe is used (the Linux
detection probe really screws it up).

My recommandation -- get another drive. The oops would be nice to have
fixed, but don't expect this drive to ever work well.

-- 
Jens Axboe

[PATCH] Re: 2.4.17 Oops when trying to mount ATAPI CDROM

[Zwane Mwaikambo]

Patch to fix the oops as encountered by Manuel McLure

Quick browse of the trace...

<snip>
Code;  c0193030 <config_drive_xfer_rate+d0/110>
    0:   8a 15 63 00 00 00         mov    0x63,%dl <== Oops happens here
Code;  c0193036 <config_drive_xfer_rate+d6/110>
    6:   83 e2 08                  and    $0x8,%edx <== [1]
<snip>

specific_chipset_driver.c:
config_drive_xfer_rate (ide_drive_t *drive)
{
	struct hd_driveid *id	= drive->id;
[...]
	if (id && (id->capability & 1) && HWIF(drive)->autodma) { <== [1]
[...]
	} else if ((id->capability & 8) \ <== 2
	|| (id->field_valid & 2)) {
<snip>

[1] In this case we get to 2 because we failed the if (id) check, but then we don't check again...

Patch diffed against 2.4.18-pre7 and applies to 2.5.3-pre6 with offsets.

--- linux-2.4.18-pre7/drivers/ide/aec62xx.c.orig	Mon Feb  4 22:35:54 2002
+++ linux-2.4.18-pre7/drivers/ide/aec62xx.c	Mon Feb  4 23:00:19 2002
@@ -445,7 +445,10 @@
 	struct hd_driveid *id = drive->id;
 	ide_dma_action_t dma_func = ide_dma_on;
 
-	if (id && (id->capability & 1) && HWIF(drive)->autodma) {
+	if (id == NULL)
+		goto done;
+
+	if ((id->capability & 1) && HWIF(drive)->autodma) {
 		/* Consult the list of known "bad" drives */
 		if (ide_dmaproc(ide_dma_bad_drive, drive)) {
 			dma_func = ide_dma_off;
@@ -486,6 +489,7 @@
 no_dma_set:
 		aec62xx_tune_drive(drive, 5);
 	}
+done:
 	return HWIF(drive)->dmaproc(dma_func, drive);
 }
 
--- linux-2.4.18-pre7/drivers/ide/amd74xx.c.orig	Mon Feb  4 22:36:29 2002
+++ linux-2.4.18-pre7/drivers/ide/amd74xx.c	Mon Feb  4 23:00:30 2002
@@ -344,7 +344,10 @@
 	struct hd_driveid *id = drive->id;
 	ide_dma_action_t dma_func = ide_dma_on;
 
-	if (id && (id->capability & 1) && HWIF(drive)->autodma) {
+	if (id == NULL)
+		goto done;
+
+	if ((id->capability & 1) && HWIF(drive)->autodma) {
 		/* Consult the list of known "bad" drives */
 		if (ide_dmaproc(ide_dma_bad_drive, drive)) {
 			dma_func = ide_dma_off;
@@ -388,6 +391,7 @@
 
 		config_chipset_for_pio(drive);
 	}
+done:
 	return HWIF(drive)->dmaproc(dma_func, drive);
 }
 
--- linux-2.4.18-pre7/drivers/ide/hpt34x.c.orig	Mon Feb  4 22:36:51 2002
+++ linux-2.4.18-pre7/drivers/ide/hpt34x.c	Mon Feb  4 22:56:37 2002
@@ -255,7 +255,10 @@
 	struct hd_driveid *id = drive->id;
 	ide_dma_action_t dma_func = ide_dma_on;
 
-	if (id && (id->capability & 1) && HWIF(drive)->autodma) {
+	if (id == NULL)
+		goto done;
+
+	if ((id->capability & 1) && HWIF(drive)->autodma) {
 		/* Consult the list of known "bad" drives */
 		if (ide_dmaproc(ide_dma_bad_drive, drive)) {
 			dma_func = ide_dma_off;
@@ -302,6 +305,7 @@
 		dma_func = ide_dma_off;
 #endif /* CONFIG_HPT34X_AUTODMA */
 
+done:
 	return HWIF(drive)->dmaproc(dma_func, drive);
 }
 
--- linux-2.4.18-pre7/drivers/ide/hpt366.c.orig	Mon Feb  4 22:37:18 2002
+++ linux-2.4.18-pre7/drivers/ide/hpt366.c	Mon Feb  4 23:00:03 2002
@@ -553,7 +553,10 @@
 	struct hd_driveid *id = drive->id;
 	ide_dma_action_t dma_func = ide_dma_on;
 
-	if (id && (id->capability & 1) && HWIF(drive)->autodma) {
+	if (id == NULL)
+		goto done;
+
+	if ((id->capability & 1) && HWIF(drive)->autodma) {
 		/* Consult the list of known "bad" drives */
 		if (ide_dmaproc(ide_dma_bad_drive, drive)) {
 			dma_func = ide_dma_off;
@@ -594,6 +597,7 @@
 
 		config_chipset_for_pio(drive);
 	}
+done:
 	return HWIF(drive)->dmaproc(dma_func, drive);
 }
 
--- linux-2.4.18-pre7/drivers/ide/pdc202xx.c.orig	Mon Feb  4 22:37:39 2002
+++ linux-2.4.18-pre7/drivers/ide/pdc202xx.c	Mon Feb  4 23:04:00 2002
@@ -677,7 +677,10 @@
 	ide_hwif_t *hwif = HWIF(drive);
 	ide_dma_action_t dma_func = ide_dma_off_quietly;
 
-	if (id && (id->capability & 1) && hwif->autodma) {
+	if (id == NULL)
+		goto done;
+
+	if ((id->capability & 1) && hwif->autodma) {
 		/* Consult the list of known "bad" drives */
 		if (ide_dmaproc(ide_dma_bad_drive, drive)) {
 			dma_func = ide_dma_off;
@@ -718,7 +721,7 @@
 no_dma_set:
 		(void) config_chipset_for_pio(drive, 5);
 	}
-
+done:
 	return HWIF(drive)->dmaproc(dma_func, drive);
 }
 
--- linux-2.4.18-pre7/drivers/ide/piix.c.orig	Mon Feb  4 22:37:57 2002
+++ linux-2.4.18-pre7/drivers/ide/piix.c	Tue Feb  5 01:00:26 2002
@@ -417,7 +417,10 @@
 	struct hd_driveid *id = drive->id;
 	ide_dma_action_t dma_func = ide_dma_on;
 
-	if (id && (id->capability & 1) && HWIF(drive)->autodma) {
+	if (id == NULL)
+		goto done;
+
+	if ((id->capability & 1) && HWIF(drive)->autodma) {
 		/* Consult the list of known "bad" drives */
 		if (ide_dmaproc(ide_dma_bad_drive, drive)) {
 			dma_func = ide_dma_off;
@@ -458,6 +461,7 @@
 no_dma_set:
 		config_chipset_for_pio(drive);
 	}
+done:
 	return HWIF(drive)->dmaproc(dma_func, drive);
 }
 
--- linux-2.4.18-pre7/drivers/ide/serverworks.c.orig	Mon Feb  4 22:38:16 2002
+++ linux-2.4.18-pre7/drivers/ide/serverworks.c	Mon Feb  4 23:05:07 2002
@@ -450,7 +450,10 @@
 	struct hd_driveid *id = drive->id;
 	ide_dma_action_t dma_func = ide_dma_on;
 
-	if (id && (id->capability & 1) && HWIF(drive)->autodma) {
+	if (id == NULL)
+		goto done;
+
+	if ((id->capability & 1) && HWIF(drive)->autodma) {
 		/* Consult the list of known "bad" drives */
 		if (ide_dmaproc(ide_dma_bad_drive, drive)) {
 			dma_func = ide_dma_off;
@@ -491,6 +494,7 @@
 no_dma_set:
 		config_chipset_for_pio(drive);
 	}
+done:
 	return HWIF(drive)->dmaproc(dma_func, drive);
 }
 
--- linux-2.4.18-pre7/drivers/ide/sis5513.c.orig	Mon Feb  4 22:22:42 2002
+++ linux-2.4.18-pre7/drivers/ide/sis5513.c	Tue Feb  5 00:59:43 2002
@@ -506,8 +506,11 @@
 {
 	struct hd_driveid *id		= drive->id;
 	ide_dma_action_t dma_func	= ide_dma_off_quietly;
+	
+	if (id == NULL)
+		goto done;
 
-	if (id && (id->capability & 1) && HWIF(drive)->autodma) {
+	if ((id->capability & 1) && HWIF(drive)->autodma) {
 		/* Consult the list of known "bad" drives */
 		if (ide_dmaproc(ide_dma_bad_drive, drive)) {
 			dma_func = ide_dma_off;
@@ -546,7 +549,7 @@
 no_dma_set:
 		(void) config_chipset_for_pio(drive, 5);
 	}
-
+done:
 	return HWIF(drive)->dmaproc(dma_func, drive);
 }
 

Re: [PATCH] Re: 2.4.17 Oops when trying to mount ATAPI CDROM

[Andre Hedrick]

Folks there is a bigger problem if we are able to get to loading a
sub-driver and we do not have a valid copy of the identify page attached
to the drive struct.  The resulting patch below is a nice bounds check for
presense of "struct hd_driveid *id = drive->id" but this should have
bombed out long before hand.  I do not know if the suspend to swap has
altered the behavor or not, but the driver has lost it knowledge about the
device attached.

Andre Hedrick
Linux Disk Certification Project                Linux ATA Development

On Tue, 5 Feb 2002, Zwane Mwaikambo wrote:

> Patch to fix the oops as encountered by Manuel McLure
> 
> Quick browse of the trace...
> 
> <snip>
> Code;  c0193030 <config_drive_xfer_rate+d0/110>
>     0:   8a 15 63 00 00 00         mov    0x63,%dl <== Oops happens here
> Code;  c0193036 <config_drive_xfer_rate+d6/110>
>     6:   83 e2 08                  and    $0x8,%edx <== [1]
> <snip>
> 
> specific_chipset_driver.c:
> config_drive_xfer_rate (ide_drive_t *drive)
> {
> 	struct hd_driveid *id	= drive->id;
> [...]
> 	if (id && (id->capability & 1) && HWIF(drive)->autodma) { <== [1]
> [...]
> 	} else if ((id->capability & 8) \ <== 2
> 	|| (id->field_valid & 2)) {
> <snip>
> 
> [1] In this case we get to 2 because we failed the if (id) check, but then we don't check again...
> 
> Patch diffed against 2.4.18-pre7 and applies to 2.5.3-pre6 with offsets.
> 
> --- linux-2.4.18-pre7/drivers/ide/aec62xx.c.orig	Mon Feb  4 22:35:54 2002
> +++ linux-2.4.18-pre7/drivers/ide/aec62xx.c	Mon Feb  4 23:00:19 2002
> @@ -445,7 +445,10 @@
>  	struct hd_driveid *id = drive->id;
>  	ide_dma_action_t dma_func = ide_dma_on;
>  
> -	if (id && (id->capability & 1) && HWIF(drive)->autodma) {
> +	if (id == NULL)
> +		goto done;
> +
> +	if ((id->capability & 1) && HWIF(drive)->autodma) {
>  		/* Consult the list of known "bad" drives */
>  		if (ide_dmaproc(ide_dma_bad_drive, drive)) {
>  			dma_func = ide_dma_off;
> @@ -486,6 +489,7 @@
>  no_dma_set:
>  		aec62xx_tune_drive(drive, 5);
>  	}
> +done:
>  	return HWIF(drive)->dmaproc(dma_func, drive);
>  }
>  
> --- linux-2.4.18-pre7/drivers/ide/amd74xx.c.orig	Mon Feb  4 22:36:29 2002
> +++ linux-2.4.18-pre7/drivers/ide/amd74xx.c	Mon Feb  4 23:00:30 2002
> @@ -344,7 +344,10 @@
>  	struct hd_driveid *id = drive->id;
>  	ide_dma_action_t dma_func = ide_dma_on;
>  
> -	if (id && (id->capability & 1) && HWIF(drive)->autodma) {
> +	if (id == NULL)
> +		goto done;
> +
> +	if ((id->capability & 1) && HWIF(drive)->autodma) {
>  		/* Consult the list of known "bad" drives */
>  		if (ide_dmaproc(ide_dma_bad_drive, drive)) {
>  			dma_func = ide_dma_off;
> @@ -388,6 +391,7 @@
>  
>  		config_chipset_for_pio(drive);
>  	}
> +done:
>  	return HWIF(drive)->dmaproc(dma_func, drive);
>  }
>  
> --- linux-2.4.18-pre7/drivers/ide/hpt34x.c.orig	Mon Feb  4 22:36:51 2002
> +++ linux-2.4.18-pre7/drivers/ide/hpt34x.c	Mon Feb  4 22:56:37 2002
> @@ -255,7 +255,10 @@
>  	struct hd_driveid *id = drive->id;
>  	ide_dma_action_t dma_func = ide_dma_on;
>  
> -	if (id && (id->capability & 1) && HWIF(drive)->autodma) {
> +	if (id == NULL)
> +		goto done;
> +
> +	if ((id->capability & 1) && HWIF(drive)->autodma) {
>  		/* Consult the list of known "bad" drives */
>  		if (ide_dmaproc(ide_dma_bad_drive, drive)) {
>  			dma_func = ide_dma_off;
> @@ -302,6 +305,7 @@
>  		dma_func = ide_dma_off;
>  #endif /* CONFIG_HPT34X_AUTODMA */
>  
> +done:
>  	return HWIF(drive)->dmaproc(dma_func, drive);
>  }
>  
> --- linux-2.4.18-pre7/drivers/ide/hpt366.c.orig	Mon Feb  4 22:37:18 2002
> +++ linux-2.4.18-pre7/drivers/ide/hpt366.c	Mon Feb  4 23:00:03 2002
> @@ -553,7 +553,10 @@
>  	struct hd_driveid *id = drive->id;
>  	ide_dma_action_t dma_func = ide_dma_on;
>  
> -	if (id && (id->capability & 1) && HWIF(drive)->autodma) {
> +	if (id == NULL)
> +		goto done;
> +
> +	if ((id->capability & 1) && HWIF(drive)->autodma) {
>  		/* Consult the list of known "bad" drives */
>  		if (ide_dmaproc(ide_dma_bad_drive, drive)) {
>  			dma_func = ide_dma_off;
> @@ -594,6 +597,7 @@
>  
>  		config_chipset_for_pio(drive);
>  	}
> +done:
>  	return HWIF(drive)->dmaproc(dma_func, drive);
>  }
>  
> --- linux-2.4.18-pre7/drivers/ide/pdc202xx.c.orig	Mon Feb  4 22:37:39 2002
> +++ linux-2.4.18-pre7/drivers/ide/pdc202xx.c	Mon Feb  4 23:04:00 2002
> @@ -677,7 +677,10 @@
>  	ide_hwif_t *hwif = HWIF(drive);
>  	ide_dma_action_t dma_func = ide_dma_off_quietly;
>  
> -	if (id && (id->capability & 1) && hwif->autodma) {
> +	if (id == NULL)
> +		goto done;
> +
> +	if ((id->capability & 1) && hwif->autodma) {
>  		/* Consult the list of known "bad" drives */
>  		if (ide_dmaproc(ide_dma_bad_drive, drive)) {
>  			dma_func = ide_dma_off;
> @@ -718,7 +721,7 @@
>  no_dma_set:
>  		(void) config_chipset_for_pio(drive, 5);
>  	}
> -
> +done:
>  	return HWIF(drive)->dmaproc(dma_func, drive);
>  }
>  
> --- linux-2.4.18-pre7/drivers/ide/piix.c.orig	Mon Feb  4 22:37:57 2002
> +++ linux-2.4.18-pre7/drivers/ide/piix.c	Tue Feb  5 01:00:26 2002
> @@ -417,7 +417,10 @@
>  	struct hd_driveid *id = drive->id;
>  	ide_dma_action_t dma_func = ide_dma_on;
>  
> -	if (id && (id->capability & 1) && HWIF(drive)->autodma) {
> +	if (id == NULL)
> +		goto done;
> +
> +	if ((id->capability & 1) && HWIF(drive)->autodma) {
>  		/* Consult the list of known "bad" drives */
>  		if (ide_dmaproc(ide_dma_bad_drive, drive)) {
>  			dma_func = ide_dma_off;
> @@ -458,6 +461,7 @@
>  no_dma_set:
>  		config_chipset_for_pio(drive);
>  	}
> +done:
>  	return HWIF(drive)->dmaproc(dma_func, drive);
>  }
>  
> --- linux-2.4.18-pre7/drivers/ide/serverworks.c.orig	Mon Feb  4 22:38:16 2002
> +++ linux-2.4.18-pre7/drivers/ide/serverworks.c	Mon Feb  4 23:05:07 2002
> @@ -450,7 +450,10 @@
>  	struct hd_driveid *id = drive->id;
>  	ide_dma_action_t dma_func = ide_dma_on;
>  
> -	if (id && (id->capability & 1) && HWIF(drive)->autodma) {
> +	if (id == NULL)
> +		goto done;
> +
> +	if ((id->capability & 1) && HWIF(drive)->autodma) {
>  		/* Consult the list of known "bad" drives */
>  		if (ide_dmaproc(ide_dma_bad_drive, drive)) {
>  			dma_func = ide_dma_off;
> @@ -491,6 +494,7 @@
>  no_dma_set:
>  		config_chipset_for_pio(drive);
>  	}
> +done:
>  	return HWIF(drive)->dmaproc(dma_func, drive);
>  }
>  
> --- linux-2.4.18-pre7/drivers/ide/sis5513.c.orig	Mon Feb  4 22:22:42 2002
> +++ linux-2.4.18-pre7/drivers/ide/sis5513.c	Tue Feb  5 00:59:43 2002
> @@ -506,8 +506,11 @@
>  {
>  	struct hd_driveid *id		= drive->id;
>  	ide_dma_action_t dma_func	= ide_dma_off_quietly;
> +	
> +	if (id == NULL)
> +		goto done;
>  
> -	if (id && (id->capability & 1) && HWIF(drive)->autodma) {
> +	if ((id->capability & 1) && HWIF(drive)->autodma) {
>  		/* Consult the list of known "bad" drives */
>  		if (ide_dmaproc(ide_dma_bad_drive, drive)) {
>  			dma_func = ide_dma_off;
> @@ -546,7 +549,7 @@
>  no_dma_set:
>  		(void) config_chipset_for_pio(drive, 5);
>  	}
> -
> +done:
>  	return HWIF(drive)->dmaproc(dma_func, drive);
>  }
>  
> 

Re: [PATCH] Re: 2.4.17 Oops when trying to mount ATAPI CDROM

[Dave Jones]

On Mon, Feb 04, 2002 at 10:27:42PM -0800, Andre Hedrick wrote:
 > 
 > Folks there is a bigger problem if we are able to get to loading a
 > sub-driver and we do not have a valid copy of the identify page attached
 > to the drive struct.  The resulting patch below is a nice bounds check for
 > presense of "struct hd_driveid *id = drive->id" but this should have
 > bombed out long before hand.  I do not know if the suspend to swap has
 > altered the behavor or not, but the driver has lost it knowledge about the
 > device attached.

 Suspend to swap hasn't been merged yet, so you shouldn't need
 to worry about this..

-- 
| Dave Jones.        http://www.codemonkey.org.uk
| SuSE Labs

Re: [PATCH] Re: 2.4.17 Oops when trying to mount ATAPI CDROM

[Bill Davidsen]

On Tue, 5 Feb 2002, Zwane Mwaikambo wrote:

> Patch to fix the oops as encountered by Manuel McLure
> 
> Quick browse of the trace...
> 
> <snip>
    [big snip]

> [1] In this case we get to 2 because we failed the if (id) check, but then we don't check again...
> 
> Patch diffed against 2.4.18-pre7 and applies to 2.5.3-pre6 with offsets.

I have two comments on that code, first that it is some of the ugliest
code I've seen in a while in terms of goto's, unintuitive tests, etc. And
the patch adds a goto to avoid duplicating the test for the missing id,
which really could be made more readable. If I thought a patch just to
make the code readable and maintainable would be accepted I'd write it
(while I have a flow diagram in front of me), but given the recent
discussion of path acceptable lately I won't bother. The code really is
uglier than a hedgehog's asshole, though.

MORE IMPORTANT: doesn't this imply that the device id has either been lost
or not initialized? I haven't finished grepping for calls to this code
yet, but intuitively I would guess that if we don't have the id all the
other stuff might be suspect as well.

-- 
bill davidsen <davidsen@tmr.com>
  CTO, TMR Associates, Inc
Doing interesting things with little computers since 1979.

Re: [PATCH] Re: 2.4.17 Oops when trying to mount ATAPI CDROM

[Zwane Mwaikambo]

On Tue, 5 Feb 2002, Bill Davidsen wrote:

> I have two comments on that code, first that it is some of the ugliest
> code I've seen in a while in terms of goto's, 

10: You ain't seen nothing...

> the patch adds a goto to avoid duplicating the test for the missing id,
> which really could be made more readable.

I don't see whats so unreadable about it though, its a short function with 
just a few possible code paths, (out of interest) could you post what you 
have in mind.

> (while I have a flow diagram in front of me), but given the recent
> discussion of path acceptable lately I won't bother. The code really is
> uglier than a hedgehog's asshole, though.

GOTO 10

> MORE IMPORTANT: doesn't this imply that the device id has either been lost
> or not initialized? I haven't finished grepping for calls to this code
> yet, but intuitively I would guess that if we don't have the id all the
> other stuff might be suspect as well.

Indeed, frankly i have no idea how he managed to go on in that state, but 
do note his device is horribly broken. This patch doesn't really fix his 
case, but adds a (needed) NULL pointer check.

Regards,
	Zwane Mwaikambo

Re: 2.4.17 Oops when trying to mount ATAPI CDROM

[Manuel McLure]

----- Original Message -----
From: "Manuel McLure" <manuel@mclure.org>
To: <linux-kernel@vger.kernel.org>
Cc: "Andre Hedrick" <andre@linuxdiskcert.org>
Sent: Sunday, February 03, 2002 10:32 AM
Subject: Re: 2.4.17 Oops when trying to mount ATAPI CDROM


>
> On 2002.02.03 10:21 Manuel McLure wrote:
> >
> > On 2002.02.02 22:04 Andre Hedrick wrote:
> > >
> > > Manuel,
> > >
> > > Would you be kind enough to be a little more specific on the hardware?
> > > The attached devices bu make model and real vender if known.
> > kml/
> >
> > The CD-ROM is detected as a Pioneer CD-ROM ATAPI Model DR-A24X 0104 - I
> > haven't opened the case to look at it but I do recall that it is
> > definitely a 24X Pioneer ATAPI CDROM.
> >
>
> Some more information - if I boot without "hdc=noprobe hdc=cdrom", I don't
> get the oops whel loading the "ide-cd" module - instead I get
>
> hdc: set_drive_speed_status: status=0x00 { }
> hdc: lost interrupt
> hdc: ATAPI 20X CD-ROM drive, 128kB Cache, DMA
> Uniform CD-ROM driver Revision: 3.12
> hdc: lost interrupt
> hdc: lost interrupt
> hdc: lost interrupt
>
> The module eventually finishes initializing but is not usable due to the
> "lost interrupt"s.

After checking through the code, I can't see any way that "noprobe" with an
existing device would ever work on 2.4. Unless I'm missing something the
only place where "drive->id" is allocated is in do_probe() which isn't
called for a device with "noprobe" set, yet all the config_drive_xfer_rate()
functions will oops if "drive->id" is NULL.

Is "noprobe" on an existing device no longer supported in 2.4?
--
Manuel A. McLure KE6TAW | ...for in Ulthar, according to an ancient
<manuel@mclure.org>     | and significant law, no man may kill a cat.
<http://www.mclure.org> |             -- H.P. Lovecraft

Re: 2.4.17 Oops when trying to mount ATAPI CDROM - Conclusion

[Manuel McLure]

As a conclusion to this, I followed Jens' advice and bought a new $28 
CD-ROM drive (talk about your "no-name" drive - this thing doesn't have 
any information other than "Made in China" - it's detected as a "CD-ROM 
Drive/G6D") which works just fine and even does DMA. The Pioneer 24X is 
going in the trash.

-- 
Manuel A. McLure KE6TAW | ...for in Ulthar, according to an ancient
<manuel@mclure.org>     | and significant law, no man may kill a cat.
<http://www.mclure.org> |             -- H.P. Lovecraft