Date:	Fri, 28 Dec 2007 10:00:38 +0100
From:	Ingo Molnar <mingo@...e.hu>
To:	Al Viro <viro@...IV.linux.org.uk>
Cc:	Christoph Lameter <clameter@....com>, Theodore Tso <tytso@....edu>,
	Andi Kleen <andi@...stfloor.org>, Willy Tarreau <w@....eu>,
	Steven Rostedt <rostedt@...dmis.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	LKML <linux-kernel@...r.kernel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Christoph Hellwig <hch@...radead.org>,
	"Rafael J. Wysocki" <rjw@...k.pl>
Subject: Re: Major regression on hackbench with SLUB (more numbers)


* Al Viro <viro@...IV.linux.org.uk> wrote:

> > > So two questions: why isn't -f the default?  And is /sys/slab
> > 
> > Because it gives misleading output. It displays the name of the 
> > first of multiple slabs that share the same storage structures.
> 
> Erm...  Let me spell it out: current lifetime rules are completely 
> broken. As it is, create/destroy/create cache sequence will do 
> kobject_put() on kfree'd object.  Even without people playing with 
> holding sysfs files open or doing IO on those.
> 
> a) you have kobject embedded into struct with the lifetime rules of 
> its own.  When its refcount hits zero you kfree() the sucker, even if 
> you still have references to embedded kobject.
> 
> b) your symlinks stick around.  Even when cache is long gone you still 
> have a sysfs symlink with its embedded kobject as a target.  They are 
> eventually removed when cache with the same name gets created.  _Then_ 
> you get the target kobject dropped - when the memory it used to be in 
> had been freed for hell knows how long and reused by something that 
> would not appreciate slub.c code suddenly deciding to decrement some 
> word in that memory.
> 
> c) you leak references to these kobject; kobject_del() only removes it 
> from the tree undoing the effect of kobject_add() and you still need 
> kobject_put() to deal with the last reference.

as a sidenote: bugs like this seem to be reoccuring. People implement 
sysfs bindings (without being sysfs internals experts - and why should 
they be) - and create hard to debug problems. We've seen that with the 
scheduler's recent sysfs changes too.

shouldnt the sysfs code be designed in a way to not allow such bugs? The 
primary usecase of sysfs is by people who do _not_ deal with it on a 
daily basis. So if they pick APIs that look obvious and create hard to 
debug problems (and userspace incompatibilities) that's a primary 
failure of sysfs, not a failure of those who utilize it.

At a minimum there should be some _strong_ debugging facility that 
transparently detects and reports such bugs as they occur. 
CONFIG_DEBUG_KOBJECT is totally unusable right now, it spams the syslog 
(so no distro ever enables it - i disable it in random bootups as well 
because it takes _ages_ to even get to a boot prompt) and never finds 
any of these hard-to-find-but-easy-to-explain bugs.

or if sysfs/kobjects should be scrapped and rewritten, do you have any 
insight into what kind of abstraction could/should replace it? Should we 
go back to procfs and get rid of kobjects altogether? (as it's slowly 
turning into a /proc problem of itself, with worse compatibility and 
sneakier bugs.)

	Ingo
--
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/

Powered by blists - more mailing lists