2.4.21-pre1 broke the ide-tape driver

DISCUSSION Mikael Pettersson 2 participants 2 messages Dec 16, 2002 – Dec 19, 2002

Share & tools

Mikael Pettersson

author

2002-12-16 23:23 UTC #0 lore raw

Quoted 11 lines · I wrote:

Quoted 6 lines · Marc-Christian Petersen
Quoted 5 lines
Kernel 2.4.21-pre1 broke the ide-tape driver: the driver now hangs during initialisation. 2.2 kernels (with Andre's IDE patch) and 2.4 up to 2.4.20 do not have this problem. My box has a Seagate STT8000A ATAPI tape drive as hdd; hdc is a Philips CD-RW, and the controller is ICH2 (i850 chipset).
http://linux.bkbits.net:8080/linux-2.4/[email protected]?nav=index.html|ChangeSet@-7d|[email protected]
Addendum: this patch fixes the init-time hang, and ide-tape does seem to work fine, but 'rmmod ide-tape' oopses -- 2.4.20-ac2 also oopses on 'rmmod ide-tape'.

Solved, I think. Observe ide-tape's module_exit() procedure idetape_exit():

Quoted 14 lines

static void __exit idetape_exit (void)

{
	ide_drive_t *drive;
	int minor;

	for (minor = 0; minor < MAX_HWIFS * MAX_DRIVES; minor++) {
		drive = idetape_chrdevs[minor].drive;
		if (drive != NULL && idetape_cleanup(drive))
			printk(KERN_ERR "ide-tape: %s: cleanup_module() "
				"called while still busy\n", drive->name);
	}
#ifdef CONFIG_PROC_FS
	if (drive->proc)
		ide_remove_proc_entries(drive->proc, idetape_proc);

Quoted 4 lines

#endif

	ide_unregister_module(&idetape_module);
}

In the "if (drive->proc)" line, drive==NULL when I rmmod ide-tape, causing the oops.

I'm not sure if ide_remove_proc_entries() is needed or not, but the current code is obviously broken.

- ide_unregister_module() removes ide-tape's proc entry (/proc/ide/ideX/hdY/name) for us, at least that's what happens on my box after I commented out the entire "if (drive->proc) ..." statement to prevent the oops. So possibly the call should be deleted.

- ide-disk/ide-floppy do the test&call inside the loop rather than after, so possibly the call should be moved into the loop, and augmented to be "if (drive && drive->proc) ide_remove_proc_entries(...)".

/Mikael

Marcelo Tosatti

2002-12-19 22:58 UTC #1 lore raw

Quoted 40 lines · Mikael Pettersson

Quoted 11 lines · I wrote:
Quoted 6 lines · Marc-Christian Petersen
Quoted 5 lines
Kernel 2.4.21-pre1 broke the ide-tape driver: the driver now hangs during initialisation. 2.2 kernels (with Andre's IDE patch) and 2.4 up to 2.4.20 do not have this problem. My box has a Seagate STT8000A ATAPI tape drive as hdd; hdc is a Philips CD-RW, and the controller is ICH2 (i850 chipset).
http://linux.bkbits.net:8080/linux-2.4/[email protected]?nav=index.html|ChangeSet@-7d|[email protected]
Addendum: this patch fixes the init-time hang, and ide-tape does seem to work fine, but 'rmmod ide-tape' oopses -- 2.4.20-ac2 also oopses on 'rmmod ide-tape'.
Solved, I think. Observe ide-tape's module_exit() procedure idetape_exit():
Quoted 14 lines
static void __exit idetape_exit (void)
{
	ide_drive_t *drive;
	int minor;

	for (minor = 0; minor < MAX_HWIFS * MAX_DRIVES; minor++) {
		drive = idetape_chrdevs[minor].drive;
		if (drive != NULL && idetape_cleanup(drive))
			printk(KERN_ERR "ide-tape: %s: cleanup_module() "
				"called while still busy\n", drive->name);
	}
#ifdef CONFIG_PROC_FS
	if (drive->proc)
		ide_remove_proc_entries(drive->proc, idetape_proc);
Quoted 4 lines
#endif
	ide_unregister_module(&idetape_module);
}
In the "if (drive->proc)" line, drive==NULL when I rmmod ide-tape, causing the oops.
I'm not sure if ide_remove_proc_entries() is needed or not, but the current code is obviously broken.

Quoted 8 lines

- ide_unregister_module() removes ide-tape's proc entry (/proc/ide/ideX/hdY/name) for us, at least that's what happens on my box after I commented out the entire "if (drive->proc) ..." statement to prevent the oops. So possibly the call should be deleted.
- ide-disk/ide-floppy do the test&call inside the loop rather than after, so possibly the call should be moved into the loop, and augmented to be "if (drive && drive->proc) ide_remove_proc_entries(...)".

Yes... here is a patch which moves the ide_remove_proc_entries inside the detection loop. Without ide_remove_proc_entries inside the loop we would also not unregister more than one device /proc entries, too.

Please test it, works for me.

drivers/ide/ide-tape.c +8 -6

@@ -6597,14 +6597,16 @@
 
 	for (minor = 0; minor < MAX_HWIFS * MAX_DRIVES; minor++) {
 		drive = idetape_chrdevs[minor].drive;
-		if (drive != NULL && idetape_cleanup(drive))
-			printk(KERN_ERR "ide-tape: %s: cleanup_module() "
-				"called while still busy\n", drive->name);
-	}
+		if (drive) {
+			if (idetape_cleanup(drive))
+				printk(KERN_ERR "ide-tape: %s: cleanup_module() "
+					"called while still busy\n", drive->name);
 #ifdef CONFIG_PROC_FS
-	if (drive->proc)
-		ide_remove_proc_entries(drive->proc, idetape_proc);
+			if (drive->proc)
+				ide_remove_proc_entries(drive->proc, idetape_proc);
 #endif
+		}
+	}
 
 	ide_unregister_module(&idetape_module);
 }