diff options
author | Boaz Harrosh <bharrosh@panasas.com> | 2009-11-29 09:25:26 -0500 |
---|---|---|
committer | James Bottomley <James.Bottomley@suse.de> | 2009-12-04 13:01:45 -0500 |
commit | d6ae4333e648492721a098bdc329bbd82d25eb67 (patch) | |
tree | 88a78be7ebd13ac3d1cfb386dd1a9bb68ba9aa1f /drivers/scsi | |
parent | 89f5e1f2f13b1079b8d7ff7d3ade345b7ad7c009 (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>
Diffstat (limited to 'drivers/scsi')
-rw-r--r-- | drivers/scsi/osd/osd_uld.c | 162 |
1 files changed, 77 insertions, 85 deletions
diff --git a/drivers/scsi/osd/osd_uld.c b/drivers/scsi/osd/osd_uld.c index 1ea6447f9418..fc6fc1c4d4d1 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 | ||
73 | static const char osd_name[] = "osd"; | 73 | static const char osd_name[] = "osd"; |
74 | static const char *osd_version_string = "open-osd 0.1.0"; | 74 | static const char *osd_version_string = "open-osd 0.2.0"; |
75 | const char osd_symlink[] = "scsi_osd"; | ||
76 | 75 | ||
77 | MODULE_AUTHOR("Boaz Harrosh <bharrosh@panasas.com>"); | 76 | MODULE_AUTHOR("Boaz Harrosh <bharrosh@panasas.com>"); |
78 | MODULE_DESCRIPTION("open-osd Upper-Layer-Driver osd.ko"); | 77 | MODULE_DESCRIPTION("open-osd Upper-Layer-Driver osd.ko"); |
@@ -82,15 +81,24 @@ MODULE_ALIAS_SCSI_DEVICE(TYPE_OSD); | |||
82 | 81 | ||
83 | struct osd_uld_device { | 82 | struct 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 | ||
92 | static void __uld_get(struct osd_uld_device *oud); | 90 | struct osd_dev_handle { |
93 | static 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 | |||
96 | static DEFINE_IDA(osd_minor_ida); | ||
97 | |||
98 | static 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 = { | |||
177 | struct osd_dev *osduld_path_lookup(const char *name) | 185 | struct 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 | ||
211 | close_file: | 220 | close_file: |
212 | fput(file); | 221 | fput(file); |
213 | free_od: | 222 | free_od: |
214 | kfree(od); | 223 | kfree(odh); |
215 | return ERR_PTR(error); | 224 | return ERR_PTR(error); |
216 | } | 225 | } |
217 | EXPORT_SYMBOL(osduld_path_lookup); | 226 | EXPORT_SYMBOL(osduld_path_lookup); |
218 | 227 | ||
219 | void osduld_put_device(struct osd_dev *od) | 228 | void 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 | } |
239 | EXPORT_SYMBOL(osduld_put_device); | 249 | EXPORT_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 | ||
267 | static struct class *osd_sysfs_class; | 277 | static void __remove(struct device *dev) |
268 | static 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 | ||
270 | static int osd_probe(struct device *dev) | 299 | static 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 | ||
394 | static 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 | |||
416 | static void __uld_get(struct osd_uld_device *oud) | ||
417 | { | ||
418 | kref_get(&oud->kref); | ||
419 | } | ||
420 | |||
421 | static 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) | |||
467 | err_out_chrdev: | 459 | err_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); |
469 | err_out: | 461 | err_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 | ||