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
<[email protected]> | and significant law, no man may kill a cat.
<http://www.mclure.org> | -- H.P. Lovecraft
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
> <[email protected]> | 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
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.
-
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
<[email protected]> | and significant law, no man may kill a cat.
<http://www.mclure.org> | -- H.P. Lovecraft
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
<[email protected]> | and significant law, no man may kill a cat.
<http://www.mclure.org> | -- H.P. Lovecraft
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
----- Original Message -----
From: "Manuel McLure" <[email protected]>
To: <[email protected]>
Cc: "Andre Hedrick" <[email protected]>
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
<[email protected]> | and significant law, no man may kill a cat.
<http://www.mclure.org> | -- H.P. Lovecraft
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
<[email protected]> | and significant law, no man may kill a cat.
<http://www.mclure.org> | -- H.P. Lovecraft
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);
}
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);
> }
>
>
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
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 <[email protected]>
CTO, TMR Associates, Inc
Doing interesting things with little computers since 1979.
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