lkml.org 
[lkml]   [2008]   [Nov]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH RESEND] char_dev: add cdev->release() and convert cdev_alloc() to use it
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


\
 
 \ /
  Last update: 2008-11-13 10:01    [W:0.053 / U:0.048 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site