* [PATCH] scsi_host sysfs updates scsi-misc-2.5 [0/2]
@ 2003-05-09 6:33 Mike Anderson
2003-05-09 6:34 ` [PATCH] scsi_host sysfs updates scsi-misc-2.5 [1/2] Mike Anderson
` (2 more replies)
0 siblings, 3 replies; 22+ messages in thread
From: Mike Anderson @ 2003-05-09 6:33 UTC (permalink / raw)
To: linux-scsi
This series of patches is an update to the scsi_host sysfs / ref
counting patches previously merged into the scsi-misc-2.5 tree.
insmod / rmmod Testing:
Legacy registration:
aic7xxx_old, qlogicisp
qla2xxx v8.00.00b1 - Driver oops system with garbled
messages on rmmod. Does not seem related to this patch.
New registration:
scsi_debug
aic7xxx has an issue with holding lock while calling
scsi_remove_host. I created a test patch to not hold the
lock and the scsi mid layer illegal context messages
stopped. Justin will need to address a real fix.
I am booted on the ips driver, but have done no insmod / rmmod
testing.
Attached is the current Reference Counting Rules and a small future
TODO cleanup list.
Post feedback I will work to roll this into the scsi documentation.
-andmike
--
Michael Anderson
andmike@us.ibm.com
<Referemnce Counting Rules>
1.) Reference counting for scsi_host
scsi_register (scsi_alloc_host):
(A) kmalloc Scsi_Host + xtr_bytes
(B) Call device_initialize, class_device_initialize
refcount is 1 on host_gendev.
refcount is 1 on class_dev.
scsi_get_host:
(A) Call get_device on host_gendev
refcount +1 on host_gendev
(B) Call class_device_get on class_dev
refcount +1 on class_dev
scsi_host_put:
(A) Call put_device on host_gendev
refcount -1 on host_gendev
(B) Call class_device_put on class_dev
refcount -1 on class_dev
scsi_add_host:
(A) Call scsi_sysfs_add_host
(1) Call device_add
refcount +1 on parent struct device
host_gendev now visible in sysfs tree.
(2) Call class_device_add
refcount +1 on scsi_host class
refcount +1 on parent struct device
class_dev now visible in sysfs tree
(C) Call scsi_proc_host_add
Scsi_Host now visible in proc.
(D) Call scsi_scan_host
refcount +1 on host_gendev for each scsi_device
discovered
(E) Call scsi_attach_device.
scsi_remove_host:
(A) "scsi_offline_host" see note above.
(B) Call scsi_proc_host_rm
(C) Call scsi_forget_host
refcount -1 on host_gendev for each scsi_device
unregistered
(D) Call scsi_sysfs_remove_host
(1) Call class_device_del
(2) Call device_del
scsi_unregister:
(A) Call scsi_put_host
scsi_host_release:
(A) Call scsi_free_shost
(1) Remove Scsi_Host from scsi_host_list list.
(2) Kill error recovery thread.
(3) Call scsi_destroy_command_freelist
(4) kfree(shost)
scsi_register_host:
(A) Call template detect
(1) Call scsi_register for each instance
(B) Call scsi_add_host for each instance
scsi_unregister_host:
(A) Call scsi_remove_host, hostt->release for all
Scsi_Host instances.
2.) Reference counting for scsi_device
scsi_add_lun:
(A) Call scsi_device_register
refcount 1 on sdev_driverfs_dev
refcount +1 on parent struct device (i.e host_gendev)
scsi_add_device:
(A) Call scsi_probe_and_add_lun
scsi_remove_device:
(A) Call scsi_detach_device
(B) Call scsi_device_unregister
refcount -1 on sdev_driverfs_dev
refcount -1 on parent struct device (i.e host_gendev)
scsi_device_get:
(A) try_module_get
(B) access_count +1
scsi_device_put:
(A) access_count -1
(B) module_put on host module
TODO:
- scsi_set_host_offline needs work on sync between LLDD and mid on
surprise remove.
- Move upper level attaches to driver model so scsi_add_host does
not need to loop over my_devices. Post the cleanups that have
happen on the upper level driver we really close to matching the
model already this should be doable. (Christoph has old patch
that needs refreshed to handle this)
- Remove scsi_host_get_next.
- Should move scsi_device device_register to (initialize, add)
pair. (Low priority)
- Evaluate the need for scsi_host_list as this is duplicated in
the scsi_host class structure. (Low priority)
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] scsi_host sysfs updates scsi-misc-2.5 [1/2]
2003-05-09 6:33 [PATCH] scsi_host sysfs updates scsi-misc-2.5 [0/2] Mike Anderson
@ 2003-05-09 6:34 ` Mike Anderson
2003-05-09 6:35 ` [PATCH] scsi_host sysfs updates scsi-misc-2.5 [2/2] Mike Anderson
2003-05-12 3:57 ` [PATCH] scsi_host sysfs updates scsi-misc-2.5 [0/2] James Bottomley
2003-05-12 15:15 ` Andrew Vasquez
2 siblings, 1 reply; 22+ messages in thread
From: Mike Anderson @ 2003-05-09 6:34 UTC (permalink / raw)
To: linux-scsi
-andmike
--
Michael Anderson
andmike@us.ibm.com
DESC
scsi_debug cleanups for scsi-misc-2.5
- Remove release function.
- Remove scsi_debug wrapper driver register / unregister functions.
- Douglas's target == this_id fix.
- Remove some old cleanups that where incorrect.
- Move code back into sdebug_driver_remove.
EDESC
drivers/scsi/scsi_debug.c | 78 ++++++++++++++--------------------------------
drivers/scsi/scsi_debug.h | 1
2 files changed, 25 insertions(+), 54 deletions(-)
diff -puN drivers/scsi/scsi_debug.c~scsi_debug-misc-fix drivers/scsi/scsi_debug.c
--- sysfs-scsi-misc-2.5/drivers/scsi/scsi_debug.c~scsi_debug-misc-fix Thu May 8 22:46:47 2003
+++ sysfs-scsi-misc-2.5-andmike/drivers/scsi/scsi_debug.c Thu May 8 22:46:47 2003
@@ -55,7 +55,7 @@
#include "scsi_logging.h"
#include "scsi_debug.h"
-static const char * scsi_debug_version_str = "Version: 1.70 (20030416)";
+static const char * scsi_debug_version_str = "Version: 1.70 (20030507)";
/* Additional Sense Code (ASC) used */
#define NO_ADDED_SENSE 0x0
@@ -169,7 +169,6 @@ static struct sdebug_queued_cmd queued_a
static Scsi_Host_Template sdebug_driver_template = {
.proc_info = scsi_debug_proc_info,
.name = "SCSI DEBUG",
- .release = scsi_debug_release,
.info = scsi_debug_info,
.slave_alloc = scsi_debug_slave_alloc,
.slave_configure = scsi_debug_slave_configure,
@@ -207,9 +206,11 @@ static char sdebug_proc_name[] = "scsi_d
static int sdebug_driver_probe(struct device *);
static int sdebug_driver_remove(struct device *);
+static struct bus_type pseudo_lld_bus;
static struct device_driver sdebug_driverfs_driver = {
.name = sdebug_proc_name,
+ .bus = &pseudo_lld_bus,
.probe = sdebug_driver_probe,
.remove = sdebug_driver_remove,
};
@@ -250,8 +251,6 @@ static int sdebug_add_adapter(void);
static void sdebug_remove_adapter(void);
static struct device pseudo_primary;
static struct bus_type pseudo_lld_bus;
-static int scsi_debug_register_driver(struct device_driver *);
-static int scsi_debug_unregister_driver(struct device_driver *);
static unsigned char * scatg2virt(const struct scatterlist * sclp)
{
@@ -296,10 +295,12 @@ int scsi_debug_queuecommand(struct scsi_
bufflen = SDEBUG_SENSE_LEN;
}
+
if(target == sdebug_driver_template.this_id) {
- printk(KERN_WARNING
- "scsi_debug: initiator's id used as target!\n");
- return schedule_resp(SCpnt, NULL, done, 0, 0);
+ printk(KERN_INFO "scsi_debug: initiator's id used as "
+ "target!\n");
+ return schedule_resp(SCpnt, NULL, done,
+ DID_NO_CONNECT << 16, 0);
}
if (SCpnt->device->lun >= scsi_debug_max_luns)
@@ -1523,7 +1524,7 @@ static int __init scsi_debug_init(void)
device_register(&pseudo_primary);
bus_register(&pseudo_lld_bus);
- scsi_debug_register_driver(&sdebug_driverfs_driver);
+ driver_register(&sdebug_driverfs_driver);
do_create_driverfs_files();
sdebug_driver_template.proc_name = (char *)sdebug_proc_name;
@@ -1554,7 +1555,7 @@ static void __exit scsi_debug_exit(void)
for (; k; k--)
sdebug_remove_adapter();
do_remove_driverfs_files();
- scsi_debug_unregister_driver(&sdebug_driverfs_driver);
+ driver_unregister(&sdebug_driverfs_driver);
bus_unregister(&pseudo_lld_bus);
device_unregister(&pseudo_primary);
@@ -1580,20 +1581,6 @@ static struct bus_type pseudo_lld_bus =
.match = pseudo_lld_bus_match,
};
-static int scsi_debug_register_driver(struct device_driver *dev_driver)
-{
- dev_driver->bus = &pseudo_lld_bus;
- driver_register(dev_driver);
-
- return 0;
-}
-
-static int scsi_debug_unregister_driver(struct device_driver *dev_driver)
-{
- driver_unregister(dev_driver);
- return 0;
-}
-
static void sdebug_release_adapter(struct device * dev)
{
struct sdebug_host_info *sdbg_host;
@@ -1698,7 +1685,7 @@ static int sdebug_driver_probe(struct de
if (NULL == hpnt) {
printk(KERN_ERR "%s: scsi_register failed\n", __FUNCTION__);
error = -ENODEV;
- goto clean1;
+ return error;
}
sdbg_host->shost = hpnt;
@@ -1713,34 +1700,34 @@ static int sdebug_driver_probe(struct de
if (error) {
printk(KERN_ERR "%s: scsi_add_host failed\n", __FUNCTION__);
error = -ENODEV;
- goto clean2;
+ scsi_unregister(hpnt);
}
- return 0;
-
-clean2:
- scsi_unregister(hpnt);
-clean1:
- kfree(sdbg_host);
-
return error;
}
-static int scsi_debug_release(struct Scsi_Host * shost)
+static int sdebug_driver_remove(struct device * dev)
{
struct list_head *lh, *lh_sf;
- struct sdebug_dev_info *sdbg_devinfo;
struct sdebug_host_info *sdbg_host;
+ struct sdebug_dev_info *sdbg_devinfo;
- sdbg_host = *(struct sdebug_host_info **)shost->hostdata;
- scsi_unregister(shost);
+ sdbg_host = to_sdebug_host(dev);
if (!sdbg_host) {
- printk(KERN_ERR "Unable to locate host info\n");
- return 0;
+ printk(KERN_ERR "%s: Unable to locate host info\n",
+ __FUNCTION__);
+ return -ENODEV;
}
+ if (scsi_remove_host(sdbg_host->shost)) {
+ printk(KERN_ERR "%s: scsi_remove_host failed\n", __FUNCTION__);
+ return -EBUSY;
+ }
+
+ scsi_unregister(sdbg_host->shost);
+
list_for_each_safe(lh, lh_sf, &sdbg_host->dev_info_list) {
sdbg_devinfo = list_entry(lh, struct sdebug_dev_info,
dev_list);
@@ -1749,19 +1736,4 @@ static int scsi_debug_release(struct Scs
}
return 0;
-
-}
-
-static int sdebug_driver_remove(struct device * dev)
-{
- struct sdebug_host_info *sdbg_host;
-
- sdbg_host = to_sdebug_host(dev);
-
- if (sdbg_host && scsi_remove_host(sdbg_host->shost)) {
- printk(KERN_ERR "%s: scsi_remove_host failed\n", __FUNCTION__);
- return -EBUSY;
- }
-
- return 0;
}
diff -puN drivers/scsi/scsi_debug.h~scsi_debug-misc-fix drivers/scsi/scsi_debug.h
--- sysfs-scsi-misc-2.5/drivers/scsi/scsi_debug.h~scsi_debug-misc-fix Thu May 8 22:46:47 2003
+++ sysfs-scsi-misc-2.5-andmike/drivers/scsi/scsi_debug.h Thu May 8 22:46:47 2003
@@ -16,7 +16,6 @@ static int scsi_debug_device_reset(struc
static int scsi_debug_host_reset(struct scsi_cmnd *);
static int scsi_debug_proc_info(char *, char **, off_t, int, int, int);
static const char * scsi_debug_info(struct Scsi_Host *);
-static int scsi_debug_release(struct Scsi_Host *);
/*
* This driver is written for the lk 2.5 series
_
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] scsi_host sysfs updates scsi-misc-2.5 [2/2]
2003-05-09 6:34 ` [PATCH] scsi_host sysfs updates scsi-misc-2.5 [1/2] Mike Anderson
@ 2003-05-09 6:35 ` Mike Anderson
2003-05-09 6:59 ` Christoph Hellwig
0 siblings, 1 reply; 22+ messages in thread
From: Mike Anderson @ 2003-05-09 6:35 UTC (permalink / raw)
To: linux-scsi
-andmike
--
Michael Anderson
andmike@us.ibm.com
DESC
scsi shost sysfs cleanups for scsi-misc-2.5
- Add LLDD short name to scsi_host struct device.
- scsi_host_release now calls scsi_free_shost.
- Switched from device_register / device_unregister and class_register
/ class_register to initialize, add, del, put pairs.
- Moved some function from scsi_register and scsi_unregister.
- Filled in scsi_host_put and scsi_host_get.
EDESC
drivers/scsi/hosts.c | 33 ++++++++++++++++++++++++++++-----
drivers/scsi/hosts.h | 2 ++
drivers/scsi/scsi_sysfs.c | 19 +++++++++++--------
3 files changed, 41 insertions(+), 13 deletions(-)
diff -puN drivers/scsi/scsi_sysfs.c~scsi_shost_sysfs-misc-fix drivers/scsi/scsi_sysfs.c
--- sysfs-scsi-misc-2.5/drivers/scsi/scsi_sysfs.c~scsi_shost_sysfs-misc-fix Thu May 8 22:46:53 2003
+++ sysfs-scsi-misc-2.5-andmike/drivers/scsi/scsi_sysfs.c Thu May 8 22:46:53 2003
@@ -317,7 +317,8 @@ static void scsi_host_release(struct dev
shost = dev_to_shost(dev);
if (!shost)
return;
- shost->hostt->release(shost);
+
+ scsi_free_shost(shost);
}
/**
@@ -329,13 +330,15 @@ int scsi_sysfs_add_host(struct Scsi_Host
{
int i, error;
- sprintf(shost->host_gendev.bus_id,"host%d",
+ snprintf(shost->host_gendev.bus_id, BUS_ID_SIZE, "host%d",
shost->host_no);
+ snprintf(shost->host_gendev.name, DEVICE_NAME_SIZE, "%s",
+ shost->hostt->proc_name);
if (!shost->host_gendev.parent)
shost->host_gendev.parent = (dev) ? dev : &legacy_bus;
shost->host_gendev.release = scsi_host_release;
- error = device_register(&shost->host_gendev);
+ error = device_add(&shost->host_gendev);
if (error)
return error;
@@ -343,7 +346,7 @@ int scsi_sysfs_add_host(struct Scsi_Host
shost->class_dev.class = &shost_class;
snprintf(shost->class_dev.class_id, BUS_ID_SIZE, "host%d",
shost->host_no);
- error = class_device_register(&shost->class_dev);
+ error = class_device_add(&shost->class_dev);
if (error)
goto clean_device;
@@ -356,9 +359,9 @@ int scsi_sysfs_add_host(struct Scsi_Host
return error;
clean_class:
- class_device_unregister(&shost->class_dev);
+ class_device_del(&shost->class_dev);
clean_device:
- device_unregister(&shost->host_gendev);
+ device_del(&shost->host_gendev);
return error;
}
@@ -369,7 +372,7 @@ clean_device:
**/
void scsi_sysfs_remove_host(struct Scsi_Host *shost)
{
- class_device_unregister(&shost->class_dev);
- device_unregister(&shost->host_gendev);
+ class_device_del(&shost->class_dev);
+ device_del(&shost->host_gendev);
}
diff -puN drivers/scsi/hosts.c~scsi_shost_sysfs-misc-fix drivers/scsi/hosts.c
--- sysfs-scsi-misc-2.5/drivers/scsi/hosts.c~scsi_shost_sysfs-misc-fix Thu May 8 22:46:53 2003
+++ sysfs-scsi-misc-2.5-andmike/drivers/scsi/hosts.c Thu May 8 22:46:53 2003
@@ -212,6 +212,7 @@ int scsi_remove_host(struct Scsi_Host *s
list_for_each_entry(sdev, &shost->my_devices, siblings)
sdev->online = FALSE;
+ scsi_proc_host_rm(shost);
scsi_forget_host(shost);
scsi_sysfs_remove_host(shost);
return 0;
@@ -238,6 +239,8 @@ int scsi_add_host(struct Scsi_Host *shos
if (error)
return error;
+ scsi_proc_host_add(shost);
+
scsi_scan_host(shost);
list_for_each_entry (sdev, &shost->my_devices, siblings) {
@@ -255,6 +258,15 @@ int scsi_add_host(struct Scsi_Host *shos
**/
void scsi_unregister(struct Scsi_Host *shost)
{
+ scsi_host_put(shost);
+}
+
+/**
+ * scsi_free_sdev - free a scsi hosts resources
+ * @shost: scsi host to free
+ **/
+void scsi_free_shost(struct Scsi_Host *shost)
+{
/* Remove shost from scsi_host_list */
spin_lock(&scsi_host_list_lock);
list_del(&shost->sh_list);
@@ -273,7 +285,6 @@ void scsi_unregister(struct Scsi_Host *s
}
shost->hostt->present--;
- scsi_proc_host_rm(shost);
scsi_destroy_command_freelist(shost);
kfree(shost);
}
@@ -394,7 +405,8 @@ found:
rval = scsi_setup_command_freelist(shost);
if (rval)
goto fail;
- scsi_proc_host_add(shost);
+ device_initialize(&shost->host_gendev);
+ class_device_initialize(&shost->class_dev);
shost->eh_notify = &sem;
kernel_thread((int (*)(void *)) scsi_error_handler, (void *) shost, 0);
@@ -523,15 +535,26 @@ struct Scsi_Host *scsi_host_hn_get(unsig
}
/**
+ * *scsi_host_get - inc a Scsi_Host ref count
+ * @shost: Pointer to Scsi_Host to inc.
+ **/
+void scsi_host_get(struct Scsi_Host *shost)
+{
+
+ get_device(&shost->host_gendev);
+ class_device_get(&shost->class_dev);
+ return;
+}
+
+/**
* *scsi_host_put - dec a Scsi_Host ref count
* @shost: Pointer to Scsi_Host to dec.
**/
void scsi_host_put(struct Scsi_Host *shost)
{
- /* XXX Get list lock */
- /* XXX dec ref count */
- /* XXX Release list lock */
+ put_device(&shost->host_gendev);
+ class_device_put(&shost->class_dev);
return;
}
diff -puN drivers/scsi/hosts.h~scsi_shost_sysfs-misc-fix drivers/scsi/hosts.h
--- sysfs-scsi-misc-2.5/drivers/scsi/hosts.h~scsi_shost_sysfs-misc-fix Thu May 8 22:46:53 2003
+++ sysfs-scsi-misc-2.5-andmike/drivers/scsi/hosts.h Thu May 8 22:46:53 2003
@@ -556,6 +556,7 @@ extern int scsi_unregister_device(struct
*/
extern struct Scsi_Host * scsi_register(Scsi_Host_Template *, int);
extern void scsi_unregister(struct Scsi_Host *);
+extern void scsi_free_shost(struct Scsi_Host *);
/*
* HBA registration/unregistration.
@@ -570,6 +571,7 @@ extern int scsi_register_host(Scsi_Host_
extern int scsi_unregister_host(Scsi_Host_Template *);
extern struct Scsi_Host *scsi_host_hn_get(unsigned short);
+extern void scsi_host_get(struct Scsi_Host *);
extern void scsi_host_put(struct Scsi_Host *);
/**
_
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] scsi_host sysfs updates scsi-misc-2.5 [2/2]
2003-05-09 6:35 ` [PATCH] scsi_host sysfs updates scsi-misc-2.5 [2/2] Mike Anderson
@ 2003-05-09 6:59 ` Christoph Hellwig
2003-05-09 7:50 ` Mike Anderson
0 siblings, 1 reply; 22+ messages in thread
From: Christoph Hellwig @ 2003-05-09 6:59 UTC (permalink / raw)
To: linux-scsi
On Thu, May 08, 2003 at 11:35:45PM -0700, Mike Anderson wrote:
> --- sysfs-scsi-misc-2.5/drivers/scsi/hosts.h~scsi_shost_sysfs-misc-fix Thu May 8 22:46:53 2003
> +++ sysfs-scsi-misc-2.5-andmike/drivers/scsi/hosts.h Thu May 8 22:46:53 2003
> @@ -556,6 +556,7 @@ extern int scsi_unregister_device(struct
> */
> extern struct Scsi_Host * scsi_register(Scsi_Host_Template *, int);
> extern void scsi_unregister(struct Scsi_Host *);
> +extern void scsi_free_shost(struct Scsi_Host *);
This should go into scsi_priv.h
>
> /*
> * HBA registration/unregistration.
> @@ -570,6 +571,7 @@ extern int scsi_register_host(Scsi_Host_
> extern int scsi_unregister_host(Scsi_Host_Template *);
>
> extern struct Scsi_Host *scsi_host_hn_get(unsigned short);
> +extern void scsi_host_get(struct Scsi_Host *);
> extern void scsi_host_put(struct Scsi_Host *);
This should go either into scsi_priv.h or exported if we see
a use for it in driver (I'd rather avoid drivers messing with
reference counts, though)
Patch looks fine otherwise.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] scsi_host sysfs updates scsi-misc-2.5 [2/2]
2003-05-09 6:59 ` Christoph Hellwig
@ 2003-05-09 7:50 ` Mike Anderson
2003-05-09 8:21 ` Mike Anderson
0 siblings, 1 reply; 22+ messages in thread
From: Mike Anderson @ 2003-05-09 7:50 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-scsi
Christoph Hellwig [hch@infradead.org] wrote:
> On Thu, May 08, 2003 at 11:35:45PM -0700, Mike Anderson wrote:
> > --- sysfs-scsi-misc-2.5/drivers/scsi/hosts.h~scsi_shost_sysfs-misc-fix Thu May 8 22:46:53 2003
> > +++ sysfs-scsi-misc-2.5-andmike/drivers/scsi/hosts.h Thu May 8 22:46:53 2003
> > @@ -556,6 +556,7 @@ extern int scsi_unregister_device(struct
> > */
> > extern struct Scsi_Host * scsi_register(Scsi_Host_Template *, int);
> > extern void scsi_unregister(struct Scsi_Host *);
> > +extern void scsi_free_shost(struct Scsi_Host *);
>
> This should go into scsi_priv.h
>
Ok I will move this and resend to James.
> >
> > /*
> > * HBA registration/unregistration.
> > @@ -570,6 +571,7 @@ extern int scsi_register_host(Scsi_Host_
> > extern int scsi_unregister_host(Scsi_Host_Template *);
> >
> > extern struct Scsi_Host *scsi_host_hn_get(unsigned short);
> > +extern void scsi_host_get(struct Scsi_Host *);
> > extern void scsi_host_put(struct Scsi_Host *);
>
> This should go either into scsi_priv.h or exported if we see
> a use for it in driver (I'd rather avoid drivers messing with
> reference counts, though)
>
I think it should go into scsi_priv.h. I thought it would be good to have
a symmetric interface, but would worry about drivers messing with the
counts if it was exported.
Note: scsi_host_get_next and scsi_host_hn_get need ref counting, but
I wanted to look at this a bit and see if we could utilize some
sysfs interfaces.
-andmike
--
Michael Anderson
andmike@us.ibm.com
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] scsi_host sysfs updates scsi-misc-2.5 [2/2]
2003-05-09 7:50 ` Mike Anderson
@ 2003-05-09 8:21 ` Mike Anderson
0 siblings, 0 replies; 22+ messages in thread
From: Mike Anderson @ 2003-05-09 8:21 UTC (permalink / raw)
To: James Bottomley; +Cc: linux-scsi
Here is an update of the patch with the externs in scsi_priv.h
-andmike
--
Michael Anderson
andmike@us.ibm.com
DESC
scsi shost sysfs cleanups for scsi-misc-2.5
- Add LLDD short name to scsi_host struct device.
- scsi_host_release now calls scsi_free_shost.
- Switched from device_register / device_unregister and class_register
/ class_register to initialize, add, del, put pairs.
- Moved some function from scsi_register and scsi_unregister.
- Filled in scsi_host_put and scsi_host_get.
Rev 2 move externs to scsi_priv.h
EDESC
drivers/scsi/hosts.c | 33 ++++++++++++++++++++++++++++-----
drivers/scsi/scsi_priv.h | 2 ++
drivers/scsi/scsi_sysfs.c | 19 +++++++++++--------
3 files changed, 41 insertions(+), 13 deletions(-)
diff -puN drivers/scsi/scsi_sysfs.c~scsi_shost_sysfs-misc-fix drivers/scsi/scsi_sysfs.c
--- sysfs-scsi-misc-2.5/drivers/scsi/scsi_sysfs.c~scsi_shost_sysfs-misc-fix Fri May 9 00:50:22 2003
+++ sysfs-scsi-misc-2.5-andmike/drivers/scsi/scsi_sysfs.c Fri May 9 00:50:22 2003
@@ -317,7 +317,8 @@ static void scsi_host_release(struct dev
shost = dev_to_shost(dev);
if (!shost)
return;
- shost->hostt->release(shost);
+
+ scsi_free_shost(shost);
}
/**
@@ -329,13 +330,15 @@ int scsi_sysfs_add_host(struct Scsi_Host
{
int i, error;
- sprintf(shost->host_gendev.bus_id,"host%d",
+ snprintf(shost->host_gendev.bus_id, BUS_ID_SIZE, "host%d",
shost->host_no);
+ snprintf(shost->host_gendev.name, DEVICE_NAME_SIZE, "%s",
+ shost->hostt->proc_name);
if (!shost->host_gendev.parent)
shost->host_gendev.parent = (dev) ? dev : &legacy_bus;
shost->host_gendev.release = scsi_host_release;
- error = device_register(&shost->host_gendev);
+ error = device_add(&shost->host_gendev);
if (error)
return error;
@@ -343,7 +346,7 @@ int scsi_sysfs_add_host(struct Scsi_Host
shost->class_dev.class = &shost_class;
snprintf(shost->class_dev.class_id, BUS_ID_SIZE, "host%d",
shost->host_no);
- error = class_device_register(&shost->class_dev);
+ error = class_device_add(&shost->class_dev);
if (error)
goto clean_device;
@@ -356,9 +359,9 @@ int scsi_sysfs_add_host(struct Scsi_Host
return error;
clean_class:
- class_device_unregister(&shost->class_dev);
+ class_device_del(&shost->class_dev);
clean_device:
- device_unregister(&shost->host_gendev);
+ device_del(&shost->host_gendev);
return error;
}
@@ -369,7 +372,7 @@ clean_device:
**/
void scsi_sysfs_remove_host(struct Scsi_Host *shost)
{
- class_device_unregister(&shost->class_dev);
- device_unregister(&shost->host_gendev);
+ class_device_del(&shost->class_dev);
+ device_del(&shost->host_gendev);
}
diff -puN drivers/scsi/hosts.c~scsi_shost_sysfs-misc-fix drivers/scsi/hosts.c
--- sysfs-scsi-misc-2.5/drivers/scsi/hosts.c~scsi_shost_sysfs-misc-fix Fri May 9 00:50:22 2003
+++ sysfs-scsi-misc-2.5-andmike/drivers/scsi/hosts.c Fri May 9 00:50:22 2003
@@ -212,6 +212,7 @@ int scsi_remove_host(struct Scsi_Host *s
list_for_each_entry(sdev, &shost->my_devices, siblings)
sdev->online = FALSE;
+ scsi_proc_host_rm(shost);
scsi_forget_host(shost);
scsi_sysfs_remove_host(shost);
return 0;
@@ -238,6 +239,8 @@ int scsi_add_host(struct Scsi_Host *shos
if (error)
return error;
+ scsi_proc_host_add(shost);
+
scsi_scan_host(shost);
list_for_each_entry (sdev, &shost->my_devices, siblings) {
@@ -255,6 +258,15 @@ int scsi_add_host(struct Scsi_Host *shos
**/
void scsi_unregister(struct Scsi_Host *shost)
{
+ scsi_host_put(shost);
+}
+
+/**
+ * scsi_free_sdev - free a scsi hosts resources
+ * @shost: scsi host to free
+ **/
+void scsi_free_shost(struct Scsi_Host *shost)
+{
/* Remove shost from scsi_host_list */
spin_lock(&scsi_host_list_lock);
list_del(&shost->sh_list);
@@ -273,7 +285,6 @@ void scsi_unregister(struct Scsi_Host *s
}
shost->hostt->present--;
- scsi_proc_host_rm(shost);
scsi_destroy_command_freelist(shost);
kfree(shost);
}
@@ -394,7 +405,8 @@ found:
rval = scsi_setup_command_freelist(shost);
if (rval)
goto fail;
- scsi_proc_host_add(shost);
+ device_initialize(&shost->host_gendev);
+ class_device_initialize(&shost->class_dev);
shost->eh_notify = &sem;
kernel_thread((int (*)(void *)) scsi_error_handler, (void *) shost, 0);
@@ -523,15 +535,26 @@ struct Scsi_Host *scsi_host_hn_get(unsig
}
/**
+ * *scsi_host_get - inc a Scsi_Host ref count
+ * @shost: Pointer to Scsi_Host to inc.
+ **/
+void scsi_host_get(struct Scsi_Host *shost)
+{
+
+ get_device(&shost->host_gendev);
+ class_device_get(&shost->class_dev);
+ return;
+}
+
+/**
* *scsi_host_put - dec a Scsi_Host ref count
* @shost: Pointer to Scsi_Host to dec.
**/
void scsi_host_put(struct Scsi_Host *shost)
{
- /* XXX Get list lock */
- /* XXX dec ref count */
- /* XXX Release list lock */
+ put_device(&shost->host_gendev);
+ class_device_put(&shost->class_dev);
return;
}
diff -puN drivers/scsi/scsi_priv.h~scsi_shost_sysfs-misc-fix drivers/scsi/scsi_priv.h
--- sysfs-scsi-misc-2.5/drivers/scsi/scsi_priv.h~scsi_shost_sysfs-misc-fix Fri May 9 00:50:22 2003
+++ sysfs-scsi-misc-2.5-andmike/drivers/scsi/scsi_priv.h Fri May 9 00:51:20 2003
@@ -109,6 +109,8 @@ extern void scsi_exit_procfs(void);
extern void scsi_scan_host(struct Scsi_Host *shost);
extern void scsi_forget_host(struct Scsi_Host *shost);
extern void scsi_free_sdev(struct scsi_device *);
+extern void scsi_free_shost(struct Scsi_Host *);
+extern void scsi_host_get(struct Scsi_Host *);
/* scsi_sysfs.c */
extern int scsi_device_register(struct scsi_device *);
_
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] scsi_host sysfs updates scsi-misc-2.5 [0/2]
2003-05-09 6:33 [PATCH] scsi_host sysfs updates scsi-misc-2.5 [0/2] Mike Anderson
2003-05-09 6:34 ` [PATCH] scsi_host sysfs updates scsi-misc-2.5 [1/2] Mike Anderson
@ 2003-05-12 3:57 ` James Bottomley
2003-05-12 6:38 ` Mike Anderson
2003-05-12 15:15 ` Andrew Vasquez
2 siblings, 1 reply; 22+ messages in thread
From: James Bottomley @ 2003-05-12 3:57 UTC (permalink / raw)
To: Mike Anderson; +Cc: SCSI Mailing List
[-- Attachment #1: Type: text/plain, Size: 723 bytes --]
On Fri, 2003-05-09 at 01:33, Mike Anderson wrote:
> This series of patches is an update to the scsi_host sysfs / ref
> counting patches previously merged into the scsi-misc-2.5 tree.
I'm still getting an oops in a scsi_register followed by a
scsi_unregister (because of an error in the driver setup). The problem
occurs because the mid-layer is now dependent on the host_gendev.release
method. Unfortunately, this isn't set until scsi_host_add, which may be
quite a while after scsi_register.
The quick "fix" is attached below, but I think we need all of this to be
symmetric (i.e. scsi_unregister can be called any time after
scsi_register) so probably the host_gendev.release method should be set
elsewhere.
James
[-- Attachment #2: tmp.diff --]
[-- Type: text/plain, Size: 360 bytes --]
===== hosts.c 1.62 vs edited =====
--- 1.62/drivers/scsi/hosts.c Thu May 8 19:50:22 2003
+++ edited/hosts.c Sun May 11 22:38:50 2003
@@ -258,7 +258,10 @@
**/
void scsi_unregister(struct Scsi_Host *shost)
{
- scsi_host_put(shost);
+ if(shost->host_gendev.release == NULL)
+ scsi_free_shost(shost);
+ else
+ scsi_host_put(shost);
}
/**
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] scsi_host sysfs updates scsi-misc-2.5 [0/2]
2003-05-12 3:57 ` [PATCH] scsi_host sysfs updates scsi-misc-2.5 [0/2] James Bottomley
@ 2003-05-12 6:38 ` Mike Anderson
2003-05-12 17:50 ` James Bottomley
0 siblings, 1 reply; 22+ messages in thread
From: Mike Anderson @ 2003-05-12 6:38 UTC (permalink / raw)
To: James Bottomley; +Cc: SCSI Mailing List
James Bottomley [James.Bottomley@steeleye.com] wrote:
> On Fri, 2003-05-09 at 01:33, Mike Anderson wrote:
> > This series of patches is an update to the scsi_host sysfs / ref
> > counting patches previously merged into the scsi-misc-2.5 tree.
>
> I'm still getting an oops in a scsi_register followed by a
> scsi_unregister (because of an error in the driver setup). The problem
> occurs because the mid-layer is now dependent on the host_gendev.release
> method. Unfortunately, this isn't set until scsi_host_add, which may be
> quite a while after scsi_register.
>
> The quick "fix" is attached below, but I think we need all of this to be
> symmetric (i.e. scsi_unregister can be called any time after
> scsi_register) so probably the host_gendev.release method should be set
> elsewhere.
I agree on the symmetric interface. I attached a patch that adds a
scsi_sysfs_init_host function call which moves more initialization during
the scsi_register time frame.
I tested the attached patch on my current config of ips, qlogicisp,
aic7xxx, and scsi_debug.
Can you run this on your system and see if it addresses your issue? If
not I can hack scsi_debug to make back to back scsi_register /
scsi_unregister calls.
-andmike
--
Michael Anderson
andmike@us.ibm.com
DESC
Patch against scsi-misc-2.5
Fix scsi sysfs init so that a scsi_unregister can be called anytime after
a scsi_register.
- Create scsi_sysfs_init_host function and call from
scsi_register.
EDESC
drivers/scsi/hosts.c | 4 ++--
drivers/scsi/scsi_priv.h | 1 +
drivers/scsi/scsi_sysfs.c | 25 ++++++++++++++++---------
3 files changed, 19 insertions(+), 11 deletions(-)
diff -puN drivers/scsi/scsi_priv.h~scsi_unregister-syfs-fix drivers/scsi/scsi_priv.h
--- sysfs-scsi-misc-2.5/drivers/scsi/scsi_priv.h~scsi_unregister-syfs-fix Sun May 11 22:05:01 2003
+++ sysfs-scsi-misc-2.5-andmike/drivers/scsi/scsi_priv.h Sun May 11 22:12:54 2003
@@ -123,6 +123,7 @@ extern int scsi_device_register(struct s
extern void scsi_device_unregister(struct scsi_device *);
extern int scsi_upper_driver_register(struct Scsi_Device_Template *);
extern void scsi_upper_driver_unregister(struct Scsi_Device_Template *);
+extern void scsi_sysfs_init_host(struct Scsi_Host *);
extern int scsi_sysfs_add_host(struct Scsi_Host *, struct device *);
extern void scsi_sysfs_remove_host(struct Scsi_Host *);
extern int scsi_sysfs_register(void);
diff -puN drivers/scsi/scsi_sysfs.c~scsi_unregister-syfs-fix drivers/scsi/scsi_sysfs.c
--- sysfs-scsi-misc-2.5/drivers/scsi/scsi_sysfs.c~scsi_unregister-syfs-fix Sun May 11 22:06:08 2003
+++ sysfs-scsi-misc-2.5-andmike/drivers/scsi/scsi_sysfs.c Sun May 11 22:12:40 2003
@@ -321,6 +321,22 @@ static void scsi_host_release(struct dev
scsi_free_shost(shost);
}
+void scsi_sysfs_init_host(struct Scsi_Host *shost)
+{
+ device_initialize(&shost->host_gendev);
+ snprintf(shost->host_gendev.bus_id, BUS_ID_SIZE, "host%d",
+ shost->host_no);
+ snprintf(shost->host_gendev.name, DEVICE_NAME_SIZE, "%s",
+ shost->hostt->proc_name);
+ shost->host_gendev.release = scsi_host_release;
+
+ class_device_initialize(&shost->class_dev);
+ shost->class_dev.dev = &shost->host_gendev;
+ shost->class_dev.class = &shost_class;
+ snprintf(shost->class_dev.class_id, BUS_ID_SIZE, "host%d",
+ shost->host_no);
+}
+
/**
* scsi_sysfs_add_host - add scsi host to subsystem
* @shost: scsi host struct to add to subsystem
@@ -330,22 +346,13 @@ int scsi_sysfs_add_host(struct Scsi_Host
{
int i, error;
- snprintf(shost->host_gendev.bus_id, BUS_ID_SIZE, "host%d",
- shost->host_no);
- snprintf(shost->host_gendev.name, DEVICE_NAME_SIZE, "%s",
- shost->hostt->proc_name);
if (!shost->host_gendev.parent)
shost->host_gendev.parent = (dev) ? dev : &legacy_bus;
- shost->host_gendev.release = scsi_host_release;
error = device_add(&shost->host_gendev);
if (error)
return error;
- shost->class_dev.dev = &shost->host_gendev;
- shost->class_dev.class = &shost_class;
- snprintf(shost->class_dev.class_id, BUS_ID_SIZE, "host%d",
- shost->host_no);
error = class_device_add(&shost->class_dev);
if (error)
goto clean_device;
diff -puN drivers/scsi/hosts.c~scsi_unregister-syfs-fix drivers/scsi/hosts.c
--- sysfs-scsi-misc-2.5/drivers/scsi/hosts.c~scsi_unregister-syfs-fix Sun May 11 22:06:19 2003
+++ sysfs-scsi-misc-2.5-andmike/drivers/scsi/hosts.c Sun May 11 22:11:08 2003
@@ -405,8 +405,8 @@ found:
rval = scsi_setup_command_freelist(shost);
if (rval)
goto fail;
- device_initialize(&shost->host_gendev);
- class_device_initialize(&shost->class_dev);
+
+ scsi_sysfs_init_host(shost);
shost->eh_notify = &sem;
kernel_thread((int (*)(void *)) scsi_error_handler, (void *) shost, 0);
_
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] scsi_host sysfs updates scsi-misc-2.5 [0/2]
2003-05-09 6:33 [PATCH] scsi_host sysfs updates scsi-misc-2.5 [0/2] Mike Anderson
2003-05-09 6:34 ` [PATCH] scsi_host sysfs updates scsi-misc-2.5 [1/2] Mike Anderson
2003-05-12 3:57 ` [PATCH] scsi_host sysfs updates scsi-misc-2.5 [0/2] James Bottomley
@ 2003-05-12 15:15 ` Andrew Vasquez
2003-05-13 18:51 ` Mike Anderson
2 siblings, 1 reply; 22+ messages in thread
From: Andrew Vasquez @ 2003-05-12 15:15 UTC (permalink / raw)
To: linux-scsi
On Thu, 08 May 2003, Mike Anderson wrote:
> This series of patches is an update to the scsi_host sysfs / ref
> counting patches previously merged into the scsi-misc-2.5 tree.
>
> insmod / rmmod Testing:
> Legacy registration:
> aic7xxx_old, qlogicisp
> qla2xxx v8.00.00b1 - Driver oops system with garbled
> messages on rmmod. Does not seem related to this patch.
>
Is the driver still oops'ng during your testing? Could you provide
some specifics on attached storage as well as environment (fabric,
loop, hba type)? I'll resync my working tree with scsi-misc and
perform some additional investigation.
Thanks,
Andrew Vasquez
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] scsi_host sysfs updates scsi-misc-2.5 [0/2]
2003-05-12 6:38 ` Mike Anderson
@ 2003-05-12 17:50 ` James Bottomley
2003-05-12 17:59 ` James Bottomley
2003-05-12 18:18 ` Mike Anderson
0 siblings, 2 replies; 22+ messages in thread
From: James Bottomley @ 2003-05-12 17:50 UTC (permalink / raw)
To: Mike Anderson; +Cc: SCSI Mailing List
On Mon, 2003-05-12 at 01:38, Mike Anderson wrote:
> Can you run this on your system and see if it addresses your issue? If
> not I can hack scsi_debug to make back to back scsi_register /
> scsi_unregister calls.
Surprisingly, it still fails. I've debugged the refcounting and I see
it fall to zero in scsi_host_put but the dev release method doesn't
trigger for no reason I can see.
It's probably a kobject initialisation problem somewhere.
Also, eventually, don't we need to do class_device_remove_files() on the
created class files, so that will have to be done in release and we've
got symmetry problems again?
James
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] scsi_host sysfs updates scsi-misc-2.5 [0/2]
2003-05-12 17:50 ` James Bottomley
@ 2003-05-12 17:59 ` James Bottomley
2003-05-12 18:41 ` Mike Anderson
2003-05-12 18:18 ` Mike Anderson
1 sibling, 1 reply; 22+ messages in thread
From: James Bottomley @ 2003-05-12 17:59 UTC (permalink / raw)
To: James Bottomley; +Cc: Mike Anderson, SCSI Mailing List, mochel
On Mon, 2003-05-12 at 12:50, James Bottomley wrote:
> Surprisingly, it still fails. I've debugged the refcounting and I see
> it fall to zero in scsi_host_put but the dev release method doesn't
> trigger for no reason I can see.
>
> It's probably a kobject initialisation problem somewhere.
OK, found it. the ->release is triggered by the kobj_kset, which isn't
set until device_add time, not device init time.
Therefore, we shouldn't really put and get objects until they've been
added (or the kobj_set_kset_s needs to be moved to device_initialize()).
The more I think about it, the more it does look like we need to
separate the partially initialized case from the fully initialized one,
so perhaps the initial patch I made and thought was awful may actually
turn out to be the more correct one.
James
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] scsi_host sysfs updates scsi-misc-2.5 [0/2]
2003-05-12 17:50 ` James Bottomley
2003-05-12 17:59 ` James Bottomley
@ 2003-05-12 18:18 ` Mike Anderson
1 sibling, 0 replies; 22+ messages in thread
From: Mike Anderson @ 2003-05-12 18:18 UTC (permalink / raw)
To: James Bottomley; +Cc: SCSI Mailing List
James Bottomley [James.Bottomley@steeleye.com] wrote:
> Also, eventually, don't we need to do class_device_remove_files() on the
> created class files, so that will have to be done in release and we've
> got symmetry problems again?
>
I have been told that we you do not need to call
class_device_remove_files as the cleanup functionality of *_remove_files
happens implicitly through the call to sysfs_remove_dir. I will verify
this.
If we needed to add this back in we would add it to scsi_sysfs_remove_host
as they are created in scsi_sysfs_add_host.
-andmike
--
Michael Anderson
andmike@us.ibm.com
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] scsi_host sysfs updates scsi-misc-2.5 [0/2]
2003-05-12 17:59 ` James Bottomley
@ 2003-05-12 18:41 ` Mike Anderson
2003-05-12 20:10 ` James Bottomley
2003-05-14 0:00 ` Patrick Mochel
0 siblings, 2 replies; 22+ messages in thread
From: Mike Anderson @ 2003-05-12 18:41 UTC (permalink / raw)
To: James Bottomley; +Cc: SCSI Mailing List, mochel
James Bottomley [James.Bottomley@steeleye.com] wrote:
> On Mon, 2003-05-12 at 12:50, James Bottomley wrote:
> > Surprisingly, it still fails. I've debugged the refcounting and I see
> > it fall to zero in scsi_host_put but the dev release method doesn't
> > trigger for no reason I can see.
> >
> > It's probably a kobject initialisation problem somewhere.
>
> OK, found it. the ->release is triggered by the kobj_kset, which isn't
> set until device_add time, not device init time.
>
> Therefore, we shouldn't really put and get objects until they've been
> added (or the kobj_set_kset_s needs to be moved to device_initialize()).
>
I would like to have feedback from Mochel on the device_initialize
call.
The documentation I have read indicates the device_initialize call was to
allow the struct device to be usable for reference counting. I think we
should determine if that is the intent and can we call kobj_set_kset_s
in device_initialize.
Another option may be to set the ktype value on our struct device
kobject until device_add overrides this with the call to kobj_set_kset_s.
-andmike
--
Michael Anderson
andmike@us.ibm.com
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] scsi_host sysfs updates scsi-misc-2.5 [0/2]
2003-05-12 18:41 ` Mike Anderson
@ 2003-05-12 20:10 ` James Bottomley
2003-05-12 20:35 ` Mike Anderson
2003-05-14 0:00 ` Patrick Mochel
1 sibling, 1 reply; 22+ messages in thread
From: James Bottomley @ 2003-05-12 20:10 UTC (permalink / raw)
To: Mike Anderson; +Cc: SCSI Mailing List, mochel
On Mon, 2003-05-12 at 13:41, Mike Anderson wrote:
> I would like to have feedback from Mochel on the device_initialize
> call.
>
> The documentation I have read indicates the device_initialize call was to
> allow the struct device to be usable for reference counting. I think we
> should determine if that is the intent and can we call kobj_set_kset_s
> in device_initialize.
>
> Another option may be to set the ktype value on our struct device
> kobject until device_add overrides this with the call to kobj_set_kset_s.
I can confirm that moving kobj_set_kset_s fixes my boot up problem (with
your other patch). However, now I'm running into an obscure slab
corruption issue, so I suspect the move has to be more carefully
orchestrated.
James
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] scsi_host sysfs updates scsi-misc-2.5 [0/2]
2003-05-12 20:10 ` James Bottomley
@ 2003-05-12 20:35 ` Mike Anderson
2003-05-12 20:42 ` James Bottomley
0 siblings, 1 reply; 22+ messages in thread
From: Mike Anderson @ 2003-05-12 20:35 UTC (permalink / raw)
To: James Bottomley; +Cc: SCSI Mailing List, mochel
James Bottomley [James.Bottomley@steeleye.com] wrote:
> On Mon, 2003-05-12 at 13:41, Mike Anderson wrote:
> > I would like to have feedback from Mochel on the device_initialize
> > call.
> >
> > The documentation I have read indicates the device_initialize call was to
> > allow the struct device to be usable for reference counting. I think we
> > should determine if that is the intent and can we call kobj_set_kset_s
> > in device_initialize.
> >
> > Another option may be to set the ktype value on our struct device
> > kobject until device_add overrides this with the call to kobj_set_kset_s.
>
> I can confirm that moving kobj_set_kset_s fixes my boot up problem (with
> your other patch). However, now I'm running into an obscure slab
> corruption issue, so I suspect the move has to be more carefully
> orchestrated.
I checked out that setting kobj.ktype also seems to solve the problem
(using a hacked version of scsi_debug and calling scsi_register followed
by a call to scsi_unregister). This would give a more localized work
around, but device_initialize kobj_set_kset_s may be a better choice.
What slab size? (size-512)
-andmike
--
Michael Anderson
andmike@us.ibm.com
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] scsi_host sysfs updates scsi-misc-2.5 [0/2]
2003-05-12 20:35 ` Mike Anderson
@ 2003-05-12 20:42 ` James Bottomley
2003-05-12 20:53 ` James Bottomley
2003-05-12 21:49 ` Mike Anderson
0 siblings, 2 replies; 22+ messages in thread
From: James Bottomley @ 2003-05-12 20:42 UTC (permalink / raw)
To: Mike Anderson; +Cc: SCSI Mailing List, mochel
On Mon, 2003-05-12 at 15:35, Mike Anderson wrote:
> I checked out that setting kobj.ktype also seems to solve the problem
> (using a hacked version of scsi_debug and calling scsi_register followed
> by a call to scsi_unregister). This would give a more localized work
> around, but device_initialize kobj_set_kset_s may be a better choice.
Yes, that might fix it, setting the kset would cause a spurious
kset_put. I'll try it out.
> What slab size? (size-512)
size-4096
James
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] scsi_host sysfs updates scsi-misc-2.5 [0/2]
2003-05-12 20:42 ` James Bottomley
@ 2003-05-12 20:53 ` James Bottomley
2003-05-12 21:49 ` Mike Anderson
1 sibling, 0 replies; 22+ messages in thread
From: James Bottomley @ 2003-05-12 20:53 UTC (permalink / raw)
To: James Bottomley; +Cc: Mike Anderson, SCSI Mailing List, mochel
On Mon, 2003-05-12 at 15:42, James Bottomley wrote:
> On Mon, 2003-05-12 at 15:35, Mike Anderson wrote:
> > I checked out that setting kobj.ktype also seems to solve the problem
> > (using a hacked version of scsi_debug and calling scsi_register followed
> > by a call to scsi_unregister). This would give a more localized work
> > around, but device_initialize kobj_set_kset_s may be a better choice.
>
> Yes, that might fix it, setting the kset would cause a spurious
> kset_put. I'll try it out.
>
> > What slab size? (size-512)
>
> size-4096
well, just setting the kset still gives me my check_poison_obj use after
free. I'll see if I can find it.
James
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] scsi_host sysfs updates scsi-misc-2.5 [0/2]
2003-05-12 20:42 ` James Bottomley
2003-05-12 20:53 ` James Bottomley
@ 2003-05-12 21:49 ` Mike Anderson
2003-05-12 21:50 ` James Bottomley
1 sibling, 1 reply; 22+ messages in thread
From: Mike Anderson @ 2003-05-12 21:49 UTC (permalink / raw)
To: James Bottomley; +Cc: SCSI Mailing List, mochel
James Bottomley [James.Bottomley@steeleye.com] wrote:
> > What slab size? (size-512)
>
> size-4096
>
I am seeing size-512 with my modified version of scsi_debug. I modified
slab.c to store last user for this size and it indicates it was
scsi_free_shost. I am looking at this now.
-andmike
--
Michael Anderson
andmike@us.ibm.com
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] scsi_host sysfs updates scsi-misc-2.5 [0/2]
2003-05-12 21:49 ` Mike Anderson
@ 2003-05-12 21:50 ` James Bottomley
2003-05-12 22:15 ` Mike Anderson
0 siblings, 1 reply; 22+ messages in thread
From: James Bottomley @ 2003-05-12 21:50 UTC (permalink / raw)
To: Mike Anderson; +Cc: SCSI Mailing List, mochel
On Mon, 2003-05-12 at 16:49, Mike Anderson wrote:
> I am seeing size-512 with my modified version of scsi_debug. I modified
> slab.c to store last user for this size and it indicates it was
> scsi_free_shost. I am looking at this now.
I found it: you have a use after free in the sysfs code:
scsi_host_put does put_device followed by class_device_put, but the
put_device will free the shost containing the class_device in it's
release, so the class_device_put touches a freed object.
The solution is just to reverse the order of the puts.
James
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] scsi_host sysfs updates scsi-misc-2.5 [0/2]
2003-05-12 21:50 ` James Bottomley
@ 2003-05-12 22:15 ` Mike Anderson
0 siblings, 0 replies; 22+ messages in thread
From: Mike Anderson @ 2003-05-12 22:15 UTC (permalink / raw)
To: James Bottomley; +Cc: SCSI Mailing List, mochel
James Bottomley [James.Bottomley@steeleye.com] wrote:
> On Mon, 2003-05-12 at 16:49, Mike Anderson wrote:
> > I am seeing size-512 with my modified version of scsi_debug. I modified
> > slab.c to store last user for this size and it indicates it was
> > scsi_free_shost. I am looking at this now.
>
> I found it: you have a use after free in the sysfs code:
>
> scsi_host_put does put_device followed by class_device_put, but the
> put_device will free the shost containing the class_device in it's
> release, so the class_device_put touches a freed object.
>
> The solution is just to reverse the order of the puts.
Thanks for finding this. This fixes my slab issue also.
-andmike
--
Michael Anderson
andmike@us.ibm.com
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] scsi_host sysfs updates scsi-misc-2.5 [0/2]
2003-05-12 15:15 ` Andrew Vasquez
@ 2003-05-13 18:51 ` Mike Anderson
0 siblings, 0 replies; 22+ messages in thread
From: Mike Anderson @ 2003-05-13 18:51 UTC (permalink / raw)
To: Andrew Vasquez, linux-scsi
Andrew Vasquez [praka@san.rr.com] wrote:
> On Thu, 08 May 2003, Mike Anderson wrote:
>
> > This series of patches is an update to the scsi_host sysfs / ref
> > counting patches previously merged into the scsi-misc-2.5 tree.
> >
> > insmod / rmmod Testing:
> > Legacy registration:
> > aic7xxx_old, qlogicisp
> > qla2xxx v8.00.00b1 - Driver oops system with garbled
> > messages on rmmod. Does not seem related to this patch.
> >
>
> Is the driver still oops'ng during your testing? Could you provide
> some specifics on attached storage as well as environment (fabric,
> loop, hba type)? I'll resync my working tree with scsi-misc and
> perform some additional investigation.
The driver is still oops'ng on current scsi-misc-2.5, but I tried
linux-2.5 bk 2.5.69+ (change set 1.1095) and it did not oops after
multiple insmod/rmmod cycles.
I do not get consistent oops, but the one below occurs frequently on
rmmod. I will track it down more on my system and see if there are
changes between linux-2.5 and scsi-misc that would address what I am
seeing.
My configuration is 2 qla2300, IBM 2109 switch, IBM FASTt 200, 10 FC
JBOD disks.
-andmike
--
Michael Anderson
andmike@us.ibm.com
Unable to handle kernel paging request at virtual address d08a9110
printing eip:
d08a9110
*pde = 00000000
Oops: 0000 [#1]
CPU: 1
EIP: 0060:[<d08a9110>] Not tainted
EFLAGS: 00010246
EIP is at 0xd08a9110
eax: 00000000 ebx: c12ba000 ecx: cb967948 edx: c12bbf24
esi: cb9641c0 edi: c1287f80 ebp: c1287f80 esp: c12bbf10
ds: 007b es: 007b ss: 0068
Process swapper (pid: 0, threadinfo=c12ba000 task=c12bc6a<10>)Un
abSlet atcko: h acn0dl1e2 d1kce0r ncebl 9p6a4g1cin0g rce12bqbuefs24t
a<t4> vci12rbtbuaf2l4 add0dr8eas9s11 d00 8ca9c128510f
8 p<4r>icnct2i8n5gf e48 ip:0
000d0080a19 11<40> 0
* pd e = bf<4ff>fcf03699
0U8n abflffe ftfof fdha nd0l0e00 k00er46n elc 0pa12gi8cn6gb
recq03u9es2ta0 8 at <v4i>0r0tu00a0l0 a0d4 dr0e0ssed 70aff8f0 f2ca412
b f7prci n<ti4>ng b
e i p :
c0<14>1cd0012078
a0* p<d4e>c 0=1 1a5953ba50a 5a0
00fff cdc22084 c0391f00 ce560080 c01089a0 c01089a0
Call Trace:
[<c012d1c0>] run_timer_softirq+0x1c0/0x270
[<c0128c6b>] do_softirq+0x6b/0xd0
[<c01089a0>] default_idle+0x0/0x40
[<c01193b0>] smp_apic_timer_interrupt+0x140/0x160
[<c01089a0>] default_idle+0x0/0x40
[<c01089a0>] default_idle+0x0/0x40
[<c010b8ce>] apic_timer_interrupt+0x1a/0x20
[<c01089a0>] default_idle+0x0/0x40
[<c01089a0>] default_idle+0x0/0x40
[<c01089ca>] default_idle+0x2a/0x40
[<c0108a72>] cpu_idle+0x52/0x70
[<c0125533>] release_console_sem+0xd3/0x190
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] scsi_host sysfs updates scsi-misc-2.5 [0/2]
2003-05-12 18:41 ` Mike Anderson
2003-05-12 20:10 ` James Bottomley
@ 2003-05-14 0:00 ` Patrick Mochel
1 sibling, 0 replies; 22+ messages in thread
From: Patrick Mochel @ 2003-05-14 0:00 UTC (permalink / raw)
To: Mike Anderson; +Cc: James Bottomley, SCSI Mailing List
On Mon, 12 May 2003, Mike Anderson wrote:
> James Bottomley [James.Bottomley@steeleye.com] wrote:
> > On Mon, 2003-05-12 at 12:50, James Bottomley wrote:
> > > Surprisingly, it still fails. I've debugged the refcounting and I see
> > > it fall to zero in scsi_host_put but the dev release method doesn't
> > > trigger for no reason I can see.
> > >
> > > It's probably a kobject initialisation problem somewhere.
> >
> > OK, found it. the ->release is triggered by the kobj_kset, which isn't
> > set until device_add time, not device init time.
> >
> > Therefore, we shouldn't really put and get objects until they've been
> > added (or the kobj_set_kset_s needs to be moved to device_initialize()).
> >
>
> I would like to have feedback from Mochel on the device_initialize
> call.
Just to set the record straight - James suggested this on irc yesterday,
and I've added it to my tree. I haven't seen any issues with it, and based
on later emails, you've worked out the slab issue. So, it'll be pushed to
Linus.
-pat
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2003-05-13 23:47 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2003-05-09 6:33 [PATCH] scsi_host sysfs updates scsi-misc-2.5 [0/2] Mike Anderson
2003-05-09 6:34 ` [PATCH] scsi_host sysfs updates scsi-misc-2.5 [1/2] Mike Anderson
2003-05-09 6:35 ` [PATCH] scsi_host sysfs updates scsi-misc-2.5 [2/2] Mike Anderson
2003-05-09 6:59 ` Christoph Hellwig
2003-05-09 7:50 ` Mike Anderson
2003-05-09 8:21 ` Mike Anderson
2003-05-12 3:57 ` [PATCH] scsi_host sysfs updates scsi-misc-2.5 [0/2] James Bottomley
2003-05-12 6:38 ` Mike Anderson
2003-05-12 17:50 ` James Bottomley
2003-05-12 17:59 ` James Bottomley
2003-05-12 18:41 ` Mike Anderson
2003-05-12 20:10 ` James Bottomley
2003-05-12 20:35 ` Mike Anderson
2003-05-12 20:42 ` James Bottomley
2003-05-12 20:53 ` James Bottomley
2003-05-12 21:49 ` Mike Anderson
2003-05-12 21:50 ` James Bottomley
2003-05-12 22:15 ` Mike Anderson
2003-05-14 0:00 ` Patrick Mochel
2003-05-12 18:18 ` Mike Anderson
2003-05-12 15:15 ` Andrew Vasquez
2003-05-13 18:51 ` Mike Anderson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox