From: Namhyung Kim <namhyung.kim@lge.com>
To: Christoph Lameter <cl@gentwo.org>
Cc: Namhyung Kim <namhyung@gmail.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: Fri, 02 Mar 2012 16:12:52 +0900 [thread overview]
Message-ID: <4F5072F4.3030505@lge.com> (raw)
In-Reply-To: <alpine.DEB.2.00.1203010901020.5004@router.home>
2012-03-02 12:03 AM, Christoph Lameter wrote:
> On Thu, 1 Mar 2012, Namhyung Kim wrote:
>
>>> 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.
>
> ?? One generally passed a struct page pointer to the page allocator. Slab
> allocator takes pointers to object. The calls that take a pointer to an
> object must have a page aligned value.
>
Please see free_pages(). It converts the pointer using virt_to_page().
>> 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.
>
> Ok then fix the page allocator to BUG() on tail pages. That is an issue
> with the page allocator not the slab allocator.
>
> Adding PG_tail to the flags checked on free should do the trick (at least
> for 64 bit).
>
Yeah, but doing it requires to change free path of compound pages. It seems
freeing normal compound pages would not clear PG_head/tail bits before
free_pages_check() called. I guess moving destroy_compound_page() into
free_pages_prepare() will solved this issue but I want to make sure it's the
right approach since I have no idea how it affects huge page behaviors.
Besides, as it has no effect on 32 bit kernels I still want add the PG_slab
flag to those pages. If you care about the performance of hot path, how about
adding it under debug configurations at least?
Thanks,
Namhyung Kim
next prev parent reply other threads:[~2012-03-02 7:13 UTC|newest]
Thread overview: 11+ 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 15:24 ` Christoph Lameter
2012-03-01 7:30 ` Namhyung Kim
2012-03-01 15:03 ` Christoph Lameter
2012-03-02 7:12 ` Namhyung Kim [this message]
2012-03-02 16:13 ` Christoph Lameter
2012-03-04 10:34 ` Minchan Kim
2012-03-05 8:42 ` Namhyung Kim
2012-03-05 10:59 ` Minchan Kim
2012-03-05 14:48 ` Christoph Lameter
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=4F5072F4.3030505@lge.com \
--to=namhyung.kim@lge.com \
--cc=cl@gentwo.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mpm@selenic.com \
--cc=namhyung@gmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox