Re: [PATCH RESEND] char_dev: add cdev->release() and convert cdev_alloc() to use it

[Tejun Heo <tj@kernel.org>]

Hello, Greg.

I'm now trying to convert the cdev thing for CUSE and it just ain't
feel right.

Greg KH wrote:
>> Ah.. right, but taking cdev refcount out of the picture requires adding
>> 'severing' operation on cdev f_ops, which certainly is doable but isn't
>> it just cleaner to make cdev follow the usual lifetime management rules?
>>  An object which is referenced counted requires ->release if it's gonna
>> be used in any non-simplistic way.
> 
> Yes, but as you "normally" tie the cdev to the module itself, that
> handles the lifetime rules.

That "normally" is from the days when only a single or fixed number of
devices are associated with single module or driver.

> Now I really dont' like the current cdev interface, it's a bit too
> complex as it needed to support the old-style interfaces without any
> build time changes, but I think your change isn't needed as somehow all
> the current drivers that support dynamic devices don't need it.

Yes, it requires all drivers to have global device table and check
whether the device is still available at ->open.  For most, drivers
usually have certain fixed number of devices which can directly be
indexed with minor.  For CUSE, it gotta be a hash table or a b-tree.
I don't really see any point in not adding ->release.  Time has
changed and everybody is playing with reference counts and ->release
methods.  Plus, cdev_alloc() interface is half-baked anyway (no free
function for cases where cdev_add() fails, drivers call cdev_del() in
those cases risking unregistering other driver's map).  It's perfectly
okay to keep it around for compatibility but there just is no reason
to cling to it.

> Actually, the kobject in cdev shouldn't be an kobject, it's not used for
> registering with sysfs at all, it should just be a kref.  I sweep the
> tree for code that sets the name of the cdev every few months as people
> don't seem to realize this :)

Heh... CUSE did that too.  Removing it.

Thanks.

-- 
tejun