Messages in this thread | | | Date | Thu, 28 Aug 2008 11:17:20 -0700 | From | Greg KH <> | Subject | Re: [PATCH RESEND] char_dev: add cdev->release() and convert cdev_alloc() to use it |
| |
On Thu, Aug 28, 2008 at 07:55:39PM +0200, Tejun Heo wrote: > Greg KH wrote: > >> The problem is not the device to talk to CUSE (/dev/cuse as in > >> /dev/fuse), for which module refcount and device refcount match fine. > >> But the whole point of CUSE is allowing CUSE clients to create arbitrary > >> character devices, so in addition to /dev/cuse which clients use to talk > >> to CUSE, CUSE hosts character devices for its clients and they come and > >> go dynamically and thus requires proper lifetime management. > > > > That's fine, it's just like a USB device that uses a cdev, right? Or a > > PCI device, or any other type of device that can come and go independant > > of the cdev or module lifespan. > > > > So, you tie the cdev lifespan to the module lifespan with the > > MODULE_OWNER in the file operations. As the cuse.ko module will own the > > cdev, it can handle that reference, and it can not be removed as long as > > userspace has the cdev open, right? > > > > It really isn't any different from any other type of removable device > > (i.e. any type of device...) > > 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.
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.
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 :)
thanks,
greg k-h
| |