[PATCHSET 0/5] osd: Revised patches for the 2.6.33 merge window

[PATCHSET 0/5] osd: Revised patches for the 2.6.33 merge window

[Boaz Harrosh]

Hi James

I've revisited my last patches, and am submitting a new set. They address
the concern you had that I was searching for devices by /dev/* names where
actually user-mode can change those.

Additionally there is a related bugfix.

These are the patches

[PATCH 1/5] libosd: osd_dev_is_ver1 - Minor API cleanup
[PATCH 2/5] libosd: osd_sense: OSD_CFO_PERMISSIONS
	Same as before, please submit regardless.

[PATCH 3/5] osduld: Ref-counting bug fix
	This is a bug fix. In the long past it used to work, by chance.
	With this fix it does what it's suppose to do.
	(Please submit)

[PATCH 4/5] osduld: Use device->release instead of internal kref
[PATCH 5/5] libosd: osd_dev_info: Unique Identification of an OSD device
	These two patches are new implementation to the API introduced in previous
	patchset. (Plus a related addition needed by multy-device exofs support).

	It will now use the class_find_device() API to iterate through devices looking
	for the requested one. Using class_find_device forced a ref-counting shift.
	(Please review and submit)

I've tested these patches with multy-device exofs installation also exported to
pnfs-objects clients (also in multi-device) and I'm satisfied with the results.
There are no apparent new instabilities.

I would want to submit a substantial patchset to exofs, supporting multiple devices,
mirror operations. The exofs patches are dependent on these patches to osd

[I'm putting these patches and a rebased set of exofs in linux-next. There should be
 no problems merging once they get accepted into scsi-misc. As these patches are
 based on by exofs and pnfs-objects]

Thanks
Boaz

[PATCH 1/5] libosd: osd_dev_is_ver1 - Minor API cleanup

[Boaz Harrosh]

define a new osd_dev_is_ver1 that operates on devices
and the old osd_req_is_ver1 uses that new API.

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 include/scsi/osd_initiator.h |   16 ++++++++++------
 1 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/include/scsi/osd_initiator.h b/include/scsi/osd_initiator.h
index 02bd9f7..f787d24 100644
--- a/include/scsi/osd_initiator.h
+++ b/include/scsi/osd_initiator.h
@@ -84,6 +84,15 @@ static inline void osd_dev_set_ver(struct osd_dev *od, enum osd_std_version v)
 #endif
 }
 
+static inline bool osd_dev_is_ver1(struct osd_dev *od)
+{
+#ifdef OSD_VER1_SUPPORT
+	return od->version == OSD_VER1;
+#else
+	return false;
+#endif
+}
+
 struct osd_request;
 typedef void (osd_req_done_fn)(struct osd_request *or, void *private);
 
@@ -120,14 +129,9 @@ struct osd_request {
 	int async_error;
 };
 
-/* OSD Version control */
 static inline bool osd_req_is_ver1(struct osd_request *or)
 {
-#ifdef OSD_VER1_SUPPORT
-	return or->osd_dev->version == OSD_VER1;
-#else
-	return false;
-#endif
+	return osd_dev_is_ver1(or->osd_dev);
 }
 
 /*
-- 
1.6.5.1

[PATCH 2/5] libosd: osd_sense: OSD_CFO_PERMISSIONS

[Boaz Harrosh]

Add one more important cdb_field_offset that can be returned with
scsi_invalid_field_in_cdb. It is the offset of the permissions_bit_mask
field in the capabilities structure.

Interestingly, the offset is the same for V1/V2

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 include/scsi/osd_sense.h |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/include/scsi/osd_sense.h b/include/scsi/osd_sense.h
index ff9b33c..91db543 100644
--- a/include/scsi/osd_sense.h
+++ b/include/scsi/osd_sense.h
@@ -255,6 +255,9 @@ enum osdv2_cdb_field_offset {
 	OSD_CFO_STARTING_BYTE	= OSD_CDB_OFFSET(v2.start_address),
 	OSD_CFO_PARTITION_ID	= OSD_CDB_OFFSET(partition),
 	OSD_CFO_OBJECT_ID	= OSD_CDB_OFFSET(object),
+	OSD_CFO_PERMISSIONS	= sizeof(struct osd_cdb_head) +
+					offsetof(struct osd_capability_head,
+						 permissions_bit_mask),
 };
 
 #endif /* ndef __OSD_SENSE_H__ */
-- 
1.6.5.1

[PATCH 3/5] osduld: Ref-counting bug fix

[Boaz Harrosh]

If scsi has released the device (logout), and exofs has last
reference on the osduld_device it will be freed by
osd_uld_release() within the call to fput(). But this will
oops in cdev_release() which is called after the fops->release.
(cdev is embedded within osduld_device). __uld_get/put pair
makes sure we have a cdev for the duration of fput()

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 drivers/scsi/osd/osd_uld.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/osd/osd_uld.c b/drivers/scsi/osd/osd_uld.c
index 0bdef33..1ea6447 100644
--- a/drivers/scsi/osd/osd_uld.c
+++ b/drivers/scsi/osd/osd_uld.c
@@ -224,7 +224,15 @@ void osduld_put_device(struct osd_dev *od)
 
 		BUG_ON(od->scsi_device != oud->od.scsi_device);
 
+		/* If scsi has released the device (logout), and exofs has last
+		 * reference on oud it will be freed by above osd_uld_release
+		 * within fput below. But this will oops in cdev_release which
+		 * is called after the fops->release. __uld_get/put pair makes
+		 * sure we have a cdev for the duration of fput
+		 */
+		__uld_get(oud);
 		fput(od->file);
+		__uld_put(oud);
 		kfree(od);
 	}
 }
-- 
1.6.5.1

[PATCH 4/5] osduld: Use device->release instead of internal kref

[Boaz Harrosh]

The true logic of this patch will be clear in the next patch where we
use the class_find_device???() API. When doing so the use of an internal
kref leaves us a narrow window where a find is started while the actual
object can go away. Using the device's kobj reference solves this problem
because now the same kref is used for both operations. (Remove and find)

Some cleanups
* Use class register/unregister is cleaner for this driver now.
* Call create_device with the right parameters, and save calling
  set_drv_data() (It was not a bug just a side effect)
* cdev ref-counting games are no longer necessary

Core changes
* At create_device (in osd_probe), returned device->release is saved in an internal
  member and the final __remove function is now registered as the device->release
  function. (__Remove was moved in the file to avoid forward declaration)
* At __remove() chain to the old device->release() to de-allocate the device
  created by create_device.
* __uld_get/put is just device_get/put. Now every thing is accounted for on the
  device object.

I have incremented the device version string in case of new bugs.

Note: that previous bugfix of taking the reference around fput() still applies.

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 drivers/scsi/osd/osd_uld.c   |  129 ++++++++++++++++++++++--------------------
 include/scsi/osd_initiator.h |    1 -
 2 files changed, 67 insertions(+), 63 deletions(-)

diff --git a/drivers/scsi/osd/osd_uld.c b/drivers/scsi/osd/osd_uld.c
index 1ea6447..7ec12ef 100644
--- a/drivers/scsi/osd/osd_uld.c
+++ b/drivers/scsi/osd/osd_uld.c
@@ -71,8 +71,7 @@
 #define SCSI_OSD_MAX_MINOR 64
 
 static const char osd_name[] = "osd";
-static const char *osd_version_string = "open-osd 0.1.0";
-const char osd_symlink[] = "scsi_osd";
+static const char *osd_version_string = "open-osd 0.2.0";
 
 MODULE_AUTHOR("Boaz Harrosh <bharrosh@panasas.com>");
 MODULE_DESCRIPTION("open-osd Upper-Layer-Driver osd.ko");
@@ -80,18 +79,33 @@ MODULE_LICENSE("GPL");
 MODULE_ALIAS_CHARDEV_MAJOR(SCSI_OSD_MAJOR);
 MODULE_ALIAS_SCSI_DEVICE(TYPE_OSD);
 
+#define OSD_FMT_d "osd%d"
+
 struct osd_uld_device {
 	int minor;
-	struct kref kref;
 	struct cdev cdev;
 	struct osd_dev od;
 	struct gendisk *disk;
 	struct device *class_member;
+	typeof(((struct device *)NULL)->release) save_release;
 };
 
+struct osd_dev_handle {
+	struct osd_dev od;
+	struct file *file;
+	struct osd_uld_device *oud;
+} ;
+
+static DEFINE_IDA(osd_minor_ida);
+
 static void __uld_get(struct osd_uld_device *oud);
 static void __uld_put(struct osd_uld_device *oud);
 
+static struct class osd_uld_class = {
+	.owner		= THIS_MODULE,
+	.name		= "scsi_osd",
+};
+
 /*
  * Char Device operations
  */
