[PATCH] prevent module unloading for legacy IDE chipset drivers

[PATCH] prevent module unloading for legacy IDE chipset drivers

[Bartlomiej Zolnierkiewicz]

It is unsafe thing to do (no locking, no reference counting etc).
Just remove module_exit() as it was done for IDE PCI drivers.

 linux-2.6.6-rc1-bk5-bzolnier/drivers/ide/legacy/ali14xx.c |   18 ---------
 linux-2.6.6-rc1-bk5-bzolnier/drivers/ide/legacy/dtc2278.c |   20 ----------
 linux-2.6.6-rc1-bk5-bzolnier/drivers/ide/legacy/ht6560b.c |   24 ------------
 linux-2.6.6-rc1-bk5-bzolnier/drivers/ide/legacy/pdc4030.c |   22 -----------
 linux-2.6.6-rc1-bk5-bzolnier/drivers/ide/legacy/qd65xx.c  |   11 +----
 linux-2.6.6-rc1-bk5-bzolnier/drivers/ide/legacy/umc8672.c |   26 --------------
 6 files changed, 2 insertions(+), 119 deletions(-)

diff -puN drivers/ide/legacy/ali14xx.c~ide_legacy_mods drivers/ide/legacy/ali14xx.c
--- linux-2.6.6-rc1-bk5/drivers/ide/legacy/ali14xx.c~ide_legacy_mods	2004-04-21 17:51:19.499226456 +0200
+++ linux-2.6.6-rc1-bk5-bzolnier/drivers/ide/legacy/ali14xx.c	2004-04-21 17:51:19.531221592 +0200
@@ -243,25 +243,7 @@ int __init ali14xx_init(void)
 }
 
 #ifdef MODULE
-static void __exit ali14xx_release_hwif(ide_hwif_t *hwif)
-{
-	if (hwif->chipset != ide_ali14xx)
-		return;
-
-	hwif->chipset = ide_unknown;
-	hwif->tuneproc = NULL;
-	hwif->mate = NULL;
-	hwif->channel = 0;
-}
-
-static void __exit ali14xx_exit(void)
-{
-	ali14xx_release_hwif(&ide_hwifs[0]);
-	ali14xx_release_hwif(&ide_hwifs[1]);
-}
-
 module_init(ali14xx_init);
-module_exit(ali14xx_exit);
 #endif
 
 MODULE_AUTHOR("see local file");
diff -puN drivers/ide/legacy/dtc2278.c~ide_legacy_mods drivers/ide/legacy/dtc2278.c
--- linux-2.6.6-rc1-bk5/drivers/ide/legacy/dtc2278.c~ide_legacy_mods	2004-04-21 17:51:19.503225848 +0200
+++ linux-2.6.6-rc1-bk5-bzolnier/drivers/ide/legacy/dtc2278.c	2004-04-21 17:51:19.559217336 +0200
@@ -155,27 +155,7 @@ int __init dtc2278_init(void)
 }
 
 #ifdef MODULE
-static void __exit dtc2278_release_hwif(ide_hwif_t *hwif)
-{
-	if (hwif->chipset != ide_dtc2278)
-		return;
-
-	hwif->serialized = 0;
-	hwif->chipset = ide_unknown;
-	hwif->tuneproc = NULL;
-	hwif->drives[0].no_unmask = 0;
-	hwif->drives[1].no_unmask = 0;
-	hwif->mate = NULL;
-}
-
-static void __exit dtc2278_exit(void)
-{
-	dtc2278_release_hwif(&ide_hwifs[0]);
-	dtc2278_release_hwif(&ide_hwifs[1]);
-}
-
 module_init(dtc2278_init);
-module_exit(dtc2278_exit);
 #endif
 
 MODULE_AUTHOR("See Local File");
