All of lore.kernel.org
 help / color / mirror / Atom feed
From: Namhyung Kim <namhyung@gmail.com>
To: Christoph Lameter <cl@gentwo.org>
Cc: Namhyung Kim <namhyung.kim@lge.com>,
	Pekka Enberg <penberg@kernel.org>, Matt Mackall <mpm@selenic.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH -next] slub: set PG_slab on all of slab pages
Date: Thu, 01 Mar 2012 16:30:31 +0900	[thread overview]
Message-ID: <1330587031.1762.46.camel@leonhard> (raw)
In-Reply-To: <alpine.DEB.2.00.1202290922210.32268@router.home>

Hi,

2012-02-29, 09:24 -0600, Christoph Lameter wrote:
> On Wed, 29 Feb 2012, Namhyung Kim wrote:
> 
> > Unlike SLAB, SLUB doesn't set PG_slab on tail pages, so if a user would
> > call free_pages() incorrectly on a object in a tail page, she will get
> > confused with the undefined result. Setting the flag would help her by
> > emitting a warning on bad_page() in such a case.
> 
> NAK
> 
> You cannot free a tail page of a compound higher order page independently.
> You must free the whole compound.
> 

I meant freeing a *slab object* resides in a compound page using buddy
system API (e.g. free_pages). I know it's definitely a programming
error. However there's no safety net to protect and/or warn such a
misbehavior AFAICS - except for head page which has PG_slab set - when
it happened by any chance.

Without it, it might be possible to free part of tail pages silently,
and cause unexpected not-so-funny results some time later. It should be
hard to find out.

When I ran such a bad code using SLAB, I was able to be notified
immediately. That's why I'd like to add this patch to SLUB too. In
addition, it will give more correct value for slab pages when
using /proc/kpageflags IMHO.


-- 
Regards,
Namhyung Kim


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

WARNING: multiple messages have this Message-ID (diff)
From: Namhyung Kim <namhyung@gmail.com>
To: Christoph Lameter <cl@gentwo.org>
Cc: Namhyung Kim <namhyung.kim@lge.com>,
	Pekka Enberg <penberg@kernel.org>, Matt Mackall <mpm@selenic.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH -next] slub: set PG_slab on all of slab pages
Date: Thu, 01 Mar 2012 16:30:31 +0900	[thread overview]
Message-ID: <1330587031.1762.46.camel@leonhard> (raw)
In-Reply-To: <alpine.DEB.2.00.1202290922210.32268@router.home>

Hi,

2012-02-29, 09:24 -0600, Christoph Lameter wrote:
> On Wed, 29 Feb 2012, Namhyung Kim wrote:
> 
> > Unlike SLAB, SLUB doesn't set PG_slab on tail pages, so if a user would
> > call free_pages() incorrectly on a object in a tail page, she will get
> > confused with the undefined result. Setting the flag would help her by
> > emitting a warning on bad_page() in such a case.
> 
> NAK
> 
> You cannot free a tail page of a compound higher order page independently.
> You must free the whole compound.
> 

I meant freeing a *slab object* resides in a compound page using buddy
system API (e.g. free_pages). I know it's definitely a programming
error. However there's no safety net to protect and/or warn such a
misbehavior AFAICS - except for head page which has PG_slab set - when
it happened by any chance.

Without it, it might be possible to free part of tail pages silently,
and cause unexpected not-so-funny results some time later. It should be
hard to find out.

When I ran such a bad code using SLAB, I was able to be notified
immediately. That's why I'd like to add this patch to SLUB too. In
addition, it will give more correct value for slab pages when
using /proc/kpageflags IMHO.


-- 
Regards,
Namhyung Kim



  reply	other threads:[~2012-03-01  7:30 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-29  8:54 [PATCH -next] slub: set PG_slab on all of slab pages Namhyung Kim
2012-02-29  8:54 ` Namhyung Kim
2012-02-29 15:24 ` Christoph Lameter
2012-02-29 15:24   ` Christoph Lameter
2012-03-01  7:30   ` Namhyung Kim [this message]
2012-03-01  7:30     ` Namhyung Kim
2012-03-01 15:03     ` Christoph Lameter
2012-03-01 15:03       ` Christoph Lameter
2012-03-02  7:12       ` Namhyung Kim
2012-03-02  7:12         ` Namhyung Kim
2012-03-02 16:13         ` Christoph Lameter
2012-03-02 16:13           ` Christoph Lameter
2012-03-04 10:34 ` Minchan Kim
2012-03-04 10:34   ` Minchan Kim
2012-03-05  8:42   ` Namhyung Kim
2012-03-05  8:42     ` Namhyung Kim
2012-03-05 10:59     ` Minchan Kim
2012-03-05 10:59       ` Minchan Kim
2012-03-05 14:48   ` Christoph Lameter
2012-03-05 14:48     ` Christoph Lameter
2012-03-06  1:16     ` Minchan Kim
2012-03-06  1:16       ` Minchan Kim

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=1330587031.1762.46.camel@leonhard \
    --to=namhyung@gmail.com \
    --cc=cl@gentwo.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mpm@selenic.com \
    --cc=namhyung.kim@lge.com \
    --cc=penberg@kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.