@@ -177,7 +191,7 @@ static const struct file_operations osd_fops = {
 struct osd_dev *osduld_path_lookup(const char *name)
 {
 	struct osd_uld_device *oud;
-	struct osd_dev *od;
+	struct osd_dev_handle *odh;
 	struct file *file;
 	int error;
 
@@ -186,8 +200,8 @@ struct osd_dev *osduld_path_lookup(const char *name)
 		return ERR_PTR(-EINVAL);
 	}
 
-	od = kzalloc(sizeof(*od), GFP_KERNEL);
-	if (!od)
+	odh = kzalloc(sizeof(*odh), GFP_KERNEL);
+	if (unlikely(!odh))
 		return ERR_PTR(-ENOMEM);
 
 	file = filp_open(name, O_RDWR, 0);
@@ -203,24 +217,26 @@ struct osd_dev *osduld_path_lookup(const char *name)
 
 	oud = file->private_data;
 
-	*od = oud->od;
-	od->file = file;
+	odh->od = oud->od;
+	odh->file = file;
+	odh->oud = oud;
 
-	return od;
+	return &odh->od;
 
 close_file:
 	fput(file);
 free_od:
-	kfree(od);
+	kfree(odh);
 	return ERR_PTR(error);
 }
 EXPORT_SYMBOL(osduld_path_lookup);
 
 void osduld_put_device(struct osd_dev *od)
 {
-
 	if (od && !IS_ERR(od)) {
-		struct osd_uld_device *oud = od->file->private_data;
+		struct osd_dev_handle *odh =
+				container_of(od, struct osd_dev_handle, od);
+		struct osd_uld_device *oud = odh->oud;
 
 		BUG_ON(od->scsi_device != oud->od.scsi_device);
 
@@ -231,9 +247,9 @@ void osduld_put_device(struct osd_dev *od)
 		 * sure we have a cdev for the duration of fput
 		 */
 		__uld_get(oud);
-		fput(od->file);
+		fput(odh->file);
 		__uld_put(oud);
-		kfree(od);
+		kfree(odh);
 	}
 }
 EXPORT_SYMBOL(osduld_put_device);
@@ -264,8 +280,28 @@ static int __detect_osd(struct osd_uld_device *oud)
 	return 0;
 }
 
-static struct class *osd_sysfs_class;
-static DEFINE_IDA(osd_minor_ida);
+static void __remove(struct device *dev)
+{
+	struct osd_uld_device *oud = dev_get_drvdata(dev);
+	struct scsi_device *scsi_device = oud->od.scsi_device;
+
+	oud->save_release(oud->class_member);
+
+	if (oud->cdev.owner)
+		cdev_del(&oud->cdev);
+
+	osd_dev_fini(&oud->od);
+	scsi_device_put(scsi_device);
+
+	OSD_INFO("osd_remove %s\n",
+		 oud->disk ? oud->disk->disk_name : NULL);
+
+	if (oud->disk)
+		put_disk(oud->disk);
+	ida_remove(&osd_minor_ida, oud->minor);
+
+	kfree(oud);
+}
 
 static int osd_probe(struct device *dev)
 {
@@ -297,7 +333,6 @@ static int osd_probe(struct device *dev)
 	if (NULL == oud)
 		goto err_retract_minor;
 
-	kref_init(&oud->kref);
 	dev_set_drvdata(dev, oud);
 	oud->minor = minor;
 
@@ -310,7 +345,7 @@ static int osd_probe(struct device *dev)
 	}
 	disk->major = SCSI_OSD_MAJOR;
 	disk->first_minor = oud->minor;
-	sprintf(disk->disk_name, "osd%d", oud->minor);
+	sprintf(disk->disk_name, OSD_FMT_d, oud->minor);
 	oud->disk = disk;
 
 	/* hold one more reference to the scsi_device that will get released
@@ -335,18 +370,19 @@ static int osd_probe(struct device *dev)
 		OSD_ERR("cdev_add failed\n");
 		goto err_put_disk;
 	}
-	kobject_get(&oud->cdev.kobj); /* 2nd ref see osd_remove() */
 
 	/* class_member */
-	oud->class_member = device_create(osd_sysfs_class, dev,
-		MKDEV(SCSI_OSD_MAJOR, oud->minor), "%s", disk->disk_name);
+	oud->class_member = device_create(&osd_uld_class, dev,
+		MKDEV(SCSI_OSD_MAJOR, oud->minor), oud, disk->disk_name);
 	if (IS_ERR(oud->class_member)) {
 		OSD_ERR("class_device_create failed\n");
 		error = PTR_ERR(oud->class_member);
 		goto err_put_cdev;
 	}
+	oud->save_release = oud->class_member->release;
+	oud->class_member->release = __remove;
 
-	dev_set_drvdata(oud->class_member, oud);
+	__uld_get(oud);
 
 	OSD_INFO("osd_probe %s\n", disk->disk_name);
 	return 0;
@@ -376,51 +412,21 @@ static int osd_remove(struct device *dev)
 	}
 
 	if (oud->class_member)
-		device_destroy(osd_sysfs_class,
+		device_destroy(&osd_uld_class,
 			       MKDEV(SCSI_OSD_MAJOR, oud->minor));
 
-	/* We have 2 references to the cdev. One is released here
-	 * and also takes down the /dev/osdX mapping. The second
-	 * Will be released in __remove() after all users have released
-	 * the osd_uld_device.
-	 */
-	if (oud->cdev.owner)
-		cdev_del(&oud->cdev);
-
 	__uld_put(oud);
 	return 0;
 }
 
-static void __remove(struct kref *kref)
-{
-	struct osd_uld_device *oud = container_of(kref,
-					struct osd_uld_device, kref);
-	struct scsi_device *scsi_device = oud->od.scsi_device;
-
-	/* now let delete the char_dev */
-	kobject_put(&oud->cdev.kobj);
-
-	osd_dev_fini(&oud->od);
-	scsi_device_put(scsi_device);
-
-	OSD_INFO("osd_remove %s\n",
-		 oud->disk ? oud->disk->disk_name : NULL);
-
-	if (oud->disk)
-		put_disk(oud->disk);
-
-	ida_remove(&osd_minor_ida, oud->minor);
-	kfree(oud);
-}
-
 static void __uld_get(struct osd_uld_device *oud)
 {
-	kref_get(&oud->kref);
+	get_device(oud->class_member);
 }
 
 static void __uld_put(struct osd_uld_device *oud)
 {
-	kref_put(&oud->kref, __remove);
+	put_device(oud->class_member);
 }
 
 /*
@@ -440,11 +446,10 @@ static int __init osd_uld_init(void)
 {
 	int err;
 
-	osd_sysfs_class = class_create(THIS_MODULE, osd_symlink);
-	if (IS_ERR(osd_sysfs_class)) {
-		OSD_ERR("Unable to register sysfs class => %ld\n",
-			PTR_ERR(osd_sysfs_class));
-		return PTR_ERR(osd_sysfs_class);
+	err = class_register(&osd_uld_class);
+	if (err) {
+		OSD_ERR("Unable to register sysfs class => %d\n", err);
+		return err;
 	}
 
 	err = register_chrdev_region(MKDEV(SCSI_OSD_MAJOR, 0),
@@ -467,7 +472,7 @@ static int __init osd_uld_init(void)
 err_out_chrdev:
 	unregister_chrdev_region(MKDEV(SCSI_OSD_MAJOR, 0), SCSI_OSD_MAX_MINOR);
 err_out:
-	class_destroy(osd_sysfs_class);
+	class_unregister(&osd_uld_class);
 	return err;
 }
 
@@ -475,7 +480,7 @@ static void __exit osd_uld_exit(void)
 {
 	scsi_unregister_driver(&osd_driver.gendrv);
 	unregister_chrdev_region(MKDEV(SCSI_OSD_MAJOR, 0), SCSI_OSD_MAX_MINOR);
-	class_destroy(osd_sysfs_class);
+	class_unregister(&osd_uld_class);
 	OSD_INFO("UNLOADED %s\n", osd_version_string);
 }
 
diff --git a/include/scsi/osd_initiator.h b/include/scsi/osd_initiator.h
index f787d24..589e5f0 100644
--- a/include/scsi/osd_initiator.h
+++ b/include/scsi/osd_initiator.h
@@ -48,7 +48,6 @@ enum osd_std_version {
  */
 struct osd_dev {
 	struct scsi_device *scsi_device;
-	struct file *file;
 	unsigned def_timeout;
 
 #ifdef OSD_VER1_SUPPORT
-- 
1.6.5.1

Re: [PATCH 4/5] osduld: Use device->release instead of internal kref

[James Bottomley]

On Mon, 2009-10-26 at 18:58 +0200, Boaz Harrosh wrote:
> @@ -335,18 +370,19 @@ static int osd_probe(struct device *dev)
>  		OSD_ERR("cdev_add failed\n");
>  		goto err_put_disk;
>  	}
> -	kobject_get(&oud->cdev.kobj); /* 2nd ref see osd_remove() */
>  
>  	/* class_member */
> -	oud->class_member = device_create(osd_sysfs_class, dev,
> -		MKDEV(SCSI_OSD_MAJOR, oud->minor), "%s", disk->disk_name);
> +	oud->class_member = device_create(&osd_uld_class, dev,
> +		MKDEV(SCSI_OSD_MAJOR, oud->minor), oud, disk->disk_name);
>  	if (IS_ERR(oud->class_member)) {
>  		OSD_ERR("class_device_create failed\n");
>  		error = PTR_ERR(oud->class_member);
>  		goto err_put_cdev;
>  	}
> +	oud->save_release = oud->class_member->release;
> +	oud->class_member->release = __remove;

What exactly is the reason for this iffy manipulation?  Can't this be
done properly by adding a dev_release member to osd_uld_class without
this unnecessary and improper function chaining?

James

Re: [PATCH 4/5] osduld: Use device->release instead of internal kref

[Boaz Harrosh]