diff -puN drivers/ide/legacy/ht6560b.c~ide_legacy_mods drivers/ide/legacy/ht6560b.c
--- linux-2.6.6-rc1-bk5/drivers/ide/legacy/ht6560b.c~ide_legacy_mods	2004-04-21 17:51:19.506225392 +0200
+++ linux-2.6.6-rc1-bk5-bzolnier/drivers/ide/legacy/ht6560b.c	2004-04-21 17:51:19.589212776 +0200
@@ -360,31 +360,7 @@ release_region:
 }
 
 #ifdef MODULE
-static void __exit ht6560b_release_hwif(ide_hwif_t *hwif)
-{
-	if (hwif->chipset != ide_ht6560b)
-		return;
-
-	hwif->chipset = ide_unknown;
-	hwif->tuneproc = NULL;
-	hwif->selectproc = NULL;
-	hwif->serialized = 0;
-	hwif->mate = NULL;
-	hwif->channel = 0;
-
-	hwif->drives[0].drive_data = 0;
-	hwif->drives[1].drive_data = 0;
-}
-
-static void __exit ht6560b_exit(void)
-{
-	ht6560b_release_hwif(&ide_hwifs[0]);
-	ht6560b_release_hwif(&ide_hwifs[1]);
-	release_region(HT_CONFIG_PORT, 1);
-}
-
 module_init(ht6560b_init);
-module_exit(ht6560b_exit);
 #endif
 
 MODULE_AUTHOR("See Local File");
diff -puN drivers/ide/legacy/pdc4030.c~ide_legacy_mods drivers/ide/legacy/pdc4030.c
--- linux-2.6.6-rc1-bk5/drivers/ide/legacy/pdc4030.c~ide_legacy_mods	2004-04-21 17:51:19.511224632 +0200
+++ linux-2.6.6-rc1-bk5-bzolnier/drivers/ide/legacy/pdc4030.c	2004-04-21 17:51:19.597211560 +0200
@@ -312,29 +312,7 @@ int __init pdc4030_init(void)
 }
 
 #ifdef MODULE
-static void __exit pdc4030_release_hwif(ide_hwif_t *hwif)
-{
-	hwif->chipset = ide_unknown;
-	hwif->selectproc = NULL;
-	hwif->serialized = 0;
-	hwif->drives[0].io_32bit = 0;
-	hwif->drives[1].io_32bit = 0;
-	hwif->drives[0].keep_settings = 0;
-	hwif->drives[1].keep_settings = 0;
-	hwif->drives[0].noprobe = 0;
-	hwif->drives[1].noprobe = 0;
-}
-
-static void __exit pdc4030_exit(void)
-{
-	unsigned int index;
-
-	for (index = 0; index < MAX_HWIFS; index++)
-		pdc4030_release_hwif(&ide_hwifs[index]);
-}
-
 module_init(pdc4030_init);
-module_exit(pdc4030_exit);
 #endif
 
 MODULE_AUTHOR("Peter Denison");
diff -puN drivers/ide/legacy/qd65xx.c~ide_legacy_mods drivers/ide/legacy/qd65xx.c
--- linux-2.6.6-rc1-bk5/drivers/ide/legacy/qd65xx.c~ide_legacy_mods	2004-04-21 17:51:19.515224024 +0200
+++ linux-2.6.6-rc1-bk5-bzolnier/drivers/ide/legacy/qd65xx.c	2004-04-21 18:20:32.992654936 +0200
@@ -354,12 +354,12 @@ static void __init qd_setup(ide_hwif_t *
 	probe_hwif_init(hwif);
 }
 
-#ifdef MODULE
 /*
  * qd_unsetup:
  *
  * called to unsetup an ata channel : back to default values, unlinks tuning
  */
