Skip to contentSkip to conversation

buggy scsi_register behaviour in 2.5.51

DISCUSSION Douglas Gilbert 3 participants 6 messages Dec 17, 2002 – Dec 20, 2002 SCSI

lore
Share & tools
lore

Douglas Gilbert

author
#0 lore raw

Willem,
Can't fix all those problems but I did notice that idescsi_release() was missing a call to scsi_unregister(). With this patch the ide-scsi entry remains dangling in sysfs (ide problem??) but the scsi host gets cleaned up after rmmod.

The attachment is against lk 2.5.52 . The ide-scsi driver in lk 2.5.52 needs cleaning up in several areas. If no-one else wants to attack it then I will.

Doug Gilbert

Quoted 116 lines · Willem Riede

I was investigating why I get

Dec 14 14:50:55 fallguy kernel: Badness in kobject_register at lib/kobject.c:113
Dec 14 14:50:55 fallguy kernel: Call Trace:
Dec 14 14:50:55 fallguy kernel:  [<c01c7c88>] kobject_register+0x58/0x70
Dec 14 14:50:55 fallguy kernel:  [<c01db442>] bus_add_driver+0x82/0xd0
Dec 14 14:50:55 fallguy kernel:  [<c022bd00>] ide_drive_remove+0x0/0x30
Dec 14 14:50:55 fallguy kernel:  [<c022bdec>] ide_register_driver+0xbc/0xf0
Dec 14 14:50:55 fallguy kernel:  [<c0133f6a>] sys_init_module+0x1ba/0x1d0
Dec 14 14:50:55 fallguy kernel:  [<c010b643>] syscall_call+0x7/0xb

when re-insmod-ing ide-scsi, and the "badness" is in sysfs create_dir, appearently because the ide-scsi directory is not removed when ide-scsi is unloaded so the create fails because it already exists.

But that's not the only thing going wrong when repeatedly (un)loading. See below for some really horrible effects:

[root@fallguy linux-2.5.51]# insmod drivers/scsi/ide-scsi.ko [root@fallguy linux-2.5.51]# cat /proc/scsi/scsi Attached devices:
Host: scsi0 Channel: 00 Id: 03 Lun: 00
Vendor: OnStream Model: SC-30 Rev: 1.09
Type: Sequential-Access ANSI SCSI revision: 02
Host: scsi0 Channel: 00 Id: 06 Lun: 00
Vendor: MATSHITA Model: CD-R CW-7502 Rev: 4.17
Type: CD-ROM ANSI SCSI revision: 02
Host: scsi1 Channel: 00 Id: 00 Lun: 00
Vendor: IBM Model: DNES-318350W Rev: SA30
Type: Direct-Access ANSI SCSI revision: 03
Host: scsi1 Channel: 00 Id: 01 Lun: 00
Vendor: IBM Model: DGHS09U Rev: 0350
Type: Direct-Access ANSI SCSI revision: 03
Host: scsi3 Channel: 00 Id: 00 Lun: 00
Vendor: CONNER Model: CTT8000-A Rev: 2.08
Type: Sequential-Access ANSI SCSI revision: 02
Host: scsi3 Channel: 00 Id: 01 Lun: 00
Vendor: OnStream Model: DI-30 Rev: 1.09
Type: Sequential-Access ANSI SCSI revision: 02
Host: scsi3 Channel: 00 Id: 02 Lun: 00
Vendor: OnStream Model: ADR Series Rev: 5.05
Type: Sequential-Access ANSI SCSI revision: 02
[root@fallguy linux-2.5.51]# ls -l /sysfs/bus/ide/drivers/ide-scsi total 0
[root@fallguy linux-2.5.51]# ls -ld /sysfs/bus/ide/drivers/ide-scsi
drwxr-xr-x 2 root root 0 Dec 15 15:54 /sysfs/bus/ide/drivers/ide-scsi
[root@fallguy linux-2.5.51]# rmmod ide_scsi [root@fallguy linux-2.5.51]# ls -ld /sysfs/bus/ide/drivers/ide-scsi
drwxr-xr-x 2 root root 0 Dec 15 15:54 /sysfs/bus/ide/drivers/ide-scsi