On 10/29/2009 07:11 PM, James Bottomley wrote:
> On Mon, 2009-10-26 at 18:58 +0200, Boaz Harrosh wrote:
>> @@ -335,18 +370,19 @@ static int osd_probe(struct device *dev)
>>  		OSD_ERR("cdev_add failed\n");
>>  		goto err_put_disk;
>>  	}
>> -	kobject_get(&oud->cdev.kobj); /* 2nd ref see osd_remove() */
>>  
>>  	/* class_member */
>> -	oud->class_member = device_create(osd_sysfs_class, dev,
>> -		MKDEV(SCSI_OSD_MAJOR, oud->minor), "%s", disk->disk_name);
>> +	oud->class_member = device_create(&osd_uld_class, dev,
>> +		MKDEV(SCSI_OSD_MAJOR, oud->minor), oud, disk->disk_name);
>>  	if (IS_ERR(oud->class_member)) {
>>  		OSD_ERR("class_device_create failed\n");
>>  		error = PTR_ERR(oud->class_member);
>>  		goto err_put_cdev;
>>  	}
>> +	oud->save_release = oud->class_member->release;
>> +	oud->class_member->release = __remove;
> 
> What exactly is the reason for this iffy manipulation?  Can't this be
> done properly by adding a dev_release member to osd_uld_class without
> this unnecessary and improper function chaining?
> 

I tried, it does not work if I use device_create.

because in device_release(struct kobject *kobj) it has ruffly this:
	if (dev->release)
		dev->release(dev);
	else if (class->dev_release)
		class->dev_release(dev);

(See core.c:110)

so since device_create sets a dev->release = it masks out my class->dev_release

Now I have two choice. Above release chaining, or duplicate the 9 lines of code
done in device_create() before the call to device_register. I prefer the first
since it is more resilient to future core changes. (And it is less code lines)

> James
> 
> 

Boaz

Re: [PATCH 4/5] osduld: Use device->release instead of internal kref

[James Bottomley]

On Thu, 2009-10-29 at 19:24 +0200, Boaz Harrosh wrote:
> On 10/29/2009 07:11 PM, James Bottomley wrote:
> > On Mon, 2009-10-26 at 18:58 +0200, Boaz Harrosh wrote:
> >> @@ -335,18 +370,19 @@ static int osd_probe(struct device *dev)
> >>  		OSD_ERR("cdev_add failed\n");
> >>  		goto err_put_disk;
> >>  	}
> >> -	kobject_get(&oud->cdev.kobj); /* 2nd ref see osd_remove() */
> >>  
> >>  	/* class_member */
> >> -	oud->class_member = device_create(osd_sysfs_class, dev,
> >> -		MKDEV(SCSI_OSD_MAJOR, oud->minor), "%s", disk->disk_name);
> >> +	oud->class_member = device_create(&osd_uld_class, dev,
> >> +		MKDEV(SCSI_OSD_MAJOR, oud->minor), oud, disk->disk_name);
> >>  	if (IS_ERR(oud->class_member)) {
> >>  		OSD_ERR("class_device_create failed\n");
> >>  		error = PTR_ERR(oud->class_member);
> >>  		goto err_put_cdev;
> >>  	}
> >> +	oud->save_release = oud->class_member->release;
> >> +	oud->class_member->release = __remove;
> > 
> > What exactly is the reason for this iffy manipulation?  Can't this be
> > done properly by adding a dev_release member to osd_uld_class without
> > this unnecessary and improper function chaining?
> > 
> 
> I tried, it does not work if I use device_create.
> 
> because in device_release(struct kobject *kobj) it has ruffly this:
> 	if (dev->release)
> 		dev->release(dev);
> 	else if (class->dev_release)
> 		class->dev_release(dev);
> 
> (See core.c:110)
> 
> so since device_create sets a dev->release = it masks out my class->dev_release
> 
> Now I have two choice. Above release chaining, or duplicate the 9 lines of code
> done in device_create() before the call to device_register. I prefer the first
> since it is more resilient to future core changes. (And it is less code lines)

Chaining methods like this because of inner knowledge of the
implementation isn't resilient, it's very fragile.

Isn't the correct answer to embed your device in struct osd_uld_device?
They both look to have identical lifetimes and the release rules can
then handle the dual destruction?

James

Re: [PATCH 4/5] osduld: Use device->release instead of internal kref

[Boaz Harrosh]

Some thing like below. 

There is to much knolage of device in my opinion. To much fidling
of private device members. What when some new members are added?

I hate it.

(compile tested only)
---
git diff --stat -p drivers/scsi/osd/osd_uld.c
 drivers/scsi/osd/osd_uld.c |   35 ++++++++++++++++++-----------------
 1 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/drivers/scsi/osd/osd_uld.c b/drivers/scsi/osd/osd_uld.c
index 25ef993..5f308be 100644
--- a/drivers/scsi/osd/osd_uld.c
+++ b/drivers/scsi/osd/osd_uld.c
@@ -87,7 +87,7 @@ struct osd_uld_device {
 	struct osd_dev od;
 	struct osd_dev_info odi;
 	struct gendisk *disk;
-	struct device *class_member;
+	struct device class_dev;
 	typeof(((struct device *)NULL)->release) save_release;
 };
 
@@ -316,7 +316,7 @@ void osduld_put_device(struct osd_dev *od)
 		if (odh->file)
 			fput(odh->file);
 		else
-			put_device(oud->class_member);
+			put_device(&oud->class_dev);
 		__uld_put(oud);
 		kfree(odh);
 	}
@@ -378,7 +378,6 @@ static void __remove(struct device *dev)
 	struct scsi_device *scsi_device = oud->od.scsi_device;
 
 	kfree(oud->odi.osdname);
-	oud->save_release(oud->class_member);
 
 	if (oud->cdev.owner)
 		cdev_del(&oud->cdev);
@@ -464,16 +463,20 @@ static int osd_probe(struct device *dev)
 		goto err_put_disk;
 	}
 
-	/* class_member */
-	oud->class_member = device_create(&osd_uld_class, dev,
-		MKDEV(SCSI_OSD_MAJOR, oud->minor), oud, disk->disk_name);
-	if (IS_ERR(oud->class_member)) {
-		OSD_ERR("class_device_create failed\n");
-		error = PTR_ERR(oud->class_member);
+	/* class device member */
+	oud->class_dev.devt = MKDEV(SCSI_OSD_MAJOR, oud->minor);
+	oud->class_dev.class = &osd_uld_class;
+	oud->class_dev.parent = dev;
+	oud->class_dev.release = __remove;
+	dev_set_drvdata(&oud->class_dev, oud);
+
+	error = kobject_set_name(&oud->class_dev.kobj, disk->disk_name);
+	if (error)
+		goto err_put_cdev;
+
+	error = device_register(&oud->class_dev);
+	if (error)
 		goto err_put_cdev;
-	}
-	oud->save_release = oud->class_member->release;
-	oud->class_member->release = __remove;
 
 	__uld_get(oud);
 
@@ -504,9 +507,7 @@ static int osd_remove(struct device *dev)
 			scsi_device);
 	}
 
-	if (oud->class_member)
-		device_destroy(&osd_uld_class,
-			       MKDEV(SCSI_OSD_MAJOR, oud->minor));
+	device_unregister(&oud->class_dev);
 
 	__uld_put(oud);
 	return 0;
@@ -514,12 +515,12 @@ static int osd_remove(struct device *dev)
 
 static void __uld_get(struct osd_uld_device *oud)
 {
-	get_device(oud->class_member);
+	get_device(&oud->class_dev);
 }
 
 static void __uld_put(struct osd_uld_device *oud)
 {
-	put_device(oud->class_member);
+	put_device(&oud->class_dev);
 }
 
 /*

Re: [PATCH 4/5] osduld: Use device->release instead of internal kref

[James Bottomley]

On Thu, 2009-10-29 at 19:50 +0200, Boaz Harrosh wrote:
> Some thing like below. 
> 
> There is to much knolage of device in my opinion. To much fidling
> of private device members. What when some new members are added?

What private device members ... the pieces you fill in are the public
ones.  If you want a macro to do that, I'm sure Greg would be
amenable ...

> I hate it.

Because you have to type six more lines of code?  To me it looks cleaner
because there's only a single allocation instead of two and now no need
of chaining destructors because everything goes together thus making the
lifetime rules more obvious and robust.

It also saves memory because it has two fewer pointers.

James


> (compile tested only)
> ---
> git diff --stat -p drivers/scsi/osd/osd_uld.c
>  drivers/scsi/osd/osd_uld.c |   35 ++++++++++++++++++-----------------
>  1 files changed, 18 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/scsi/osd/osd_uld.c b/drivers/scsi/osd/osd_uld.c
> index 25ef993..5f308be 100644
> --- a/drivers/scsi/osd/osd_uld.c
> +++ b/drivers/scsi/osd/osd_uld.c
> @@ -87,7 +87,7 @@ struct osd_uld_device {
>  	struct osd_dev od;
>  	struct osd_dev_info odi;
>  	struct gendisk *disk;
> -	struct device *class_member;
> +	struct device class_dev;
>  	typeof(((struct device *)NULL)->release) save_release;
>  };
>  
> @@ -316,7 +316,7 @@ void osduld_put_device(struct osd_dev *od)
>  		if (odh->file)
>  			fput(odh->file);
>  		else
> -			put_device(oud->class_member);
> +			put_device(&oud->class_dev);
>  		__uld_put(oud);
>  		kfree(odh);
>  	}
> @@ -378,7 +378,6 @@ static void __remove(struct device *dev)
>  	struct scsi_device *scsi_device = oud->od.scsi_device;
>  
>  	kfree(oud->odi.osdname);
> -	oud->save_release(oud->class_member);
>  
>  	if (oud->cdev.owner)
>  		cdev_del(&oud->cdev);
> @@ -464,16 +463,20 @@ static int osd_probe(struct device *dev)
>  		goto err_put_disk;
>  	}
>  
> -	/* class_member */
> -	oud->class_member = device_create(&osd_uld_class, dev,
> -		MKDEV(SCSI_OSD_MAJOR, oud->minor), oud, disk->disk_name);
> -	if (IS_ERR(oud->class_member)) {
> -		OSD_ERR("class_device_create failed\n");
> -		error = PTR_ERR(oud->class_member);
> +	/* class device member */
> +	oud->class_dev.devt = MKDEV(SCSI_OSD_MAJOR, oud->minor);
> +	oud->class_dev.class = &osd_uld_class;
> +	oud->class_dev.parent = dev;
> +	oud->class_dev.release = __remove;
> +	dev_set_drvdata(&oud->class_dev, oud);
> +
> +	error = kobject_set_name(&oud->class_dev.kobj, disk->disk_name);
> +	if (error)
> +		goto err_put_cdev;
> +
> +	error = device_register(&oud->class_dev);
> +	if (error)
>  		goto err_put_cdev;
> -	}
> -	oud->save_release = oud->class_member->release;
> -	oud->class_member->release = __remove;
>  
>  	__uld_get(oud);
>  
> @@ -504,9 +507,7 @@ static int osd_remove(struct device *dev)
>  			scsi_device);
>  	}
>  
> -	if (oud->class_member)
> -		device_destroy(&osd_uld_class,
> -			       MKDEV(SCSI_OSD_MAJOR, oud->minor));
> +	device_unregister(&oud->class_dev);
>  
>  	__uld_put(oud);
>  	return 0;
> @@ -514,12 +515,12 @@ static int osd_remove(struct device *dev)
>  
>  static void __uld_get(struct osd_uld_device *oud)
>  {
> -	get_device(oud->class_member);
> +	get_device(&oud->class_dev);
>  }
>  
>  static void __uld_put(struct osd_uld_device *oud)
>  {
> -	put_device(oud->class_member);
> +	put_device(&oud->class_dev);
>  }
>  
>  /*
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/5] osduld: Use device->release instead of internal kref

[Boaz Harrosh]

On 10/29/2009 08:03 PM, James Bottomley wrote:
> On Thu, 2009-10-29 at 19:50 +0200, Boaz Harrosh wrote:
>> Some thing like below. 
>>
>> There is to much knolage of device in my opinion. To much fidling
>> of private device members. What when some new members are added?
> 
> What private device members ... the pieces you fill in are the public
> ones.  If you want a macro to do that, I'm sure Greg would be
> amenable ...
> 
>> I hate it.
> 
> Because you have to type six more lines of code?  To me it looks cleaner
> because there's only a single allocation instead of two and now no need
> of chaining destructors because everything goes together thus making the
> lifetime rules more obvious and robust.
> 
> It also saves memory because it has two fewer pointers.
> 
> James
> 

OK I'll roll up a patch that introduces a device_init helper
and a patch for Greg that exchanges my helper with an export.

But I will have to test it so only next week.

Thanks for the review. Is that the only thing?

Boaz

Re: [PATCH 4/5] osduld: Use device->release instead of internal kref

[Boaz Harrosh]

On 10/29/2009 07:50 PM, Boaz Harrosh wrote:
> Some thing like below. 
> 
> There is to much knolage of device in my opinion. To much fidling
> of private device members. What when some new members are added?
> 
> I hate it.
> 
> (compile tested only)
> ---
> git diff --stat -p drivers/scsi/osd/osd_uld.c
>  drivers/scsi/osd/osd_uld.c |   35 ++++++++++++++++++-----------------
>  1 files changed, 18 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/scsi/osd/osd_uld.c b/drivers/scsi/osd/osd_uld.c
> index 25ef993..5f308be 100644
> --- a/drivers/scsi/osd/osd_uld.c
> +++ b/drivers/scsi/osd/osd_uld.c
> @@ -87,7 +87,7 @@ struct osd_uld_device {
>  	struct osd_dev od;
>  	struct osd_dev_info odi;
>  	struct gendisk *disk;
> -	struct device *class_member;
> +	struct device class_dev;
>  	typeof(((struct device *)NULL)->release) save_release;
>  };
>  
> @@ -316,7 +316,7 @@ void osduld_put_device(struct osd_dev *od)
>  		if (odh->file)
>  			fput(odh->file);
>  		else
> -			put_device(oud->class_member);
> +			put_device(&oud->class_dev);
>  		__uld_put(oud);
>  		kfree(odh);
>  	}
> @@ -378,7 +378,6 @@ static void __remove(struct device *dev)
>  	struct scsi_device *scsi_device = oud->od.scsi_device;
>  
>  	kfree(oud->odi.osdname);
> -	oud->save_release(oud->class_member);
>  
>  	if (oud->cdev.owner)
>  		cdev_del(&oud->cdev);
> @@ -464,16 +463,20 @@ static int osd_probe(struct device *dev)
>  		goto err_put_disk;
>  	}
>  
> -	/* class_member */
> -	oud->class_member = device_create(&osd_uld_class, dev,
> -		MKDEV(SCSI_OSD_MAJOR, oud->minor), oud, disk->disk_name);
> -	if (IS_ERR(oud->class_member)) {
> -		OSD_ERR("class_device_create failed\n");
> -		error = PTR_ERR(oud->class_member);
> +	/* class device member */
> +	oud->class_dev.devt = MKDEV(SCSI_OSD_MAJOR, oud->minor);
> +	oud->class_dev.class = &osd_uld_class;
> +	oud->class_dev.parent = dev;
> +	oud->class_dev.release = __remove;
> +	dev_set_drvdata(&oud->class_dev, oud);
> +
> +	error = kobject_set_name(&oud->class_dev.kobj, disk->disk_name);
> +	if (error)
> +		goto err_put_cdev;
> +

With cdev (above) I have "cdev_init(&oud->cdev, &osd_fops);"
If I would only have a:
    device_init(&osd_uld_class, dev,MKDEV(SCSI_OSD_MAJOR, oud->minor),
		oud, disk->disk_name);

That does these code lines above I would feel much more comfortable

Boaz

> +	error = device_register(&oud->class_dev);
> +	if (error)
>  		goto err_put_cdev;
> -	}
> -	oud->save_release = oud->class_member->release;
> -	oud->class_member->release = __remove;
>  
>  	__uld_get(oud);
>  
> @@ -504,9 +507,7 @@ static int osd_remove(struct device *dev)
>  			scsi_device);
>  	}
>  
> -	if (oud->class_member)
> -		device_destroy(&osd_uld_class,
> -			       MKDEV(SCSI_OSD_MAJOR, oud->minor));
> +	device_unregister(&oud->class_dev);
>  
>  	__uld_put(oud);
>  	return 0;
> @@ -514,12 +515,12 @@ static int osd_remove(struct device *dev)
>  
>  static void __uld_get(struct osd_uld_device *oud)
>  {
> -	get_device(oud->class_member);
> +	get_device(&oud->class_dev);
>  }
>  
>  static void __uld_put(struct osd_uld_device *oud)
>  {
> -	put_device(oud->class_member);
> +	put_device(&oud->class_dev);
>  }
>  
>  /*
> 
> 
> 

Re: [PATCH 4/5] osduld: Use device->release instead of internal kref

[Boaz Harrosh]

On 10/29/2009 07:41 PM, James Bottomley wrote:
> 
> Chaining methods like this because of inner knowledge of the
> implementation isn't resilient, it's very fragile.
> 

If I would just kfree, because I know the inner code, that would be
fragile.

But overriding a destructor, do what you need, and call previous
distroctor. Does not take any inner knowledge. Just the published fact
that it is a distructor, which will destroy the object.

> Isn't the correct answer to embed your device in struct osd_uld_device?
> They both look to have identical lifetimes and the release rules can
> then handle the dual destruction?
> 

See the patch. there I have direct manipulation with inner members.
The chaining is more resilient, i think.

> James
> 
> 

Boaz

Re: [PATCH 4/5] osduld: Use device->release instead of internal kref

[James Bottomley]

On Thu, 2009-10-29 at 19:58 +0200, Boaz Harrosh wrote:
> On 10/29/2009 07:41 PM, James Bottomley wrote:
> > 
> > Chaining methods like this because of inner knowledge of the
> > implementation isn't resilient, it's very fragile.
> > 
> 
> If I would just kfree, because I know the inner code, that would be
> fragile.
> 
> But overriding a destructor, do what you need, and call previous
> distroctor. Does not take any inner knowledge. Just the published fact
> that it is a distructor, which will destroy the object.

You can't even justify this on OO grounds: In OO code, you get this
override by extending the object not hijacking the method and
arbitrarily linking two separate objects.

Embedding the device is the C equivalent of the OO object extension.

Method hijacks are almost always wrong.

James

[PATCH 4/5 version 2] osduld: Use device->release instead of internal kref

[Boaz Harrosh]

The true logic of this patch will be clear in the next patch where we
use the class_find_device() API. When doing so the use of an internal
kref leaves us a narrow window where a find is started while the actual
object can go away. Using the device's kobj reference solves this
problem because now the same kref is used for both operations. (Remove
and find)

Core changes
* Embed a struct device in uld_ structure and use device_register
  instead of devie_create. Set __remove to be the device release
  function.
* __uld_get/put is just get_/put_device. Now everything is accounted
  for on the device object. Internal kref is removed.
* At __remove() we can safely de-allocate the uld_ structure. (The
  function was moved to avoid forward declaration)

Some cleanups
* Use class register/unregister is cleaner for this driver now.
* cdev ref-counting games are no longer necessary

I have incremented the device version string in case of new bugs.

Note: Previous bugfix of taking the reference around fput() still
      applies.

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 drivers/scsi/osd/osd_uld.c   |  162 ++++++++++++++++++++----------------------
 include/scsi/osd_initiator.h |    1 -
 2 files changed, 77 insertions(+), 86 deletions(-)

diff --git a/drivers/scsi/osd/osd_uld.c b/drivers/scsi/osd/osd_uld.c
index 1ea6447..9dd5948 100644
--- a/drivers/scsi/osd/osd_uld.c
+++ b/drivers/scsi/osd/osd_uld.c
@@ -71,8 +71,7 @@
 #define SCSI_OSD_MAX_MINOR 64
 
 static const char osd_name[] = "osd";
-static const char *osd_version_string = "open-osd 0.1.0";
-const char osd_symlink[] = "scsi_osd";
+static const char *osd_version_string = "open-osd 0.2.0";
 
 MODULE_AUTHOR("Boaz Harrosh <bharrosh@panasas.com>");
 MODULE_DESCRIPTION("open-osd Upper-Layer-Driver osd.ko");
@@ -82,15 +81,24 @@ MODULE_ALIAS_SCSI_DEVICE(TYPE_OSD);
 
 struct osd_uld_device {
 	int minor;
-	struct kref kref;
+	struct device class_dev;
 	struct cdev cdev;
 	struct osd_dev od;
 	struct gendisk *disk;
-	struct device *class_member;
 };
 
-static void __uld_get(struct osd_uld_device *oud);
-static void __uld_put(struct osd_uld_device *oud);
+struct osd_dev_handle {
+	struct osd_dev od;
+	struct file *file;
+	struct osd_uld_device *oud;
+} ;
+
+static DEFINE_IDA(osd_minor_ida);
+
+static struct class osd_uld_class = {
+	.owner		= THIS_MODULE,
+	.name		= "scsi_osd",
+};
 
 /*
  * Char Device operations
@@ -101,7 +109,7 @@ static int osd_uld_open(struct inode *inode, struct file *file)
 	struct osd_uld_device *oud = container_of(inode->i_cdev,
 					struct osd_uld_device, cdev);
 
-	__uld_get(oud);
+	get_device(&oud->class_dev);
 	/* cache osd_uld_device on file handle */
 	file->private_data = oud;
 	OSD_DEBUG("osd_uld_open %p\n", oud);
