Re: [PATCH -next] slub: set PG_slab on all of slab pages

[Christoph Lameter]

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 from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/