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

[Boaz Harrosh <bharrosh@panasas.com>]

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