linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Christoph Lameter <cl@linux.com>
To: Namhyung Kim <namhyung.kim@lge.com>
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, 2 Mar 2012 10:13:29 -0600 (CST)	[thread overview]
Message-ID: <alpine.DEB.2.00.1203021013180.15125@router.home> (raw)
In-Reply-To: <4F5072F4.3030505@lge.com>

On Fri, 2 Mar 2012, Namhyung Kim wrote:

> > ?? 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().

Sure. As I said you still need a page aligned value. If you were
successful in doing what you claim then there is a bug in the page
allocator because it allowed the freeing of a tail page out of a compound
page.


> > 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.

Freeing a tail page should cause a BUG() or some form of error handling.
It should not work.

> 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?

One reason to *not* do the marking of each page is that it impacts the
allocation and free paths in the allocator.

The basic notion of compound pages is that the flags in the head page are
valid for all the pages in the compound. PG_slab is set already in the
head page. So the compound is marked as a slab page. Consulting
page->firstpage->flags and not page->flags will yield the correct result.



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

  reply	other threads:[~2012-03-02 16:31 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-29  8:54 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
2012-03-02 16:13         ` Christoph Lameter [this message]
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=alpine.DEB.2.00.1203021013180.15125@router.home \
    --to=cl@linux.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mpm@selenic.com \
    --cc=namhyung.kim@lge.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