@@ -114,7 +122,7 @@ static int osd_uld_release(struct inode *inode, struct file *file)
 
 	OSD_DEBUG("osd_uld_release %p\n", file->private_data);
 	file->private_data = NULL;
-	__uld_put(oud);
+	put_device(&oud->class_dev);
 	return 0;
 }
 
@@ -177,7 +185,7 @@ static const struct file_operations osd_fops = {
 struct osd_dev *osduld_path_lookup(const char *name)
 {
 	struct osd_uld_device *oud;
-	struct osd_dev *od;
+	struct osd_dev_handle *odh;
 	struct file *file;
 	int error;
 
@@ -186,8 +194,8 @@ struct osd_dev *osduld_path_lookup(const char *name)
 		return ERR_PTR(-EINVAL);
 	}
 
-	od = kzalloc(sizeof(*od), GFP_KERNEL);
-	if (!od)
+	odh = kzalloc(sizeof(*odh), GFP_KERNEL);
+	if (unlikely(!odh))
 		return ERR_PTR(-ENOMEM);
 
 	file = filp_open(name, O_RDWR, 0);
@@ -203,37 +211,39 @@ struct osd_dev *osduld_path_lookup(const char *name)
 
 	oud = file->private_data;
 
-	*od = oud->od;
-	od->file = file;
+	odh->od = oud->od;
+	odh->file = file;
+	odh->oud = oud;
 
-	return od;
+	return &odh->od;
 
 close_file:
 	fput(file);
 free_od:
-	kfree(od);
+	kfree(odh);
 	return ERR_PTR(error);
 }
 EXPORT_SYMBOL(osduld_path_lookup);
 
 void osduld_put_device(struct osd_dev *od)
 {
-
 	if (od && !IS_ERR(od)) {
-		struct osd_uld_device *oud = od->file->private_data;
+		struct osd_dev_handle *odh =
+				container_of(od, struct osd_dev_handle, od);
+		struct osd_uld_device *oud = odh->oud;
 
 		BUG_ON(od->scsi_device != oud->od.scsi_device);
 
 		/* If scsi has released the device (logout), and exofs has last
 		 * reference on oud it will be freed by above osd_uld_release
 		 * within fput below. But this will oops in cdev_release which
-		 * is called after the fops->release. __uld_get/put pair makes
+		 * is called after the fops->release. A get_/put_ pair makes
 		 * sure we have a cdev for the duration of fput
 		 */
-		__uld_get(oud);
-		fput(od->file);
-		__uld_put(oud);
-		kfree(od);
+		get_device(&oud->class_dev);
+		fput(odh->file);
+		put_device(&oud->class_dev);
+		kfree(odh);
 	}
 }
 EXPORT_SYMBOL(osduld_put_device);
@@ -264,8 +274,26 @@ static int __detect_osd(struct osd_uld_device *oud)
 	return 0;
 }
 
-static struct class *osd_sysfs_class;
-static DEFINE_IDA(osd_minor_ida);
+static void __remove(struct device *dev)
+{
+	struct osd_uld_device *oud = dev_get_drvdata(dev);
+	struct scsi_device *scsi_device = oud->od.scsi_device;
+
+	if (oud->cdev.owner)
+		cdev_del(&oud->cdev);
+
+	osd_dev_fini(&oud->od);
+	scsi_device_put(scsi_device);
+
+	OSD_INFO("osd_remove %s\n",
+		 oud->disk ? oud->disk->disk_name : NULL);
+
+	if (oud->disk)
+		put_disk(oud->disk);
+	ida_remove(&osd_minor_ida, oud->minor);
+
+	kfree(oud);
+}
 
 static int osd_probe(struct device *dev)
 {
@@ -297,7 +325,6 @@ static int osd_probe(struct device *dev)
 	if (NULL == oud)
 		goto err_retract_minor;
 
-	kref_init(&oud->kref);
 	dev_set_drvdata(dev, oud);
 	oud->minor = minor;
 
@@ -335,18 +362,26 @@ static int osd_probe(struct device *dev)
 		OSD_ERR("cdev_add failed\n");
 		goto err_put_disk;
 	}
-	kobject_get(&oud->cdev.kobj); /* 2nd ref see osd_remove() */
-
-	/* class_member */
-	oud->class_member = device_create(osd_sysfs_class, dev,
-		MKDEV(SCSI_OSD_MAJOR, oud->minor), "%s", disk->disk_name);
-	if (IS_ERR(oud->class_member)) {
-		OSD_ERR("class_device_create failed\n");
-		error = PTR_ERR(oud->class_member);
+
+	/* class device member */
+	oud->class_dev.devt = oud->cdev.dev;
+	oud->class_dev.class = &osd_uld_class;
+	oud->class_dev.parent = dev;
+	oud->class_dev.release = __remove;
+	dev_set_drvdata(&oud->class_dev, oud);
+	error = dev_set_name(&oud->class_dev, disk->disk_name);
+	if (error) {
+		OSD_ERR("dev_set_name failed => %d\n", error);
+		goto err_put_cdev;
+	}
+
+	error = device_register(&oud->class_dev);
+	if (error) {
+		OSD_ERR("device_register failed => %d\n", error);
 		goto err_put_cdev;
 	}
 
-	dev_set_drvdata(oud->class_member, oud);
+	get_device(&oud->class_dev);
 
 	OSD_INFO("osd_probe %s\n", disk->disk_name);
 	return 0;
@@ -375,54 +410,12 @@ static int osd_remove(struct device *dev)
 			scsi_device);
 	}
 
-	if (oud->class_member)
-		device_destroy(osd_sysfs_class,
-			       MKDEV(SCSI_OSD_MAJOR, oud->minor));
-
-	/* We have 2 references to the cdev. One is released here
-	 * and also takes down the /dev/osdX mapping. The second
-	 * Will be released in __remove() after all users have released
-	 * the osd_uld_device.
-	 */
-	if (oud->cdev.owner)
-		cdev_del(&oud->cdev);
+	device_unregister(&oud->class_dev);
 
-	__uld_put(oud);
+	put_device(&oud->class_dev);
 	return 0;
 }
 
