linux-kernel - Re: [PATCH RESEND] char_dev: add cdev->release() and convert cdev

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <491BEC4E.9030505@kernel.org>
Date:	Thu, 13 Nov 2008 17:58:54 +0900
From:	Tejun Heo <tj@...nel.org>
To:	Greg KH <greg@...ah.com>
CC:	Andrew Morton <akpm@...ux-foundation.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/