^^ the directory is still there ! ^^

[root@fallguy linux-2.5.51]# cat /proc/scsi/scsi Attached devices:
Host: scsi0 Channel: 00 Id: 03 Lun: 00
Vendor: OnStream Model: SC-30 Rev: 1.09
Type: Sequential-Access ANSI SCSI revision: 02
Host: scsi0 Channel: 00 Id: 06 Lun: 00
Vendor: MATSHITA Model: CD-R CW-7502 Rev: 4.17
Type: CD-ROM ANSI SCSI revision: 02
Host: scsi1 Channel: 00 Id: 00 Lun: 00
Vendor: IBM Model: DNES-318350W Rev: SA30
Type: Direct-Access ANSI SCSI revision: 03
Host: scsi1 Channel: 00 Id: 01 Lun: 00
Vendor: IBM Model: DGHS09U Rev: 0350
Type: Direct-Access ANSI SCSI revision: 03
[root@fallguy linux-2.5.51]# insmod drivers/scsi/ide-scsi.ko [root@fallguy linux-2.5.51]# ls -ld /sysfs/bus/ide/drivers/ide-scsi
drwxr-xr-x 2 root root 0 Dec 15 15:54 /sysfs/bus/ide/drivers/ide-scsi
[root@fallguy linux-2.5.51]# cat /proc/scsi/scsi Attached devices:
Host: scsi0 Channel: 00 Id: 03 Lun: 00
Vendor: OnStream Model: SC-30 Rev: 1.09
Type: Sequential-Access ANSI SCSI revision: 02
Host: scsi0 Channel: 00 Id: 06 Lun: 00
Vendor: MATSHITA Model: CD-R CW-7502 Rev: 4.17
Type: CD-ROM ANSI SCSI revision: 02
Host: scsi1 Channel: 00 Id: 00 Lun: 00
Vendor: IBM Model: DNES-318350W Rev: SA30
Type: Direct-Access ANSI SCSI revision: 03
Host: scsi1 Channel: 00 Id: 01 Lun: 00
Vendor: IBM Model: DGHS09U Rev: 0350
Type: Direct-Access ANSI SCSI revision: 03
Host: scsi3 Channel: 00 Id: 00 Lun: 00
Vendor: OnStream Model: ADR Series Rev: 5.05
Type: Sequential-Access ANSI SCSI revision: 02
Host: scsi3 Channel: 00 Id: 01 Lun: 00
Vendor: OnStream Model: DI-30 Rev: 1.09
Type: Sequential-Access ANSI SCSI revision: 02
Host: scsi3 Channel: 00 Id: 02 Lun: 00
Vendor: CONNER Model: CTT8000-A Rev: 2.08
Type: Sequential-Access ANSI SCSI revision: 02
Host: scsi4 Channel: 00 Id: 00 Lun: 00
Vendor: OnStream Model: ADR Series Rev: 5.05
Type: Sequential-Access ANSI SCSI revision: 02
Host: scsi4 Channel: 00 Id: 01 Lun: 00
Vendor: OnStream Model: DI-30 Rev: 1.09
Type: Sequential-Access ANSI SCSI revision: 02
Host: scsi4 Channel: 00 Id: 02 Lun: 00
Vendor: CONNER Model: CTT8000-A Rev: 2.08
Type: Sequential-Access ANSI SCSI revision: 02

ide-scsi has now been assigned both scsi3 and scsi4?? Its devices appear twice!

Clearly not correct. I'll work on fixing this, but first I have a question.

Reading the source (hosts.c), it appears that a host adapter will only get the same number as it had before, if it has been specified in the module parameter "scsihosts".

Is it a concious design decision to not try and re-assign the same number as before if an adapter not in "scsihosts" gets (un)loaded repeatedly?

I think a fix for this would be different if I would try to reassign the same number than if the intention is to keep assigning new numbers.

Quoted 7 lines

Thanks for your advise. Willem Riede.

-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

drivers/scsi/ide-scsi.c2552x renamed from drivers/scsi/ide-scsi.c +1 -0
@@ -658,6 +658,7 @@
 		if (drive)
 			DRIVER(drive)->busy--;
 	}
+	scsi_unregister(host);
 	return 0;
 }
#1 lore raw
Quoted 7 lines · On 2002.12.16 22:27 Douglas Gilbert wrote:

Willem,
Can't fix all those problems but I did notice that idescsi_release() was missing a call to scsi_unregister(). With this patch the ide-scsi entry remains dangling in sysfs (ide problem??) but the scsi host gets cleaned up after rmmod.

Quoted 5 lines

The attachment is against lk 2.5.52 . The ide-scsi driver in lk 2.5.52 needs cleaning up in several areas. If no-one else wants to attack it then I will.

Thanks Doug.

I'll also see if I can find what the remaining problems are. I'd also be prepared do do "cleaning up" of ide-scsi -- what in your opinion needs taking care of?

Regards, Willem Riede.

#5 lore raw
In reply to Douglas Gilbert
Quoted 5 lines · On 2002.12.16 22:27 Douglas Gilbert wrote:

Willem,
Can't fix all those problems but I did notice that idescsi_release() was missing a call to scsi_unregister(). With this patch the ide-scsi entry remains dangling in sysfs (ide problem??)

Indeed. There is a call to driver_register in ide_register_driver but no call to driver_unregister in ide_unregister_driver.

I put one in, and that takes care of it. PATCH to ide.c attached.

Regards, Willem Riede.

ide.c renamed from ide.c-l2552 +2 -0
@@ -2314,6 +2314,8 @@
 {
 	ide_drive_t *drive;
 
+	driver_unregister(&driver->gen_driver);
+
 	spin_lock(&drivers_lock);
 	list_del(&driver->drivers);
 	spin_unlock(&drivers_lock);
#2 lore raw
In reply to Willem Riede
Quoted 5 lines · Willem Riede

<snip/>

I'll also see if I can find what the remaining problems are. I'd also be prepared do do "cleaning up" of ide-scsi -- what in your opinion needs taking care of?

Willem,
ide-scsi in lk 2.5 ...
a) needs .eh_bus_reset_handler defined in idescsi_template and implemented. It probably would be a dummy unless there is a clean way to do an ATAPI bus (or device) reset.
b) replace .detect and .release in idescsi_template with the slave_alloc, slave_configure and slave_destroy interface

These are documented in Documentation/scsi/scsi_mid_low_api.txt . If you find mistakes or the explanation is not clear then complain to me. For an example of the use of these new interfaces see scsi_debug.c or aic7xxx_old.c

Good to see that you have fixed the ide sysfs de-registration problem.

Doug Gilbert

#3 lore raw
Quoted 8 lines · On 2002.12.18 07:14 Douglas Gilbert wrote:

Willem,
ide-scsi in lk 2.5 ...
a) needs .eh_bus_reset_handler defined in idescsi_template and implemented. It probably would be a dummy unless there is a clean way to do an ATAPI bus (or device) reset.
b) replace .detect and .release in idescsi_template with the slave_alloc, slave_configure and slave_destroy interface

And do request_sense itself as prescibed by the "new" error handling I assume.

I'll be back with code when I have it.

Thanks, Willem Riede.

#4 lore raw
In reply to Douglas Gilbert
Quoted 4 lines · On 2002.12.18 07:14 Douglas Gilbert wrote:

These are documented in Documentation/scsi/scsi_mid_low_api.txt . If you find mistakes or the explanation is not clear then complain to me.

Your doc doesn't define the second parameter to scsi_add_host. Can you explain what the purpose is of the struct device *dev and how I'm supposed to pull one out of my hat in __init? I notice that the old style registration doesn't bother with shost->host_gendev.

Thanks, Willem Riede.