2001-04-20 23:17:51

by Dan Aloni

[permalink] [raw]
Subject: cdrom driver dependency problem (and a workaround patch)


Between 2.4.3-ac9 and 2.4.3-ac10 the CDROM Uniform Driver was modified.
The modification added a dependency of the *register_cdrom() functions
on the cdrom_init() function. Theoretically, cdrom_init() should have
been called before any call to register_cdrom(). But practically, when
the CDROM driver is compiled into the kernel and not as a module, it
doesn't happen, and in 2.4.3-ac10 you get an Oops in cdrom_get_entry(),
where the NULL cdrom_numbers gets dereferenced.

One reason for this misdependency is that the IDE is initialized before
the cdrom driver, register_cdrom() gets called from inside the IDE
initialization functions. (ide_init() -> ide_init_builtin_drivers() ->
ide_cdrom_init() -> ide_cdrom_setup() -> ide_cdrom_register() ->
register_cdrom())

In order to get my kernel to boot, I've made the following temporary
workaround patch. I'd be glad to hear about other ways of solving this.


--- linux-2.4.3-ac10/drivers/cdrom/cdrom.c Fri Apr 20 23:38:37 2001
+++ linux-2.4.3-ac10/drivers/cdrom/cdrom.c Sat Apr 21 01:59:35 2001
@@ -277,6 +277,7 @@
static int autoclose=1;
static int autoeject;
static int lockdoor = 1;
+static int initialized;
/* will we ever get to use this... sigh. */
static int check_media_type;
static unsigned long *cdrom_numbers;
@@ -332,6 +333,9 @@
#ifdef CONFIG_SYSCTL
static void cdrom_sysctl_register(void);
#endif /* CONFIG_SYSCTL */
+
+static int check_cdrom_init(void);
+
static struct cdrom_device_info *topCdromPtr;
static devfs_handle_t devfs_handle;

@@ -350,6 +354,8 @@
{
int i, nr, foo;

+ printk("Holly damn. cdrom_numbers=0x%p\n",cdrom_numbers);
+
nr = 0;
foo = -1;
for (i = 0; i < CDROM_MAX_CDROMS / (sizeof(unsigned long) * 8); i++) {
@@ -368,7 +374,7 @@

static void cdrom_clear_entry(struct cdrom_device_info *cdi)
{
- int bit_nr = cdi->nr & ~(sizeof(unsigned long) * 8);
+ int bit_nr = cdi->nr % (sizeof(unsigned long) * 8);
int cd_index = cdi->nr / (sizeof(unsigned long) * 8);

clear_bit(bit_nr, &cdrom_numbers[cd_index]);
@@ -388,10 +394,14 @@
int major = MAJOR(cdi->dev);
struct cdrom_device_ops *cdo = cdi->ops;
int *change_capability = (int *)&cdo->capability; /* hack */
+ int rc;
char vname[16];

cdinfo(CD_OPEN, "entering register_cdrom\n");

+ if ((rc = check_cdrom_init()) != 0)
+ return rc;
+
if (major < 0 || major >= MAX_BLKDEV)
return -1;
if (cdo->open == NULL || cdo->release == NULL)
@@ -2706,11 +2716,29 @@

#endif /* CONFIG_SYSCTL */

-static int __init cdrom_init(void)
+static int check_cdrom_init(void)
{
- int n_entries = CDROM_MAX_CDROMS / (sizeof(unsigned long) * 8);
+ if (!initialized)
+ {
+ int n_entries;
+
+ initialized = 1;
+ n_entries = CDROM_MAX_CDROMS / (sizeof(unsigned long) * 8);

- cdrom_numbers = kmalloc(n_entries * sizeof(unsigned long), GFP_KERNEL);
+ cdrom_numbers = kmalloc(n_entries * sizeof(unsigned long), GFP_KERNEL);
+
+ if (cdrom_numbers == NULL)
+ return -ENOMEM;
+ }
+ return 0;
+}
+
+static int __init cdrom_init(void)
+{
+ int rc;
+
+ if ((rc = check_cdrom_init()) != 0)
+ return rc;

#ifdef CONFIG_SYSCTL
cdrom_sysctl_register();

--
Dan Aloni
[email protected]


2001-04-21 11:44:38

by Ingo Oeser

[permalink] [raw]
Subject: Re: cdrom driver dependency problem (and a workaround patch)

On Sat, Apr 21, 2001 at 02:17:18AM +0300, Dan Aloni wrote:
> One reason for this misdependency is that the IDE is initialized before
> the cdrom driver, register_cdrom() gets called from inside the IDE
> initialization functions. (ide_init() -> ide_init_builtin_drivers() ->
> ide_cdrom_init() -> ide_cdrom_setup() -> ide_cdrom_register() ->
> register_cdrom())
>
> In order to get my kernel to boot, I've made the following temporary
> workaround patch. I'd be glad to hear about other ways of solving this.

The link order is wrong. So why not changing the link order then?

--- Makefile.orig Sat Apr 21 12:34:34 2001
+++ Makefile Sat Apr 21 12:35:12 2001
@@ -149,15 +149,15 @@
DRIVERS-$(CONFIG_WAN) += drivers/net/wan/wan.o
DRIVERS-$(CONFIG_ARCNET) += drivers/net/arcnet/arcnetdrv.o
DRIVERS-$(CONFIG_ATM) += drivers/atm/atm.o
-DRIVERS-$(CONFIG_IDE) += drivers/ide/idedriver.o
-DRIVERS-$(CONFIG_SCSI) += drivers/scsi/scsidrv.o
-DRIVERS-$(CONFIG_FUSION_BOOT) += drivers/message/fusion/fusion.o
-DRIVERS-$(CONFIG_IEEE1394) += drivers/ieee1394/ieee1394drv.o

ifneq ($(CONFIG_CD_NO_IDESCSI)$(CONFIG_BLK_DEV_IDECD)$(CONFIG_BLK_DEV_SR)$(CONFIG_PARIDE_PCD),)
DRIVERS-y += drivers/cdrom/driver.o
endif

+DRIVERS-$(CONFIG_IDE) += drivers/ide/idedriver.o
+DRIVERS-$(CONFIG_SCSI) += drivers/scsi/scsidrv.o
+DRIVERS-$(CONFIG_FUSION_BOOT) += drivers/message/fusion/fusion.o
+DRIVERS-$(CONFIG_IEEE1394) += drivers/ieee1394/ieee1394drv.o
DRIVERS-$(CONFIG_SOUND) += drivers/sound/sounddrivers.o
DRIVERS-$(CONFIG_PCI) += drivers/pci/driver.o
DRIVERS-$(CONFIG_MTD) += drivers/mtd/mtdlink.o


Would be my idea of solving this issue.

Regards

Ingo Oeser
--
10.+11.03.2001 - 3. Chemnitzer LinuxTag <http://www.tu-chemnitz.de/linux/tag>
<<<<<<<<<<<< been there and had much fun >>>>>>>>>>>>

2001-04-21 11:46:38

by Jens Axboe

[permalink] [raw]
Subject: Re: cdrom driver dependency problem (and a workaround patch)

On Sat, Apr 21 2001, Ingo Oeser wrote:
> On Sat, Apr 21, 2001 at 02:17:18AM +0300, Dan Aloni wrote:
> > One reason for this misdependency is that the IDE is initialized before
> > the cdrom driver, register_cdrom() gets called from inside the IDE
> > initialization functions. (ide_init() -> ide_init_builtin_drivers() ->
> > ide_cdrom_init() -> ide_cdrom_setup() -> ide_cdrom_register() ->
> > register_cdrom())
> >
> > In order to get my kernel to boot, I've made the following temporary
> > workaround patch. I'd be glad to hear about other ways of solving this.
>
> The link order is wrong. So why not changing the link order then?

That's perfect, I just hadn't looked into that. The superior solution,
clearly, thanks!

--
Jens Axboe

2001-04-21 17:33:58

by Dan Aloni

[permalink] [raw]
Subject: Re: cdrom driver dependency problem (and a workaround patch)

On Sat, 21 Apr 2001, Ingo Oeser wrote:

> > In order to get my kernel to boot, I've made the following temporary
> > workaround patch. I'd be glad to hear about other ways of solving this.
>
> The link order is wrong. So why not changing the link order then?

I remember doing what the patch below does.
It didn't help.

Did you try this patch?

> --- Makefile.orig Sat Apr 21 12:34:34 2001
> +++ Makefile Sat Apr 21 12:35:12 2001
> @@ -149,15 +149,15 @@
> DRIVERS-$(CONFIG_WAN) += drivers/net/wan/wan.o
> DRIVERS-$(CONFIG_ARCNET) += drivers/net/arcnet/arcnetdrv.o
> DRIVERS-$(CONFIG_ATM) += drivers/atm/atm.o
> -DRIVERS-$(CONFIG_IDE) += drivers/ide/idedriver.o
> -DRIVERS-$(CONFIG_SCSI) += drivers/scsi/scsidrv.o
> -DRIVERS-$(CONFIG_FUSION_BOOT) += drivers/message/fusion/fusion.o
> -DRIVERS-$(CONFIG_IEEE1394) += drivers/ieee1394/ieee1394drv.o
>
> ifneq ($(CONFIG_CD_NO_IDESCSI)$(CONFIG_BLK_DEV_IDECD)$(CONFIG_BLK_DEV_SR)$(CONFIG_PARIDE_PCD),)
> DRIVERS-y += drivers/cdrom/driver.o
> endif
>
> +DRIVERS-$(CONFIG_IDE) += drivers/ide/idedriver.o
> +DRIVERS-$(CONFIG_SCSI) += drivers/scsi/scsidrv.o
> +DRIVERS-$(CONFIG_FUSION_BOOT) += drivers/message/fusion/fusion.o
> +DRIVERS-$(CONFIG_IEEE1394) += drivers/ieee1394/ieee1394drv.o
> DRIVERS-$(CONFIG_SOUND) += drivers/sound/sounddrivers.o
> DRIVERS-$(CONFIG_PCI) += drivers/pci/driver.o
> DRIVERS-$(CONFIG_MTD) += drivers/mtd/mtdlink.o
>
>
> Would be my idea of solving this issue.
>
> Regards
>
> Ingo Oeser
> --
> 10.+11.03.2001 - 3. Chemnitzer LinuxTag <http://www.tu-chemnitz.de/linux/tag>
> <<<<<<<<<<<< been there and had much fun >>>>>>>>>>>>
> -
> 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/
>

--
Dan Aloni
[email protected]

2001-04-21 19:27:25

by Ingo Oeser

[permalink] [raw]
Subject: Re: cdrom driver dependency problem (and a workaround patch)

On Sat, Apr 21, 2001 at 08:33:05PM +0300, Dan Aloni wrote:
> On Sat, 21 Apr 2001, Ingo Oeser wrote:
> > The link order is wrong. So why not changing the link order then?
>
> I remember doing what the patch below does.
> It didn't help.

Hmm, maybe you had a typo?

> Did you try this patch?

Yes, just booted an SMP machine with 2.4.3-ac11 and this patch.

I booted remote, so it was some kind of dangerous, if it wouldn't
work ;-)

We also have SCSI enabled there. So it really works ;-)


Regards

Ingo Oeser
--
10.+11.03.2001 - 3. Chemnitzer LinuxTag <http://www.tu-chemnitz.de/linux/tag>
<<<<<<<<<<<< been there and had much fun >>>>>>>>>>>>

2001-04-21 19:55:54

by Dan Aloni

[permalink] [raw]
Subject: Re: cdrom driver dependency problem (and a workaround patch)

On Sat, 21 Apr 2001, Ingo Oeser wrote:

> On Sat, Apr 21, 2001 at 08:33:05PM +0300, Dan Aloni wrote:
> > On Sat, 21 Apr 2001, Ingo Oeser wrote:
> > > The link order is wrong. So why not changing the link order then?
> >
> > I remember doing what the patch below does.
> > It didn't help.
>
> Hmm, maybe you had a typo?

No, I meant I tested this exact patch you wrote on my system and it
doesn't fix the Oops on boot problem. Maybe I forgot to recompile the
kernel while I tested it, but I doubt.

> > Did you try this patch?
>
> Yes, just booted an SMP machine with 2.4.3-ac11 and this patch.
>
> I booted remote, so it was some kind of dangerous, if it wouldn't
> work ;-)
>
> We also have SCSI enabled there. So it really works ;-)

I'm happy to hear it works on your system, but I don't think we should
relay on link ordering in order to resolve dependency problems. More
generally, it's kinda dirty the way it works now in the kernel, where the
initialization order is determined by the linkage order.

--
Dan Aloni
[email protected]