Messages in this thread |  | | | Date | Fri, 26 Mar 2010 13:49:36 +0900 | | From | Tejun Heo <> | | Subject | Re: [PATCH 0/3] refcounting improvements in sysfs. |
| |
Hello,
On 03/24/2010 12:20 PM, NeilBrown wrote: > This series tidies up the refcount of sysfs_dirents in sysfs, > using kref where appropriate and a new karef for s_active. > This achieves significant code simplification, especially the first > patch. > > This is in part inspired by http://lwn.net/Articles/336224/ :-)
Nice article. In general, yeap, I agree it would be nice to have a working reference count abstraction. However, kref along with kobject is a good example of obscurity by abstraction anti pattern. :-)
kobject API incorrectly suggests that it deals with the last put problem. There still are large number of code paths which do the following,
if (!(kob = kobject_get(kobj))) return; I believe (or at least hope) the actual problem cases are mostly fixed now but there still are a lot of misconceptions around how stuff built on kref/kobject is synchronized and they sometimes lead to race conditions buried deep under several layers of abstractions and it becomes very hard to see those race conditions when they are buried deep.
If you want to kill refcounts w/ bias based off switch, please put it inside an abstraction which at least synchronizes itself properly. Open coding w/ bias at least warns you that there is some complex stuff going on and you need to trade carefully. Putting the switch on a separate flag - people often forget how bits in a flag field are synchronized - and the rest of refcount in a nice looking kref bundle is very likely to lead to subtle race conditions which are *very* difficult to notice. Thanks.
-- tejun
|  |