public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: David Howells <dhowells@redhat.com>
To: Matt Mackall <mpm@selenic.com>
Cc: Aubrey <aubreylee@gmail.com>,
	Nick Piggin <nickpiggin@yahoo.com.au>,
	David Howells <dhowells@redhat.com>,
	linux-kernel@vger.kernel.org, davidm@snapgear.com,
	gerg@snapgear.com
Subject: Re: kernel BUGs when removing largish files with the SLOB allocator
Date: Tue, 12 Sep 2006 20:10:32 +0100	[thread overview]
Message-ID: <15193.1158088232@warthog.cambridge.redhat.com> (raw)
In-Reply-To: <20060912174339.GA19707@waste.org>

Matt Mackall <mpm@selenic.com> wrote:

> > +		for (i = 0; i < (1 << bb->order); i++) {
> > +			SetPageSlab(page);
> > +			page++;
> > +		}
> 
> for ( ; page < page + (1 << bb->order), page++)
>       SetPageSlab(page);

Ugh.  No.  You can't do that.  "page < page + X" will be true until "page + X"
wraps the end of memory.

> > +				for (i = 0; i < (1 << bb->order); i++) {
> > +					if (!TestClearPageSlab(page))
> > +						BUG();
> > +					page++;
> > +				}
> 
> Please drop the BUG. We've already established it's on our lists by
> this point.

I disagree.  Let's catch accidental reuse of pages.  It should, however, be
marked unlikely().

> You just broke the bit that shrinks the arena.

How?  This is only called once when things are being initialised.  There can
be no SLOB objects allocated prior to that point.

> I don't like this patch. We've just grown SLOB by about 10% everywhere
> to make nommu happy. Is this needed because nommu is doing something
> special for MMU-less machines or because it's doing something bogus?

See preceding discussion on LKML.

kobjsize() needs to work out how to calculate the size of an object.  One of
the main classes of object it might be given is slab/slob objects.  Either
these should be marked with PG_slab as per the slab allocator, of kobjsize()
has to go and walk all the slab allocator or slob allocator lists to find out
whether this page belongs to them.  Only then can it know whether or not it
can trust the page metadata as slab/slob metadata or not.

> Looking through all the users of kobjsize, it seems we always know
> what the type is (and it's usually a VMA). I instead propose we use
> ksize on objects we know to be SLAB/SLOB-allocated and add a new
> function (kpagesize?) to size other objects where nommu needs it.

Yes, we might have a VMA, but no, we do not know how big the object we've
_actually_ been given by whoever is.  kmalloc() doesn't tell us that and
get_unmapped_area() doesn't tell us that but we want it to account correctly
for the ammount of memory allocated.

You say "use ksize on objects we know to be SLAB/SLOB-allocated" which is the
whole point of PG_slab.

> As for whether SLOB should emulate PG_slab on general principles, as
> far as I can tell, PG_slab should be considered a private debugging
> aid to SLAB. All the other users look rather bogus to my eye.
>
> If anything, SLOB should internally abuse PG_slab to mark big pages
> and dispense with its bigblocks lists.

It's not abuse.  SLOB is a drop-in replacement for the SLAB allocator,
therefore PG_slab belongs to it instead of SLAB.


Anyway, if you've got a better idea, then patch please.

David

  reply	other threads:[~2006-09-12 19:10 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-09-04  6:56 kernel BUGs when removing largish files with the SLOB allocator Aubrey
2006-09-04 10:21 ` David Howells
2006-09-05  3:52   ` Aubrey
2006-09-05  9:35     ` David Howells
2006-09-06  2:35       ` Aubrey
2006-09-06  3:36         ` Nick Piggin
2006-09-12  8:07           ` Aubrey
2006-09-12  8:54             ` David Howells
2006-09-12 10:53               ` Aubrey
2006-09-12 17:43             ` Matt Mackall
2006-09-12 19:10               ` David Howells [this message]
2006-09-12 20:51                 ` Matt Mackall
2006-09-12 20:56                   ` David Howells
2006-09-12 21:04                     ` Matt Mackall
2006-09-12 21:59                       ` David Howells
2006-09-13 19:39                         ` Matt Mackall
2006-09-13  2:16                 ` Nick Piggin
2006-09-13  7:21                   ` Aubrey
2006-09-12 19:25               ` David Howells
2006-09-12 20:28                 ` Matt Mackall
2006-09-12 21:02                   ` David Howells
2006-09-12 21:15                     ` Matt Mackall
2006-09-12 20:49                 ` Matt Mackall

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=15193.1158088232@warthog.cambridge.redhat.com \
    --to=dhowells@redhat.com \
    --cc=aubreylee@gmail.com \
    --cc=davidm@snapgear.com \
    --cc=gerg@snapgear.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mpm@selenic.com \
    --cc=nickpiggin@yahoo.com.au \
    /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