+/*
 static void __exit qd_unsetup(ide_hwif_t *hwif)
 {
 	u8 config = hwif->config_data;
@@ -389,7 +389,7 @@ static void __exit qd_unsetup(ide_hwif_t
 		printk(KERN_WARNING "keeping settings !\n");
 	}
 }
-#endif
+*/
 
 /*
  * qd_probe:
@@ -496,14 +496,7 @@ int __init qd65xx_init(void)
 }
 
 #ifdef MODULE
-static void __exit qd65xx_exit(void)
-{
-	qd_unsetup(&ide_hwifs[0]);
-	qd_unsetup(&ide_hwifs[1]);
-}
-
 module_init(qd65xx_init);
-module_exit(qd65xx_exit);
 #endif
 
 MODULE_AUTHOR("Samuel Thibault");
diff -puN drivers/ide/legacy/umc8672.c~ide_legacy_mods drivers/ide/legacy/umc8672.c
--- linux-2.6.6-rc1-bk5/drivers/ide/legacy/umc8672.c~ide_legacy_mods	2004-04-21 17:51:19.519223416 +0200
+++ linux-2.6.6-rc1-bk5-bzolnier/drivers/ide/legacy/umc8672.c	2004-04-21 17:51:19.600211104 +0200
@@ -173,33 +173,7 @@ int __init umc8672_init(void)
 }
 
 #ifdef MODULE
-static void __exit umc8672_release_hwif(ide_hwif_t *hwif)
-{
-	if (hwif->chipset != ide_umc8672)
-		return;
-
-	hwif->chipset = ide_unknown;
-	hwif->tuneproc = NULL;
-	hwif->mate = NULL;
-	hwif->channel = 0;
-}
-
-static void __exit umc8672_exit(void)
-{
-	unsigned long flags;
-
-	umc8672_release_hwif(&ide_hwifs[0]);
-	umc8672_release_hwif(&ide_hwifs[1]);
-
-	local_irq_save(flags);
-	outb_p(0xa5, 0x108);	/* disable umc */
-	local_irq_restore(flags);
-
-	release_region(0x108, 2);
-}
-
 module_init(umc8672_init);
-module_exit(umc8672_exit);
 #endif
 
 MODULE_AUTHOR("Wolfram Podien");

_

Re: [PATCH] prevent module unloading for legacy IDE chipset drivers

[Erik Andersen]

On Wed Apr 21, 2004 at 10:19:24PM +0200, Bartlomiej Zolnierkiewicz wrote:
> 
> It is unsafe thing to do (no locking, no reference counting etc).
> Just remove module_exit() as it was done for IDE PCI drivers.

Out of curiosity, what would be needed to make it safe to unload
all ide modules from a system with a scsi rootfs?

 -Erik

--
Erik B. Andersen             http://codepoet-consulting.com/
--This message was written using 73% post-consumer electrons--

Re: [PATCH] prevent module unloading for legacy IDE chipset drivers

[Bartlomiej Zolnierkiewicz]

On Thursday 22 of April 2004 02:41, Erik Andersen wrote:
> On Wed Apr 21, 2004 at 10:19:24PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > It is unsafe thing to do (no locking, no reference counting etc).
> > Just remove module_exit() as it was done for IDE PCI drivers.
>
> Out of curiosity, what would be needed to make it safe to unload
> all ide modules from a system with a scsi rootfs?

It doesn't matter - you still may end up unloading modules which are in use.
Plus unregistering IDE interfaces leave mess in _static_ ide_hwifs[] table.

Re: [PATCH] prevent module unloading for legacy IDE chipset drivers

[Erik Mouw]

On Thu, Apr 22, 2004 at 02:50:15AM +0200, Bartlomiej Zolnierkiewicz wrote:
> On Thursday 22 of April 2004 02:41, Erik Andersen wrote:
> > Out of curiosity, what would be needed to make it safe to unload
> > all ide modules from a system with a scsi rootfs?
> 
> It doesn't matter - you still may end up unloading modules which are in use.

FWIW, with the old IDE code I've been unloading IDE modules for years
without a single problem.

What makes IDE sufficiently different from SCSI that we can't unload
IDE host drivers?


Erik

-- 
+-- Erik Mouw -- www.harddisk-recovery.com -- +31 70 370 12 90 --
| Lab address: Delftechpark 26, 2628 XH, Delft, The Netherlands

Re: [PATCH] prevent module unloading for legacy IDE chipset drivers

[Bartlomiej Zolnierkiewicz]

On Thursday 22 of April 2004 12:33, Erik Mouw wrote:
> On Thu, Apr 22, 2004 at 02:50:15AM +0200, Bartlomiej Zolnierkiewicz wrote:
> > On Thursday 22 of April 2004 02:41, Erik Andersen wrote:
> > > Out of curiosity, what would be needed to make it safe to unload
> > > all ide modules from a system with a scsi rootfs?
> >
> > It doesn't matter - you still may end up unloading modules which are in
> > use.
>
> FWIW, with the old IDE code I've been unloading IDE modules for years
> without a single problem.

IDE chipset drivers were made 'modular' in 2.4.21
(release date 13-Jun-2003) and this complicated things

> What makes IDE sufficiently different from SCSI that we can't unload
> IDE host drivers?

- no reference counting
- lack of release() method
- insufficient locking

Cheers,
Bartlomiej

Re: [PATCH] prevent module unloading for legacy IDE chipset drivers

[Erik Mouw]

On Thu, Apr 22, 2004 at 04:35:12PM +0200, Bartlomiej Zolnierkiewicz wrote:
> On Thursday 22 of April 2004 12:33, Erik Mouw wrote:
> > What makes IDE sufficiently different from SCSI that we can't unload
> > IDE host drivers?
> 
> - no reference counting
> - lack of release() method
> - insufficient locking

Do you plan to fix the module unloading in the current code, or is it
easier to write a new driver based on libata (assuming it has been
fixed in libata)? If I understood Jeff's latest libata update
correctly, it should be possible Real Soon Now [tm], right?


Erik

-- 
+-- Erik Mouw -- www.harddisk-recovery.com -- +31 70 370 12 90 --
| Lab address: Delftechpark 26, 2628 XH, Delft, The Netherlands

Re: [PATCH] prevent module unloading for legacy IDE chipset drivers

[Bartlomiej Zolnierkiewicz]

On Monday 26 of April 2004 15:50, Erik Mouw wrote:
> On Thu, Apr 22, 2004 at 04:35:12PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > On Thursday 22 of April 2004 12:33, Erik Mouw wrote:
> > > What makes IDE sufficiently different from SCSI that we can't unload
> > > IDE host drivers?
> >
> > - no reference counting
> > - lack of release() method
> > - insufficient locking
>
> Do you plan to fix the module unloading in the current code, or is it
> easier to write a new driver based on libata (assuming it has been
> fixed in libata)? If I understood Jeff's latest libata update

I'm going to fix it but doing it properly requires major changes in IDE
code (ie. get rid of static &ide_hwifs[]) but it's happening slowly.

> correctly, it should be possible Real Soon Now [tm], right?

BTW I think there is a common misunderstanding about libata:
    it will not replace IDE drivers any time soon.

I want to rewrite+merge current IDE code with libata during 2.7
(and yes, legacy naming and ordering will be preserved!).

I hope nobody starts rewriting existing IDE drivers for libata and pushing
them upstream -> it will mean maintenance problems much bigger than OSS+ALSA.

However writing _new_ libata driver for 'exotic' PATA hardware is OK.

Cheers,
Bartlomiej

Re: [PATCH] prevent module unloading for legacy IDE chipset drivers

[Bartlomiej Zolnierkiewicz]

On Monday 26 of April 2004 16:50, Bartlomiej Zolnierkiewicz wrote:

> I want to rewrite+merge current IDE code with libata during 2.7
> (and yes, legacy naming and ordering will be preserved!).

as a config option not unconditionally

Re: [PATCH] prevent module unloading for legacy IDE chipset drivers

[Erik Mouw]

On Mon, Apr 26, 2004 at 04:50:40PM +0200, Bartlomiej Zolnierkiewicz wrote:
> BTW I think there is a common misunderstanding about libata:
>     it will not replace IDE drivers any time soon.
> 
> I want to rewrite+merge current IDE code with libata during 2.7
> (and yes, legacy naming and ordering will be preserved!).
> 
> I hope nobody starts rewriting existing IDE drivers for libata and pushing
> them upstream -> it will mean maintenance problems much bigger than OSS+ALSA.

I don't think it's bad to have two drivers for the same hardware. We've
seen that with the USB UHCI host controller, RTL 8139, Intel e100,
Adaptec aic7xxx, NCR/Symbios sym53c8x. Having two drivers makes it easy
for people to switch. The difference with OSS+ALSA is that the drivers
I just mentioned have been "developed" in the tree, while ALSA was
developed outside the tree.

> However writing _new_ libata driver for 'exotic' PATA hardware is OK.

Is AMD 760/762 (amd74xx driver) considered "exotic"? ;-)


Erik

-- 
+-- Erik Mouw -- www.harddisk-recovery.com -- +31 70 370 12 90 --
| Lab address: Delftechpark 26, 2628 XH, Delft, The Netherlands

Re: [PATCH] prevent module unloading for legacy IDE chipset drivers

[Bartlomiej Zolnierkiewicz]

On Monday 26 of April 2004 17:23, Erik Mouw wrote:
> On Mon, Apr 26, 2004 at 04:50:40PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > BTW I think there is a common misunderstanding about libata:
> >     it will not replace IDE drivers any time soon.
> >
> > I want to rewrite+merge current IDE code with libata during 2.7
> > (and yes, legacy naming and ordering will be preserved!).
> >
> > I hope nobody starts rewriting existing IDE drivers for libata and
> > pushing them upstream -> it will mean maintenance problems much bigger
> > than OSS+ALSA.
>
> I don't think it's bad to have two drivers for the same hardware. We've
> seen that with the USB UHCI host controller, RTL 8139, Intel e100,

USB and networks drivers are _actively_ developed by _many_ people
(in comparison to IDE).

> Adaptec aic7xxx, NCR/Symbios sym53c8x. Having two drivers makes it easy
> for people to switch. The difference with OSS+ALSA is that the drivers

It is much easier to pull out network driver from kernel than IDE driver
and some people just won't switch until old driver works.

Just think how much effort it took to make people use ide_cd not ide_scsi
for CD writing and it's nothing compared to make them switch from
(ordered) /dev/hdX to (random) /dev/sdX.

Please remember about device ordering issues and hardcoded 'root=/'.

Not to even mention problems like that SCSI emulation won't some support
some devices supported by ide_floppy and ide_cd etc.

> I just mentioned have been "developed" in the tree, while ALSA was
> developed outside the tree.

No, the biggest difference is that subsystem core was also changed in ALSA.
-> OSS emulation in ALSA etc.

> > However writing _new_ libata driver for 'exotic' PATA hardware is OK.
>
> Is AMD 760/762 (amd74xx driver) considered "exotic"? ;-)

NO! 8)

The main problem is that even with well tested amd74xx driver for libata
it won't be possible to remove old amd74xx driver so we'll have to maintain
both for really long time and it won't be funny et all.

Bartlomiej

Re: [PATCH] prevent module unloading for legacy IDE chipset drivers

[Rogier Wolff]

On Thu, Apr 22, 2004 at 12:33:55PM +0200, Erik Mouw wrote:
> On Thu, Apr 22, 2004 at 02:50:15AM +0200, Bartlomiej Zolnierkiewicz wrote:
> > On Thursday 22 of April 2004 02:41, Erik Andersen wrote:
> > > Out of curiosity, what would be needed to make it safe to unload
> > > all ide modules from a system with a scsi rootfs?
> > 
> > It doesn't matter - you still may end up unloading modules which are in use.
> 
> FWIW, with the old IDE code I've been unloading IDE modules for years
> without a single problem.

Hmm. Not completely true: It crashes on our VIA EPIA board. Would 
anybody have an idea why it works on most systems, but crashes
hard on the VIA EPIA? (I'm not sure wether it crashes the hardware
or the software). 

			Roger. 

-- 
+-- Rogier Wolff -- www.harddisk-recovery.nl -- 0800 220 20 20 --
| Files foetsie, bestanden kwijt, alle data weg?!
| Blijf kalm en neem contact op met Harddisk-recovery.nl!