public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Al Viro <viro@ZenIV.linux.org.uk>
Cc: Christoph Lameter <clameter@sgi.com>,
	Theodore Tso <tytso@mit.edu>, Andi Kleen <andi@firstfloor.org>,
	Willy Tarreau <w@1wt.eu>, Steven Rostedt <rostedt@goodmis.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	LKML <linux-kernel@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Christoph Hellwig <hch@infradead.org>,
	"Rafael J. Wysocki" <rjw@sisk.pl>
Subject: Re: Major regression on hackbench with SLUB (more numbers)
Date: Fri, 28 Dec 2007 10:00:38 +0100	[thread overview]
Message-ID: <20071228090037.GA20372@elte.hu> (raw)
In-Reply-To: <20071226221631.GD27894@ZenIV.linux.org.uk>


* Al Viro <viro@ZenIV.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

  parent reply	other threads:[~2007-12-28  9:08 UTC|newest]

Thread overview: 93+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-12-07 17:50 Major regression on hackbench with SLUB Steven Rostedt
2007-12-07 17:55 ` Ingo Molnar
2007-12-07 17:59 ` Linus Torvalds
2007-12-07 18:36   ` Linus Torvalds
2007-12-07 18:58     ` Steven Rostedt
2007-12-08 22:16     ` Steven Rostedt
2007-12-10  7:38       ` Björn Steinbrink
2007-12-10 14:00         ` Steven Rostedt
2007-12-09  0:28     ` Steven Rostedt
2007-12-13 22:11     ` Christoph Lameter
2007-12-11 14:33 ` Major regression on hackbench with SLUB (more numbers) Ingo Molnar
2007-12-13 22:22   ` Christoph Lameter
2007-12-14  4:24     ` Christoph Lameter
2007-12-14  6:15       ` Steven Rostedt
2007-12-21 12:09       ` Ingo Molnar
2007-12-21 12:26         ` Ingo Molnar
2007-12-21 16:26           ` Christoph Lameter
2007-12-21 16:33             ` Ingo Molnar
2007-12-21 21:56               ` Christoph Lameter
2007-12-21 16:18         ` Christoph Lameter
2007-12-21 16:44           ` Linus Torvalds
2007-12-21 22:11             ` Christoph Lameter
2007-12-21 22:16               ` Peter Zijlstra
2007-12-21 22:17                 ` Peter Zijlstra
2007-12-21 22:27                   ` Christoph Lameter
2007-12-21 22:54                     ` Ingo Molnar
2007-12-21 23:20                       ` David Miller
2007-12-22  9:45                         ` Ingo Molnar
2007-12-24 19:24                           ` Christoph Lameter
2007-12-21 23:27                       ` Andrew Morton
2007-12-21 23:51                       ` Christoph Lameter
2007-12-22  0:28                       ` Andi Kleen
2007-12-22  0:33                         ` Chuck Ebbert
2007-12-22  2:19                         ` Matt Mackall
2007-12-22  2:44                           ` Andi Kleen
2007-12-22 10:03                         ` Ingo Molnar
2007-12-22 12:37                           ` Andi Kleen
2007-12-22 12:51                             ` Pekka Enberg
2007-12-22 12:54                               ` Andi Kleen
2007-12-22 13:27                                 ` Pekka Enberg
2007-12-22 13:01                             ` Alexey Dobriyan
2007-12-22 18:01                           ` Linus Torvalds
2007-12-22 19:09                             ` Ingo Molnar
2007-12-22 19:29                               ` Ingo Molnar
2007-12-22 22:52                               ` Jason L Tibbitts III
2007-12-24  3:59                                 ` Alessandro Suardi
2007-12-22 19:25                             ` Theodore Tso
2007-12-22 21:00                               ` Linus Torvalds
2007-12-22 21:50                                 ` Al Viro
2007-12-22 23:29                                   ` Al Viro
2007-12-22 22:10                                 ` Willy Tarreau
2007-12-23  0:59                                   ` Steven Rostedt
2007-12-23  5:12                                     ` Willy Tarreau
2007-12-23 14:15                                       ` Andi Kleen
2007-12-24  3:45                                         ` Theodore Tso
2007-12-24 19:21                                           ` Christoph Lameter
2007-12-24 23:37                                             ` Theodore Tso
2007-12-25  3:34                                               ` Andi Kleen
2008-01-01 12:47                                                 ` Theodore Tso
2008-01-01 15:19                                                   ` Andi Kleen
2008-01-01 16:23                                                   ` [patch] slub: provide /proc/slabinfo Ingo Molnar
2008-01-05  0:27                                                     ` Arjan van de Ven
2008-01-05  0:49                                                       ` Linus Torvalds
2007-12-26 21:31                                               ` Major regression on hackbench with SLUB (more numbers) Christoph Lameter
2007-12-26 22:16                                                 ` Al Viro
2007-12-27  5:51                                                   ` Christoph Lameter
2007-12-27 20:28                                                     ` SLUB sysfs support Christoph Lameter
2007-12-27 22:59                                                       ` Al Viro
2007-12-27 23:22                                                         ` Christoph Lameter
2007-12-27 23:53                                                           ` Christoph Lameter
2007-12-27 23:58                                                             ` Al Viro
2007-12-28  0:02                                                               ` Christoph Lameter
2007-12-28  0:45                                                                 ` Al Viro
2007-12-28  2:19                                                                   ` Christoph Lameter
2007-12-28  3:26                                                                     ` Al Viro
2008-01-02 20:25                                                                       ` Christoph Lameter
2007-12-27 23:54                                                           ` Al Viro
2007-12-28  9:00                                                   ` Ingo Molnar [this message]
2007-12-28 14:52                                                     ` Major regression on hackbench with SLUB (more numbers) Steven Rostedt
2007-12-22 19:46                             ` slabtop replacement was " Andi Kleen
2007-12-22 23:28                               ` Karol Swietlicki
2007-12-29 18:08                                 ` Karol Swietlicki
2007-12-21 22:49               ` Linus Torvalds
2007-12-21 17:44         ` Pekka Enberg
2007-12-21 22:22           ` Christoph Lameter
2007-12-21 22:37             ` Christoph Lameter
2007-12-21 22:51               ` Linus Torvalds
2007-12-21 23:17                 ` Ingo Molnar
2007-12-21 23:40                   ` Pekka Enberg
2007-12-21 23:54                   ` Christoph Lameter
2007-12-22  0:02                     ` Chuck Ebbert
2007-12-21 22:52               ` Steven Rostedt
2007-12-21 22:56               ` Ingo Molnar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20071228090037.GA20372@elte.hu \
    --to=mingo@elte.hu \
    --cc=a.p.zijlstra@chello.nl \
    --cc=akpm@linux-foundation.org \
    --cc=andi@firstfloor.org \
    --cc=clameter@sgi.com \
    --cc=hch@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rjw@sisk.pl \
    --cc=rostedt@goodmis.org \
    --cc=torvalds@linux-foundation.org \
    --cc=tytso@mit.edu \
    --cc=viro@ZenIV.linux.org.uk \
    --cc=w@1wt.eu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox