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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]
  Powered by Linux