aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorBoaz Harrosh <bharrosh@panasas.com>2009-11-29 09:25:26 -0500
committerJames Bottomley <James.Bottomley@suse.de>2009-12-04 13:01:45 -0500
commitd6ae4333e648492721a098bdc329bbd82d25eb67 (patch)
tree88a78be7ebd13ac3d1cfb386dd1a9bb68ba9aa1f
parent89f5e1f2f13b1079b8d7ff7d3ade345b7ad7c009 (diff)
[SCSI] osduld: Use device->release instead of internal kref
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 every thing is accounted for on the device object. Internal kref is removed. * At __remove() we can safely de-allocate the uld_ structure. (The function has 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> Signed-off-by: James Bottomley <James.Bottomley@suse.de>
-rw-r--r--drivers/scsi/osd/osd_uld.c162
-rw-r--r--include/scsi/osd_initiator.h1
2 files changed, 77 insertions, 86 deletions
diff --git a/drivers/scsi/osd/osd_uld.c b/drivers/scsi/osd/osd_uld.c
index 1ea6447f941..fc6fc1c4d4d 100644
--- a/drivers/scsi/osd/osd_uld.c
+++ b/drivers/scsi/osd/osd_uld.c
@@ -71,8 +71,7 @@
71#define SCSI_OSD_MAX_MINOR 64 71#define SCSI_OSD_MAX_MINOR 64
72 72
73static const char osd_name[] = "osd"; 73static const char osd_name[] = "osd";
74static const char *osd_version_string = "open-osd 0.1.0"; 74static const char *osd_version_string = "open-osd 0.2.0";
75const char osd_symlink[] = "scsi_osd";
76 75
77MODULE_AUTHOR("Boaz Harrosh <bharrosh@panasas.com>"); 76MODULE_AUTHOR("Boaz Harrosh <bharrosh@panasas.com>");
78MODULE_DESCRIPTION("open-osd Upper-Layer-Driver osd.ko"); 77MODULE_DESCRIPTION("open-osd Upper-Layer-Driver osd.ko");
@@ -82,15 +81,24 @@ MODULE_ALIAS_SCSI_DEVICE(TYPE_OSD);
82 81
83struct osd_uld_device { 82struct osd_uld_device {
84 int minor; 83 int minor;
85 struct kref kref; 84 struct device class_dev;
86 struct cdev cdev; 85 struct cdev cdev;
87 struct osd_dev od; 86 struct osd_dev od;
88 struct gendisk *disk; 87 struct gendisk *disk;
89 struct device *class_member;
90}; 88};
91 89
92static void __uld_get(struct osd_uld_device *oud); 90struct osd_dev_handle {
93static void __uld_put(struct osd_uld_device *oud); 91 struct osd_dev od;
92 struct file *file;
93 struct osd_uld_device *oud;
94} ;
95
96static DEFINE_IDA(osd_minor_ida);
97
98static struct class osd_uld_class = {
99 .owner = THIS_MODULE,
100 .name = "scsi_osd",
101};
94 102
95/* 103/*
96 * Char Device operations 104 * Char Device operations
@@ -101,7 +109,7 @@ static int osd_uld_open(struct inode *inode, struct file *file)
101 struct osd_uld_device *oud = container_of(inode->i_cdev, 109 struct osd_uld_device *oud = container_of(inode->i_cdev,
102 struct osd_uld_device, cdev); 110 struct osd_uld_device, cdev);
103 111
104 __uld_get(oud); 112 get_device(&oud->class_dev);
105 /* cache osd_uld_device on file handle */ 113 /* cache osd_uld_device on file handle */
106 file->private_data = oud; 114 file->private_data = oud;
107 OSD_DEBUG("osd_uld_open %p\n", oud); 115 OSD_DEBUG("osd_uld_open %p\n", oud);
@@ -114,7 +122,7 @@ static int osd_uld_release(struct inode *inode, struct file *file)
114 122
115 OSD_DEBUG("osd_uld_release %p\n", file->private_data); 123 OSD_DEBUG("osd_uld_release %p\n", file->private_data);
116 file->private_data = NULL; 124 file->private_data = NULL;
117 __uld_put(oud); 125 put_device(&oud->class_dev);
118 return 0; 126 return 0;
119} 127}
120 128
@@ -177,7 +185,7 @@ static const struct file_operations osd_fops = {
177struct osd_dev *osduld_path_lookup(const char *name) 185struct osd_dev *osduld_path_lookup(const char *name)
178{ 186{
179 struct osd_uld_device *oud; 187 struct osd_uld_device *oud;
180 struct osd_dev *od; 188 struct osd_dev_handle *odh;
181 struct file *file; 189 struct file *file;
182 int error; 190 int error;
183 191
@@ -186,8 +194,8 @@ struct osd_dev *osduld_path_lookup(const char *name)
186 return ERR_PTR(-EINVAL); 194 return ERR_PTR(-EINVAL);
187 } 195 }
188 196
189 od = kzalloc(sizeof(*od), GFP_KERNEL); 197 odh = kzalloc(sizeof(*odh), GFP_KERNEL);
190 if (!od) 198 if (unlikely(!odh))
191 return ERR_PTR(-ENOMEM); 199 return ERR_PTR(-ENOMEM);
192 200
193 file = filp_open(name, O_RDWR, 0); 201 file = filp_open(name, O_RDWR, 0);
@@ -203,37 +211,39 @@ struct osd_dev *osduld_path_lookup(const char *name)
203 211
204 oud = file->private_data; 212 oud = file->private_data;
205 213
206 *od = oud->od; 214 odh->od = oud->od;
207 od->file = file; 215 odh->file = file;
216 odh->oud = oud;
208 217
209 return od; 218 return &odh->od;
210 219
211close_file: 220close_file:
212 fput(file); 221 fput(file);
213free_od: 222free_od:
214 kfree(od); 223 kfree(odh);
215 return ERR_PTR(error); 224 return ERR_PTR(error);
216} 225}
217EXPORT_SYMBOL(osduld_path_lookup); 226EXPORT_SYMBOL(osduld_path_lookup);
218 227
219void osduld_put_device(struct osd_dev *od) 228void osduld_put_device(struct osd_dev *od)
220{ 229{
221
222 if (od && !IS_ERR(od)) { 230 if (od && !IS_ERR(od)) {
223 struct osd_uld_device *oud = od->file->private_data; 231 struct osd_dev_handle *odh =
232 container_of(od, struct osd_dev_handle, od);
233 struct osd_uld_device *oud = odh->oud;
224 234
225 BUG_ON(od->scsi_device != oud->od.scsi_device); 235 BUG_ON(od->scsi_device != oud->od.scsi_device);
226 236
227 /* If scsi has released the device (logout), and exofs has last 237 /* If scsi has released the device (logout), and exofs has last
228 * reference on oud it will be freed by above osd_uld_release 238 * reference on oud it will be freed by above osd_uld_release
229 * within fput below. But this will oops in cdev_release which 239 * within fput below. But this will oops in cdev_release which
230 * is called after the fops->release. __uld_get/put pair makes 240 * is called after the fops->release. A get_/put_ pair makes
231 * sure we have a cdev for the duration of fput 241 * sure we have a cdev for the duration of fput
232 */ 242 */
233 __uld_get(oud); 243 get_device(&oud->class_dev);
234 fput(od->file); 244 fput(odh->file);
235 __uld_put(oud); 245 put_device(&oud->class_dev);
236 kfree(od); 246 kfree(odh);
237 } 247 }
238} 248}
239EXPORT_SYMBOL(osduld_put_device); 249EXPORT_SYMBOL(osduld_put_device);
@@ -264,8 +274,27 @@ static int __detect_osd(struct osd_uld_device *oud)
264 return 0; 274 return 0;
265} 275}
266 276
267static struct class *osd_sysfs_class; 277static void __remove(struct device *dev)
268static DEFINE_IDA(osd_minor_ida); 278{
279 struct osd_uld_device *oud = container_of(dev, struct osd_uld_device,
280 class_dev);
281 struct scsi_device *scsi_device = oud->od.scsi_device;
282
283 if (oud->cdev.owner)
284 cdev_del(&oud->cdev);
285
286 osd_dev_fini(&oud->od);
287 scsi_device_put(scsi_device);
288
289 OSD_INFO("osd_remove %s\n",
290 oud->disk ? oud->disk->disk_name : NULL);
291
292 if (oud->disk)
293 put_disk(oud->disk);
294 ida_remove(&osd_minor_ida, oud->minor);
295
296 kfree(oud);
297}
269 298
270static int osd_probe(struct device *dev) 299static int osd_probe(struct device *dev)
271{ 300{
@@ -297,7 +326,6 @@ static int osd_probe(struct device *dev)
297 if (NULL == oud) 326 if (NULL == oud)
298 goto err_retract_minor; 327 goto err_retract_minor;
299 328
300 kref_init(&oud->kref);
301 dev_set_drvdata(dev, oud); 329 dev_set_drvdata(dev, oud);
302 oud->minor = minor; 330 oud->minor = minor;
303 331
@@ -335,18 +363,25 @@ static int osd_probe(struct device *dev)
335 OSD_ERR("cdev_add failed\n"); 363 OSD_ERR("cdev_add failed\n");
336 goto err_put_disk; 364 goto err_put_disk;
337 } 365 }
338 kobject_get(&oud->cdev.kobj); /* 2nd ref see osd_remove() */ 366
339 367 /* class device member */
340 /* class_member */ 368 oud->class_dev.devt = oud->cdev.dev;
341 oud->class_member = device_create(osd_sysfs_class, dev, 369 oud->class_dev.class = &osd_uld_class;
342 MKDEV(SCSI_OSD_MAJOR, oud->minor), "%s", disk->disk_name); 370 oud->class_dev.parent = dev;
343 if (IS_ERR(oud->class_member)) { 371 oud->class_dev.release = __remove;
344 OSD_ERR("class_device_create failed\n"); 372 error = dev_set_name(&oud->class_dev, disk->disk_name);
345 error = PTR_ERR(oud->class_member); 373 if (error) {
374 OSD_ERR("dev_set_name failed => %d\n", error);
346 goto err_put_cdev; 375 goto err_put_cdev;
347 } 376 }
348 377
349 dev_set_drvdata(oud->class_member, oud); 378 error = device_register(&oud->class_dev);
379 if (error) {
380 OSD_ERR("device_register failed => %d\n", error);
381 goto err_put_cdev;
382 }
383
384 get_device(&oud->class_dev);
350 385
351 OSD_INFO("osd_probe %s\n", disk->disk_name); 386 OSD_INFO("osd_probe %s\n", disk->disk_name);
352 return 0; 387 return 0;
@@ -375,54 +410,12 @@ static int osd_remove(struct device *dev)
375 scsi_device); 410 scsi_device);
376 } 411 }
377 412
378 if (oud->class_member) 413 device_unregister(&oud->class_dev);
379 device_destroy(osd_sysfs_class,
380 MKDEV(SCSI_OSD_MAJOR, oud->minor));
381
382 /* We have 2 references to the cdev. One is released here
383 * and also takes down the /dev/osdX mapping. The second
384 * Will be released in __remove() after all users have released
385 * the osd_uld_device.
386 */
387 if (oud->cdev.owner)
388 cdev_del(&oud->cdev);
389 414
390 __uld_put(oud); 415 put_device(&oud->class_dev);
391 return 0; 416 return 0;
392} 417}
393 418
394static void __remove(struct kref *kref)
395{
396 struct osd_uld_device *oud = container_of(kref,
397 struct osd_uld_device, kref);
398 struct scsi_device *scsi_device = oud->od.scsi_device;
399
400 /* now let delete the char_dev */
401 kobject_put(&oud->cdev.kobj);
402
403 osd_dev_fini(&oud->od);
404 scsi_device_put(scsi_device);
405
406 OSD_INFO("osd_remove %s\n",
407 oud->disk ? oud->disk->disk_name : NULL);
408
409 if (oud->disk)
410 put_disk(oud->disk);
411
412 ida_remove(&osd_minor_ida, oud->minor);
413 kfree(oud);
414}
415
416static void __uld_get(struct osd_uld_device *oud)
417{
418 kref_get(&oud->kref);
419}
420
421static void __uld_put(struct osd_uld_device *oud)
422{
423 kref_put(&oud->kref, __remove);
424}
425
426/* 419/*
427 * Global driver and scsi registration 420 * Global driver and scsi registration
428 */ 421 */
@@ -440,11 +433,10 @@ static int __init osd_uld_init(void)
440{ 433{
441 int err; 434 int err;
442 435
443 osd_sysfs_class = class_create(THIS_MODULE, osd_symlink); 436 err = class_register(&osd_uld_class);
444 if (IS_ERR(osd_sysfs_class)) { 437 if (err) {
445 OSD_ERR("Unable to register sysfs class => %ld\n", 438 OSD_ERR("Unable to register sysfs class => %d\n", err);
446 PTR_ERR(osd_sysfs_class)); 439 return err;
447 return PTR_ERR(osd_sysfs_class);
448 } 440 }
449 441
450 err = register_chrdev_region(MKDEV(SCSI_OSD_MAJOR, 0), 442 err = register_chrdev_region(MKDEV(SCSI_OSD_MAJOR, 0),
@@ -467,7 +459,7 @@ static int __init osd_uld_init(void)
467err_out_chrdev: 459err_out_chrdev:
468 unregister_chrdev_region(MKDEV(SCSI_OSD_MAJOR, 0), SCSI_OSD_MAX_MINOR); 460 unregister_chrdev_region(MKDEV(SCSI_OSD_MAJOR, 0), SCSI_OSD_MAX_MINOR);
469err_out: 461err_out:
470 class_destroy(osd_sysfs_class); 462 class_unregister(&osd_uld_class);
471 return err; 463 return err;
472} 464}
473 465
@@ -475,7 +467,7 @@ static void __exit osd_uld_exit(void)
475{ 467{
476 scsi_unregister_driver(&osd_driver.gendrv); 468 scsi_unregister_driver(&osd_driver.gendrv);
477 unregister_chrdev_region(MKDEV(SCSI_OSD_MAJOR, 0), SCSI_OSD_MAX_MINOR); 469 unregister_chrdev_region(MKDEV(SCSI_OSD_MAJOR, 0), SCSI_OSD_MAX_MINOR);
478 class_destroy(osd_sysfs_class); 470 class_unregister(&osd_uld_class);
479 OSD_INFO("UNLOADED %s\n", osd_version_string); 471 OSD_INFO("UNLOADED %s\n", osd_version_string);
480} 472}
481 473
diff --git a/include/scsi/osd_initiator.h b/include/scsi/osd_initiator.h
index f787d24d3ba..589e5f0d67b 100644
--- a/include/scsi/osd_initiator.h
+++ b/include/scsi/osd_initiator.h
@@ -48,7 +48,6 @@ enum osd_std_version {
48 */ 48 */
49struct osd_dev { 49struct osd_dev {
50 struct scsi_device *scsi_device; 50 struct scsi_device *scsi_device;
51 struct file *file;
52 unsigned def_timeout; 51 unsigned def_timeout;
53 52
54#ifdef OSD_VER1_SUPPORT 53#ifdef OSD_VER1_SUPPORT