-static void __remove(struct kref *kref)
-{
-	struct osd_uld_device *oud = container_of(kref,
-					struct osd_uld_device, kref);
-	struct scsi_device *scsi_device = oud->od.scsi_device;
-
-	/* now let delete the char_dev */
-	kobject_put(&oud->cdev.kobj);
-
-	osd_dev_fini(&oud->od);
-	scsi_device_put(scsi_device);
-
-	OSD_INFO("osd_remove %s\n",
-		 oud->disk ? oud->disk->disk_name : NULL);
-
-	if (oud->disk)
-		put_disk(oud->disk);
-
-	ida_remove(&osd_minor_ida, oud->minor);
-	kfree(oud);
-}
-
-static void __uld_get(struct osd_uld_device *oud)
-{
-	kref_get(&oud->kref);
-}
-
-static void __uld_put(struct osd_uld_device *oud)
-{
-	kref_put(&oud->kref, __remove);
-}
-
 /*
  * Global driver and scsi registration
  */
@@ -440,11 +433,10 @@ static int __init osd_uld_init(void)
 {
 	int err;
 
-	osd_sysfs_class = class_create(THIS_MODULE, osd_symlink);
-	if (IS_ERR(osd_sysfs_class)) {
-		OSD_ERR("Unable to register sysfs class => %ld\n",
-			PTR_ERR(osd_sysfs_class));
-		return PTR_ERR(osd_sysfs_class);
+	err = class_register(&osd_uld_class);
+	if (err) {
+		OSD_ERR("Unable to register sysfs class => %d\n", err);
+		return err;
 	}
 
 	err = register_chrdev_region(MKDEV(SCSI_OSD_MAJOR, 0),
@@ -467,7 +459,7 @@ static int __init osd_uld_init(void)
 err_out_chrdev:
 	unregister_chrdev_region(MKDEV(SCSI_OSD_MAJOR, 0), SCSI_OSD_MAX_MINOR);
 err_out:
-	class_destroy(osd_sysfs_class);
+	class_unregister(&osd_uld_class);
 	return err;
 }
 
@@ -475,7 +467,7 @@ static void __exit osd_uld_exit(void)
 {
 	scsi_unregister_driver(&osd_driver.gendrv);
 	unregister_chrdev_region(MKDEV(SCSI_OSD_MAJOR, 0), SCSI_OSD_MAX_MINOR);
-	class_destroy(osd_sysfs_class);
+	class_unregister(&osd_uld_class);
 	OSD_INFO("UNLOADED %s\n", osd_version_string);
 }
 
diff --git a/include/scsi/osd_initiator.h b/include/scsi/osd_initiator.h
index f787d24..589e5f0 100644
--- a/include/scsi/osd_initiator.h
+++ b/include/scsi/osd_initiator.h
@@ -48,7 +48,6 @@ enum osd_std_version {
  */
 struct osd_dev {
 	struct scsi_device *scsi_device;
-	struct file *file;
 	unsigned def_timeout;
 
 #ifdef OSD_VER1_SUPPORT
-- 
1.6.5.1

[PATCH 5/5] libosd: osd_dev_info: Unique Identification of an OSD device

[Boaz Harrosh]

Define an osd_dev_info structure that Uniquely identifies an OSD
device lun on the network. The identification is built from unique
target attributes and is the same for all network/SAN machines.

osduld_info_lookup() - NEW
    New API that will lookup an osd_dev by its osd_dev_info.
    This is used by pNFS-objects for cross network global device
    identification. And by exofs multy-device support, the device
    info is specified in the on-disk exofs device table.

osduld_device_info() - NEW
    Given an osd_dev handle returns its associated osd_dev_info.
    The ULD fetches this information at startup and hangs it on
    each OSD device. (This is a fast operation that can be called
    at any condition)

osduld_device_same() - NEW
    With a given osd_dev at one hand and an osd_dev_info
    at another, we would like to know if they are the same
    device.
    Two osd_dev handles can be checked by:
        osduld_device_same(od1, osduld_device_info(od2));

osd_auto_detect_ver() - REVISED
    Now returns an osd_dev_info structure. Is only called once
    by ULD as before. See added comments for how to use.

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 drivers/scsi/osd/osd_initiator.c |   26 ++++++++--
 drivers/scsi/osd/osd_uld.c       |   97 +++++++++++++++++++++++++++++++++++++-
 include/scsi/osd_initiator.h     |   38 +++++++++++++--
 3 files changed, 149 insertions(+), 12 deletions(-)

diff --git a/drivers/scsi/osd/osd_initiator.c b/drivers/scsi/osd/osd_initiator.c
index 7a117c1..60b7ca1 100644
--- a/drivers/scsi/osd/osd_initiator.c
+++ b/drivers/scsi/osd/osd_initiator.c
@@ -73,7 +73,8 @@ static const char *_osd_ver_desc(struct osd_request *or)
 
 #define ATTR_DEF_RI(id, len) ATTR_DEF(OSD_APAGE_ROOT_INFORMATION, id, len)
 
-static int _osd_print_system_info(struct osd_dev *od, void *caps)
+static int _osd_get_print_system_info(struct osd_dev *od,
+	void *caps, struct osd_dev_info *odi)
 {
 	struct osd_request *or;
 	struct osd_attr get_attrs[] = {
@@ -137,8 +138,12 @@ static int _osd_print_system_info(struct osd_dev *od, void *caps)
 	OSD_INFO("PRODUCT_SERIAL_NUMBER  [%s]\n",
 		(char *)pFirst);
 
-	pFirst = get_attrs[a].val_ptr;
-	OSD_INFO("OSD_NAME               [%s]\n", (char *)pFirst);
+	odi->osdname_len = get_attrs[a].len;
+	/* Avoid NULL for memcmp optimization 0-length is good enough */
+	odi->osdname = kzalloc(odi->osdname_len + 1, GFP_KERNEL);
+	if (odi->osdname_len)
+		memcpy(odi->osdname, get_attrs[a].val_ptr, odi->osdname_len);
+	OSD_INFO("OSD_NAME               [%s]\n", odi->osdname);
 	a++;
 
 	pFirst = get_attrs[a++].val_ptr;
@@ -171,6 +176,14 @@ static int _osd_print_system_info(struct osd_dev *od, void *caps)
 				   sid_dump, sizeof(sid_dump), true);
 		OSD_INFO("OSD_SYSTEM_ID(%d)\n"
 			 "        [%s]\n", len, sid_dump);
+
+		if (unlikely(len > sizeof(odi->systemid))) {
+			OSD_ERR("OSD Target error: OSD_SYSTEM_ID too long(%d). "
+				"device idetification might not work\n", len);
+			len = sizeof(odi->systemid);
+		}
+		odi->systemid_len = len;
+		memcpy(odi->systemid, get_attrs[a].val_ptr, len);
 		a++;
 	}
 out:
@@ -178,16 +191,17 @@ out:
 	return ret;
 }
 
-int osd_auto_detect_ver(struct osd_dev *od, void *caps)
+int osd_auto_detect_ver(struct osd_dev *od,
+	void *caps, struct osd_dev_info *odi)
 {
 	int ret;
 
 	/* Auto-detect the osd version */
-	ret = _osd_print_system_info(od, caps);
+	ret = _osd_get_print_system_info(od, caps, odi);
 	if (ret) {
 		osd_dev_set_ver(od, OSD_VER1);
 		OSD_DEBUG("converting to OSD1\n");
-		ret = _osd_print_system_info(od, caps);
+		ret = _osd_get_print_system_info(od, caps, odi);
 	}
 
 	return ret;
diff --git a/drivers/scsi/osd/osd_uld.c b/drivers/scsi/osd/osd_uld.c
index 7ec12ef..25ef993 100644
--- a/drivers/scsi/osd/osd_uld.c
+++ b/drivers/scsi/osd/osd_uld.c
@@ -85,6 +85,7 @@ struct osd_uld_device {
 	int minor;
 	struct cdev cdev;
 	struct osd_dev od;
+	struct osd_dev_info odi;
 	struct gendisk *disk;
 	struct device *class_member;
 	typeof(((struct device *)NULL)->release) save_release;
@@ -231,6 +232,71 @@ free_od:
 }
 EXPORT_SYMBOL(osduld_path_lookup);
 
+static inline bool _the_same_or_null(const u8 *a1, unsigned a1_len,
+				     const u8 *a2, unsigned a2_len)
+{
+	if (!a2_len) /* User string is Empty means don't care */
+		return true;
+
+	if (a1_len != a2_len)
+		return false;
+
+	return 0 == memcmp(a1, a2, a1_len);
+}
+
+struct find_oud_t {
+	const struct osd_dev_info *odi;
+	struct device *dev;
+	struct osd_uld_device *oud;
+} ;
+
+int _mach_odi(struct device *dev, void *find_data)
+{
+	struct osd_uld_device *oud = dev_get_drvdata(dev);
+	struct find_oud_t *fot = find_data;
+	const struct osd_dev_info *odi = fot->odi;
+
+	if (_the_same_or_null(oud->odi.systemid, oud->odi.systemid_len,
+			      odi->systemid, odi->systemid_len) &&
+	    _the_same_or_null(oud->odi.osdname, oud->odi.osdname_len,
+			      odi->osdname, odi->osdname_len)) {
+		OSD_DEBUG("found device sysid_len=%d osdname=%d\n",
+			  odi->systemid_len, odi->osdname_len);
+		fot->oud = oud;
+		return 1;
+	} else {
+		return 0;
+	}
+}
+
+/* osduld_info_lookup - Loop through all devices, return the requested osd_dev.
+ *
+ * if @odi->systemid_len and/or @odi->osdname_len are zero, they act as a don't
+ * care. .e.g if they're both zero /dev/osd0 is returned.
+ */
+struct osd_dev *osduld_info_lookup(const struct osd_dev_info *odi)
+{
+	struct find_oud_t find = {.odi = odi};
+
+	find.dev = class_find_device(&osd_uld_class, NULL, &find, _mach_odi);
+	if (likely(find.dev)) {
+		struct osd_dev_handle *odh = kzalloc(sizeof(*odh), GFP_KERNEL);
+
+		if (unlikely(!odh)) {
+			put_device(find.dev);
+			return ERR_PTR(-ENOMEM);
+		}
+
+		odh->od = find.oud->od;
+		odh->oud = find.oud;
+
+		return &odh->od;
+	}
+
+	return ERR_PTR(-ENODEV);
+}
+EXPORT_SYMBOL(osduld_info_lookup);
+
 void osduld_put_device(struct osd_dev *od)
 {
 	if (od && !IS_ERR(od)) {
@@ -247,13 +313,39 @@ void osduld_put_device(struct osd_dev *od)
 		 * sure we have a cdev for the duration of fput
 		 */
 		__uld_get(oud);
-		fput(odh->file);
+		if (odh->file)
+			fput(odh->file);
+		else
+			put_device(oud->class_member);
 		__uld_put(oud);
 		kfree(odh);
 	}
 }
 EXPORT_SYMBOL(osduld_put_device);
 
+const struct osd_dev_info *osduld_device_info(struct osd_dev *od)
+{
+	struct osd_dev_handle *odh =
+				container_of(od, struct osd_dev_handle, od);
+	return &odh->oud->odi;
+}
+EXPORT_SYMBOL(osduld_device_info);
+
+bool osduld_device_same(struct osd_dev *od, const struct osd_dev_info *odi)
+{
+	struct osd_dev_handle *odh =
+				container_of(od, struct osd_dev_handle, od);
+	struct osd_uld_device *oud = odh->oud;
+
+	return (oud->odi.systemid_len == odi->systemid_len) &&
+		_the_same_or_null(oud->odi.systemid, oud->odi.systemid_len,
+				 odi->systemid, odi->systemid_len) &&
+		(oud->odi.osdname_len == odi->osdname_len) &&
+		_the_same_or_null(oud->odi.osdname, oud->odi.osdname_len,
+				  odi->osdname, odi->osdname_len);
+}
+EXPORT_SYMBOL(osduld_device_same);
+
 /*
  * Scsi Device operations
  */
@@ -274,7 +366,7 @@ static int __detect_osd(struct osd_uld_device *oud)
 		OSD_ERR("warning: scsi_test_unit_ready failed\n");
 
 	osd_sec_init_nosec_doall_caps(caps, &osd_root_object, false, true);
-	if (osd_auto_detect_ver(&oud->od, caps))
+	if (osd_auto_detect_ver(&oud->od, caps, &oud->odi))
 		return -ENODEV;
 
 	return 0;
@@ -285,6 +377,7 @@ static void __remove(struct device *dev)
 	struct osd_uld_device *oud = dev_get_drvdata(dev);
 	struct scsi_device *scsi_device = oud->od.scsi_device;
 
+	kfree(oud->odi.osdname);
 	oud->save_release(oud->class_member);
 
 	if (oud->cdev.owner)
diff --git a/include/scsi/osd_initiator.h b/include/scsi/osd_initiator.h
index 589e5f0..3ec346e 100644
--- a/include/scsi/osd_initiator.h
+++ b/include/scsi/osd_initiator.h
@@ -55,10 +55,24 @@ struct osd_dev {
 #endif
 };
 
-/* Retrieve/return osd_dev(s) for use by Kernel clients */
-struct osd_dev *osduld_path_lookup(const char *dev_name); /*Use IS_ERR/ERR_PTR*/
+/* Unique Identification of an OSD device */
+struct osd_dev_info {
+	unsigned systemid_len;
+	u8 systemid[OSD_SYSTEMID_LEN];
+	unsigned osdname_len;
+	u8 *osdname;
+};
+
+/* Retrieve/return osd_dev(s) for use by Kernel clients
+ * Use IS_ERR/ERR_PTR on returned "osd_dev *".
+ */
+struct osd_dev *osduld_path_lookup(const char *dev_name);
+struct osd_dev *osduld_info_lookup(const struct osd_dev_info *odi);
 void osduld_put_device(struct osd_dev *od);
 
+const struct osd_dev_info *osduld_device_info(struct osd_dev *od);
+bool osduld_device_same(struct osd_dev *od, const struct osd_dev_info *odi);
+
 /* Add/remove test ioctls from external modules */
 typedef int (do_test_fn)(struct osd_dev *od, unsigned cmd, unsigned long arg);
 int osduld_register_test(unsigned ioctl, do_test_fn *do_test);
@@ -68,8 +82,24 @@ void osduld_unregister_test(unsigned ioctl);
 void osd_dev_init(struct osd_dev *od, struct scsi_device *scsi_device);
 void osd_dev_fini(struct osd_dev *od);
 
-/* some hi level device operations */
-int osd_auto_detect_ver(struct osd_dev *od, void *caps);    /* GFP_KERNEL */
+/**
+ * osd_auto_detect_ver - Detect the OSD version, return Unique Identification
+ *
+ * @od:     OSD target lun handle
+ * @caps:   Capabilities authorizing OSD root read attributes access
+ * @odi:    Retrieved information uniquely identifying the osd target lun
+ *          Note: odi->osdname must be kfreed by caller.
+ *
+ * Auto detects the OSD version of the OSD target and sets the @od
+ * accordingly. Meanwhile also returns the "system id" and "osd name" root
+ * attributes which uniquely identify the OSD target. This member is usually
+ * called by the ULD. ULD users should call osduld_device_info().
+ * This rutine allocates osd requests and memory at GFP_KERNEL level and might
+ * sleep.
+ */
+int osd_auto_detect_ver(struct osd_dev *od,
+	void *caps, struct osd_dev_info *odi);
+
 static inline struct request_queue *osd_request_queue(struct osd_dev *od)
 {
 	return od->scsi_device->request_queue;
-- 
1.6.5.1

[PATCH 5/5 version2] libosd: osd_dev_info: Unique Identification of an OSD device

[Boaz Harrosh]

Define an osd_dev_info structure that Uniquely identifies an OSD
device lun on the network. The identification is built from unique
target attributes and is the same for all network/SAN machines.

osduld_info_lookup() - NEW
    New API that will lookup an osd_dev by its osd_dev_info.
    This is used by pNFS-objects for cross network global device
    identification. And by exofs multy-device support, the device
    info is specified in the on-disk exofs device table.

osduld_device_info() - NEW
    Given an osd_dev handle returns its associated osd_dev_info.
    The ULD fetches this information at startup and hangs it on
    each OSD device. (This is a fast operation that can be called
    at any condition)

osduld_device_same() - NEW
    With a given osd_dev at one hand and an osd_dev_info
    at another, we would like to know if they are the same
    device.
    Two osd_dev handles can be checked by:
        osduld_device_same(od1, osduld_device_info(od2));

osd_auto_detect_ver() - REVISED
    Now returns an osd_dev_info structure. Is only called once
    by ULD as before. See added comments for how to use.

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 drivers/scsi/osd/osd_initiator.c |   26 ++++++++--
 drivers/scsi/osd/osd_uld.c       |   99 ++++++++++++++++++++++++++++++++++++-
 include/scsi/osd_initiator.h     |   38 +++++++++++++--
 3 files changed, 150 insertions(+), 13 deletions(-)

diff --git a/drivers/scsi/osd/osd_initiator.c b/drivers/scsi/osd/osd_initiator.c
index 7a117c1..60b7ca1 100644
--- a/drivers/scsi/osd/osd_initiator.c
+++ b/drivers/scsi/osd/osd_initiator.c
@@ -73,7 +73,8 @@ static const char *_osd_ver_desc(struct osd_request *or)
 
 #define ATTR_DEF_RI(id, len) ATTR_DEF(OSD_APAGE_ROOT_INFORMATION, id, len)
 
-static int _osd_print_system_info(struct osd_dev *od, void *caps)
+static int _osd_get_print_system_info(struct osd_dev *od,
+	void *caps, struct osd_dev_info *odi)
 {
 	struct osd_request *or;
 	struct osd_attr get_attrs[] = {
@@ -137,8 +138,12 @@ static int _osd_print_system_info(struct osd_dev *od, void *caps)
 	OSD_INFO("PRODUCT_SERIAL_NUMBER  [%s]\n",
 		(char *)pFirst);
 
-	pFirst = get_attrs[a].val_ptr;
-	OSD_INFO("OSD_NAME               [%s]\n", (char *)pFirst);
+	odi->osdname_len = get_attrs[a].len;
+	/* Avoid NULL for memcmp optimization 0-length is good enough */
+	odi->osdname = kzalloc(odi->osdname_len + 1, GFP_KERNEL);
+	if (odi->osdname_len)
+		memcpy(odi->osdname, get_attrs[a].val_ptr, odi->osdname_len);
+	OSD_INFO("OSD_NAME               [%s]\n", odi->osdname);
 	a++;
 
 	pFirst = get_attrs[a++].val_ptr;
@@ -171,6 +176,14 @@ static int _osd_print_system_info(struct osd_dev *od, void *caps)
 				   sid_dump, sizeof(sid_dump), true);
 		OSD_INFO("OSD_SYSTEM_ID(%d)\n"
 			 "        [%s]\n", len, sid_dump);
+
+		if (unlikely(len > sizeof(odi->systemid))) {
+			OSD_ERR("OSD Target error: OSD_SYSTEM_ID too long(%d). "
+				"device idetification might not work\n", len);
+			len = sizeof(odi->systemid);
+		}
+		odi->systemid_len = len;
+		memcpy(odi->systemid, get_attrs[a].val_ptr, len);
 		a++;
 	}
 out:
@@ -178,16 +191,17 @@ out:
 	return ret;
 }
 
-int osd_auto_detect_ver(struct osd_dev *od, void *caps)
+int osd_auto_detect_ver(struct osd_dev *od,
+	void *caps, struct osd_dev_info *odi)
 {
 	int ret;
 
 	/* Auto-detect the osd version */
-	ret = _osd_print_system_info(od, caps);
+	ret = _osd_get_print_system_info(od, caps, odi);
 	if (ret) {
 		osd_dev_set_ver(od, OSD_VER1);
 		OSD_DEBUG("converting to OSD1\n");
-		ret = _osd_print_system_info(od, caps);
+		ret = _osd_get_print_system_info(od, caps, odi);
 	}
 
 	return ret;
diff --git a/drivers/scsi/osd/osd_uld.c b/drivers/scsi/osd/osd_uld.c
index 9dd5948..0484bae 100644
--- a/drivers/scsi/osd/osd_uld.c
+++ b/drivers/scsi/osd/osd_uld.c
@@ -84,6 +84,7 @@ struct osd_uld_device {
 	struct device class_dev;
 	struct cdev cdev;
 	struct osd_dev od;
+	struct osd_dev_info odi;
 	struct gendisk *disk;
 };
 
@@ -225,6 +226,71 @@ free_od:
 }
 EXPORT_SYMBOL(osduld_path_lookup);
 
+static inline bool _the_same_or_null(const u8 *a1, unsigned a1_len,
+				     const u8 *a2, unsigned a2_len)
+{
+	if (!a2_len) /* User string is Empty means don't care */
+		return true;
+
+	if (a1_len != a2_len)
+		return false;
+
+	return 0 == memcmp(a1, a2, a1_len);
+}
+
+struct find_oud_t {
+	const struct osd_dev_info *odi;
+	struct device *dev;
+	struct osd_uld_device *oud;
+} ;
+
+int _mach_odi(struct device *dev, void *find_data)
+{
+	struct osd_uld_device *oud = dev_get_drvdata(dev);
+	struct find_oud_t *fot = find_data;
+	const struct osd_dev_info *odi = fot->odi;
+
+	if (_the_same_or_null(oud->odi.systemid, oud->odi.systemid_len,
+			      odi->systemid, odi->systemid_len) &&
+	    _the_same_or_null(oud->odi.osdname, oud->odi.osdname_len,
+			      odi->osdname, odi->osdname_len)) {
+		OSD_DEBUG("found device sysid_len=%d osdname=%d\n",
+			  odi->systemid_len, odi->osdname_len);
+		fot->oud = oud;
+		return 1;
+	} else {
+		return 0;
+	}
+}
+
+/* osduld_info_lookup - Loop through all devices, return the requested osd_dev.
+ *
+ * if @odi->systemid_len and/or @odi->osdname_len are zero, they act as a don't
+ * care. .e.g if they're both zero /dev/osd0 is returned.
+ */
+struct osd_dev *osduld_info_lookup(const struct osd_dev_info *odi)
+{
+	struct find_oud_t find = {.odi = odi};
+
+	find.dev = class_find_device(&osd_uld_class, NULL, &find, _mach_odi);
+	if (likely(find.dev)) {
+		struct osd_dev_handle *odh = kzalloc(sizeof(*odh), GFP_KERNEL);
+
+		if (unlikely(!odh)) {
+			put_device(find.dev);
+			return ERR_PTR(-ENOMEM);
+		}
+
+		odh->od = find.oud->od;
+		odh->oud = find.oud;
+
+		return &odh->od;
+	}
+
+	return ERR_PTR(-ENODEV);
+}
+EXPORT_SYMBOL(osduld_info_lookup);
+
 void osduld_put_device(struct osd_dev *od)
 {
 	if (od && !IS_ERR(od)) {
@@ -240,14 +306,39 @@ void osduld_put_device(struct osd_dev *od)
 		 * is called after the fops->release. A get_/put_ pair makes
 		 * sure we have a cdev for the duration of fput
 		 */
-		get_device(&oud->class_dev);
-		fput(odh->file);
+		if (odh->file) {
+			get_device(&oud->class_dev);
+			fput(odh->file);
+		}
 		put_device(&oud->class_dev);
 		kfree(odh);
 	}
 }
 EXPORT_SYMBOL(osduld_put_device);
 
+const struct osd_dev_info *osduld_device_info(struct osd_dev *od)
+{
+	struct osd_dev_handle *odh =
+				container_of(od, struct osd_dev_handle, od);
+	return &odh->oud->odi;
+}
+EXPORT_SYMBOL(osduld_device_info);
+
+bool osduld_device_same(struct osd_dev *od, const struct osd_dev_info *odi)
+{
+	struct osd_dev_handle *odh =
+				container_of(od, struct osd_dev_handle, od);
+	struct osd_uld_device *oud = odh->oud;
+
+	return (oud->odi.systemid_len == odi->systemid_len) &&
+		_the_same_or_null(oud->odi.systemid, oud->odi.systemid_len,
+				 odi->systemid, odi->systemid_len) &&
+		(oud->odi.osdname_len == odi->osdname_len) &&
+		_the_same_or_null(oud->odi.osdname, oud->odi.osdname_len,
+				  odi->osdname, odi->osdname_len);
+}
+EXPORT_SYMBOL(osduld_device_same);
+
 /*
  * Scsi Device operations
  */
@@ -268,7 +359,7 @@ static int __detect_osd(struct osd_uld_device *oud)
 		OSD_ERR("warning: scsi_test_unit_ready failed\n");
 
 	osd_sec_init_nosec_doall_caps(caps, &osd_root_object, false, true);
-	if (osd_auto_detect_ver(&oud->od, caps))
+	if (osd_auto_detect_ver(&oud->od, caps, &oud->odi))
 		return -ENODEV;
 
 	return 0;
@@ -279,6 +370,8 @@ static void __remove(struct device *dev)
 	struct osd_uld_device *oud = dev_get_drvdata(dev);
 	struct scsi_device *scsi_device = oud->od.scsi_device;
 
+	kfree(oud->odi.osdname);
+
 	if (oud->cdev.owner)
 		cdev_del(&oud->cdev);
 
diff --git a/include/scsi/osd_initiator.h b/include/scsi/osd_initiator.h
index 589e5f0..3ec346e 100644
--- a/include/scsi/osd_initiator.h
+++ b/include/scsi/osd_initiator.h
@@ -55,10 +55,24 @@ struct osd_dev {
 #endif
 };
 
-/* Retrieve/return osd_dev(s) for use by Kernel clients */
-struct osd_dev *osduld_path_lookup(const char *dev_name); /*Use IS_ERR/ERR_PTR*/
+/* Unique Identification of an OSD device */
+struct osd_dev_info {
+	unsigned systemid_len;
+	u8 systemid[OSD_SYSTEMID_LEN];
+	unsigned osdname_len;
+	u8 *osdname;
+};
+
+/* Retrieve/return osd_dev(s) for use by Kernel clients
+ * Use IS_ERR/ERR_PTR on returned "osd_dev *".
+ */
+struct osd_dev *osduld_path_lookup(const char *dev_name);
+struct osd_dev *osduld_info_lookup(const struct osd_dev_info *odi);
 void osduld_put_device(struct osd_dev *od);
 
+const struct osd_dev_info *osduld_device_info(struct osd_dev *od);
+bool osduld_device_same(struct osd_dev *od, const struct osd_dev_info *odi);
+
 /* Add/remove test ioctls from external modules */
 typedef int (do_test_fn)(struct osd_dev *od, unsigned cmd, unsigned long arg);
 int osduld_register_test(unsigned ioctl, do_test_fn *do_test);
@@ -68,8 +82,24 @@ void osduld_unregister_test(unsigned ioctl);
 void osd_dev_init(struct osd_dev *od, struct scsi_device *scsi_device);
 void osd_dev_fini(struct osd_dev *od);
 
-/* some hi level device operations */
-int osd_auto_detect_ver(struct osd_dev *od, void *caps);    /* GFP_KERNEL */
+/**
+ * osd_auto_detect_ver - Detect the OSD version, return Unique Identification
+ *
+ * @od:     OSD target lun handle
+ * @caps:   Capabilities authorizing OSD root read attributes access
+ * @odi:    Retrieved information uniquely identifying the osd target lun
+ *          Note: odi->osdname must be kfreed by caller.
+ *
+ * Auto detects the OSD version of the OSD target and sets the @od
+ * accordingly. Meanwhile also returns the "system id" and "osd name" root
+ * attributes which uniquely identify the OSD target. This member is usually
+ * called by the ULD. ULD users should call osduld_device_info().
+ * This rutine allocates osd requests and memory at GFP_KERNEL level and might
+ * sleep.
+ */
+int osd_auto_detect_ver(struct osd_dev *od,
+	void *caps, struct osd_dev_info *odi);
+
 static inline struct request_queue *osd_request_queue(struct osd_dev *od)
 {
 	return od->scsi_device->request_queue;
-- 
1.6.5.1