Re: [PATCH] scsi/hosts.c device_register fix

Re: [PATCH] scsi/hosts.c device_register fix

[Andries.Brouwer]

	From James.Bottomley@steeleye.com  Thu Nov 28 17:57:52 2002

	Actually, the patch is wrong.
	The device_register has to be done in scsi_add_host,
	The correct fix is to move the corresponding device_unregister
	into scsi_remove_host so that they match.

Very good. That was what I had done first, but a google search
turned up your patch and I thought that it was also OK.

	I'll also commit it to the scsi-misc-2.5 BK tree.

Hope to see it in 2.5.51.

Andries

Re: [PATCH] scsi/hosts.c device_register fix

[Douglas Gilbert]

[-- Attachment #1: Type: text/plain, Size: 1098 bytes --]

James Bottomley wrote:
> Actually, the patch is wrong.  It will wreak havoc with SCSI's use of sysfs.  
> The device_register has to be done in scsi_add_host, which is called after all 
> the driver specific sysfs setup has been done.  The correct fix is to move the 
> corresponding device_unregister into scsi_remove_host so that they match.
> 
> I've attached it below.  I'll also commit it to the scsi-misc-2.5 BK tree.
> 
> James
> 
> 
> 
> ------------------------------------------------------------------------
> 
> ===== hosts.c 1.31 vs edited =====
> --- 1.31/drivers/scsi/hosts.c	Sun Nov 17 15:47:02 2002
> +++ edited/hosts.c	Sat Nov 23 17:25:57 2002
> @@ -295,6 +295,8 @@
>  		kfree(sdev);
>  	}
>  
> +	device_unregister(&shost->host_driverfs_dev);
> +
>  	return 0;
>  }
>  
> @@ -348,7 +350,6 @@
>  
>  	/* Cleanup proc and driverfs */
>  	scsi_proc_host_rm(shost);
> -	device_unregister(&shost->host_driverfs_dev);
>  
>  	kfree(shost);
>  }

James,
The above patch is a bit noisy when applied to lk 2.5.50 .
Attached is the same patch (I hope) which applies cleanly.

Doug Gilbert

[-- Attachment #2: dev_reg_2550.diff --]
[-- Type: text/plain, Size: 444 bytes --]

--- linux/drivers/scsi/hosts.c	2002-11-29 09:27:35.000000000 +1100
+++ linux/drivers/scsi/hosts.c2550jb	2002-11-29 09:45:28.000000000 +1100
@@ -297,6 +297,8 @@
 		scsi_free_sdev(list_entry(le, Scsi_Device, siblings));
 	}
 
+	device_unregister(&shost->host_driverfs_dev);
+
 	return 0;
 }
 
@@ -348,7 +350,6 @@
 
 	/* Cleanup proc and driverfs */
 	scsi_proc_host_rm(shost);
-	device_unregister(&shost->host_driverfs_dev);
 
 	kfree(shost);
 }

Re: [PATCH] scsi/hosts.c device_register fix

[James Bottomley]

[-- Attachment #1: Type: text/plain, Size: 397 bytes --]

Actually, the patch is wrong.  It will wreak havoc with SCSI's use of sysfs.  
The device_register has to be done in scsi_add_host, which is called after all 
the driver specific sysfs setup has been done.  The correct fix is to move the 
corresponding device_unregister into scsi_remove_host so that they match.

I've attached it below.  I'll also commit it to the scsi-misc-2.5 BK tree.

James


[-- Attachment #2: tmp.diff --]
[-- Type: text/plain , Size: 396 bytes --]

===== hosts.c 1.31 vs edited =====
--- 1.31/drivers/scsi/hosts.c	Sun Nov 17 15:47:02 2002
+++ edited/hosts.c	Sat Nov 23 17:25:57 2002
@@ -295,6 +295,8 @@
 		kfree(sdev);
 	}
 
+	device_unregister(&shost->host_driverfs_dev);
+
 	return 0;
 }
 
@@ -348,7 +350,6 @@
 
 	/* Cleanup proc and driverfs */
 	scsi_proc_host_rm(shost);
-	device_unregister(&shost->host_driverfs_dev);
 
 	kfree(shost);
 }

[PATCH] scsi/hosts.c device_register fix

[Andries.Brouwer]

Many scsi hosts do scsi_register, and when the corresponding
host is not found or fails to work, do scsi_unregister again.
Thus, actions in scsi_unregister should be the inverses of those
in scsi_register, just like actions in scsi_remove_host should be
the inverses of those in scsi_add_host.

However, device_register() is done in scsi_add_host() while the
corresponding device_unregister() is done in scsi_unregister().

This causes crashes at boot (in 2.5.49 and 2.5.50).

Below a fix. This patch was first given by James Bottomley.

Andries

diff -u --recursive --new-file -X /linux/dontdiff a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
--- a/drivers/scsi/hosts.c	Thu Nov 28 15:28:19 2002
+++ b/drivers/scsi/hosts.c	Thu Nov 28 17:22:02 2002
@@ -309,7 +309,6 @@
 	printk(KERN_INFO "scsi%d : %s\n", shost->host_no,
 			sht->info ? sht->info(shost) : sht->name);
 
-	device_register(&shost->host_driverfs_dev);
 	scsi_scan_host(shost);
 			
 	list_for_each_entry (sdev, &shost->my_devices, siblings) {
@@ -358,11 +357,6 @@
  * @shost_tp:	pointer to scsi host template
  * @xtr_bytes:	extra bytes to allocate for driver
  *
- * Note:
- * 	We call this when we come across a new host adapter. We only do
- * 	this once we are 100% sure that we want to use this host adapter -
- * 	it is a pain to reverse this, so we try to avoid it 
- *
  * Return value:
  * 	Pointer to a new Scsi_Host
  **/
@@ -478,6 +472,8 @@
 
 	shost->hostt->present++;
 
+	device_register(&shost->host_driverfs_dev);
+
 	return shost;
 }