* [PATCH 1/2] Make page->private usable in compound pages V1
@ 2007-04-05 22:36 Christoph Lameter
2007-04-05 22:36 ` [PATCH 2/2] Optimize compound_head() by avoiding a shared page flag Christoph Lameter
2007-04-05 23:43 ` [PATCH 1/2] Make page->private usable in compound pages V1 Andrew Morton
0 siblings, 2 replies; 19+ messages in thread
From: Christoph Lameter @ 2007-04-05 22:36 UTC (permalink / raw)
To: akpm; +Cc: Hugh Dickins, Nick Piggin, Christoph Lameter, dgc, linux-kernel
[PATCH] Free up page->private for compound pages
If we add a new flag so that we can distinguish between the
first page and the tail pages then we can avoid to use page->private
in the first page. page->private == page for the first page, so there
is no real information in there.
Freeing up page->private makes the use of compound pages more transparent.
They become more usable like real pages. Right now we have to be careful f.e.
if we are going beyond PAGE_SIZE allocations in the slab on i386 because we
can then no longer use the private field. This is one of the issues that
cause us not to support debugging for page size slabs in SLAB.
Having page->private available for SLUB would allow more meta information
in the page struct. I can probably avoid the 16 bit ints that I have in
there right now.
Also if page->private is available then a compound page may be equipped
with buffer heads. This may free up the way for filesystems to support
larger blocks than page size.
We add PageTail as an alias of PageReclaim. Compound pages cannot
currently be reclaimed. Because of the alias one needs to check
PageCompound first.
The RFC for the this approach was discussed at
http://marc.info/?t=117574302800001&r=1&w=2
Signed-off-by: Christoph Lameter <clameter@sgi.com>
Index: linux-2.6.21-rc5-mm4/include/linux/mm.h
===================================================================
--- linux-2.6.21-rc5-mm4.orig/include/linux/mm.h 2007-04-05 13:59:23.000000000 -0700
+++ linux-2.6.21-rc5-mm4/include/linux/mm.h 2007-04-05 14:08:11.000000000 -0700
@@ -297,17 +297,28 @@ static inline int get_page_unless_zero(s
return atomic_inc_not_zero(&page->_count);
}
+static inline struct page *compound_head(struct page *page)
+{
+ /*
+ * We could avoid the PageCompound(page) check if
+ * we would not overload PageTail().
+ *
+ * This check has to be done in several performance critical
+ * paths of the slab etc. IMHO PageTail deserves its own flag.
+ */
+ if (unlikely(PageCompound(page) && PageTail(page)))
+ return page->first_page;
+ return page;
+}
+
static inline int page_count(struct page *page)
{
- if (unlikely(PageCompound(page)))
- page = (struct page *)page_private(page);
- return atomic_read(&page->_count);
+ return atomic_read(&compound_head(page)->_count);
}
static inline void get_page(struct page *page)
{
- if (unlikely(PageCompound(page)))
- page = (struct page *)page_private(page);
+ page = compound_head(page);
VM_BUG_ON(atomic_read(&page->_count) == 0);
atomic_inc(&page->_count);
}
@@ -344,6 +355,18 @@ static inline compound_page_dtor *get_co
return (compound_page_dtor *)page[1].lru.next;
}
+static inline int compound_order(struct page *page)
+{
+ if (!PageCompound(page) || PageTail(page))
+ return 0;
+ return (unsigned long)page[1].lru.prev;
+}
+
+static inline void set_compound_order(struct page *page, unsigned long order)
+{
+ page[1].lru.prev = (void *)order;
+}
+
/*
* Multiple processes may "see" the same page. E.g. for untouched
* mappings of /dev/null, all processes see the same page full of
Index: linux-2.6.21-rc5-mm4/include/linux/page-flags.h
===================================================================
--- linux-2.6.21-rc5-mm4.orig/include/linux/page-flags.h 2007-04-05 13:59:23.000000000 -0700
+++ linux-2.6.21-rc5-mm4/include/linux/page-flags.h 2007-04-05 14:00:56.000000000 -0700
@@ -95,6 +95,12 @@
/* PG_owner_priv_1 users should have descriptive aliases */
#define PG_checked PG_owner_priv_1 /* Used by some filesystems */
+/*
+ * Marks tail portion of a compound page. We currently do not reclaim
+ * compound pages so we can reuse a flag only used for reclaim here.
+ */
+#define PG_tail PG_reclaim
+
#if (BITS_PER_LONG > 32)
/*
* 64-bit-only flags build down from bit 31
@@ -214,6 +220,14 @@ static inline void SetPageUptodate(struc
#define __SetPageCompound(page) __set_bit(PG_compound, &(page)->flags)
#define __ClearPageCompound(page) __clear_bit(PG_compound, &(page)->flags)
+/*
+ * Note: PG_tail is an alias of another page flag. The result of PageTail()
+ * is only valid if PageCompound(page) is true.
+ */
+#define PageTail(page) test_bit(PG_tail, &(page)->flags)
+#define __SetPageTail(page) __set_bit(PG_tail, &(page)->flags)
+#define __ClearPageTail(page) __clear_bit(PG_tail, &(page)->flags)
+
#ifdef CONFIG_SWAP
#define PageSwapCache(page) test_bit(PG_swapcache, &(page)->flags)
#define SetPageSwapCache(page) set_bit(PG_swapcache, &(page)->flags)
Index: linux-2.6.21-rc5-mm4/mm/internal.h
===================================================================
--- linux-2.6.21-rc5-mm4.orig/mm/internal.h 2007-04-05 13:59:24.000000000 -0700
+++ linux-2.6.21-rc5-mm4/mm/internal.h 2007-04-05 14:00:56.000000000 -0700
@@ -24,7 +24,7 @@ static inline void set_page_count(struct
*/
static inline void set_page_refcounted(struct page *page)
{
- VM_BUG_ON(PageCompound(page) && page_private(page) != (unsigned long)page);
+ VM_BUG_ON(PageCompound(page) && PageTail(page));
VM_BUG_ON(atomic_read(&page->_count));
set_page_count(page, 1);
}
Index: linux-2.6.21-rc5-mm4/mm/page_alloc.c
===================================================================
--- linux-2.6.21-rc5-mm4.orig/mm/page_alloc.c 2007-04-05 13:59:24.000000000 -0700
+++ linux-2.6.21-rc5-mm4/mm/page_alloc.c 2007-04-05 14:00:56.000000000 -0700
@@ -290,7 +290,7 @@ static void bad_page(struct page *page)
static void free_compound_page(struct page *page)
{
- __free_pages_ok(page, (unsigned long)page[1].lru.prev);
+ __free_pages_ok(page, compound_order(page));
}
static void prep_compound_page(struct page *page, unsigned long order)
@@ -299,12 +299,14 @@ static void prep_compound_page(struct pa
int nr_pages = 1 << order;
set_compound_page_dtor(page, free_compound_page);
- page[1].lru.prev = (void *)order;
- for (i = 0; i < nr_pages; i++) {
+ set_compound_order(page, order);
+ __SetPageCompound(page);
+ for (i = 1; i < nr_pages; i++) {
struct page *p = page + i;
+ __SetPageTail(p);
__SetPageCompound(p);
- set_page_private(p, (unsigned long)page);
+ p->first_page = page;
}
}
@@ -313,15 +315,19 @@ static void destroy_compound_page(struct
int i;
int nr_pages = 1 << order;
- if (unlikely((unsigned long)page[1].lru.prev != order))
+ if (unlikely(compound_order(page) != order))
bad_page(page);
- for (i = 0; i < nr_pages; i++) {
+ if (unlikely(!PageCompound(page)))
+ bad_page(page);
+ __ClearPageCompound(page);
+ for (i = 1; i < nr_pages; i++) {
struct page *p = page + i;
- if (unlikely(!PageCompound(p) |
- (page_private(p) != (unsigned long)page)))
+ if (unlikely(!PageCompound(p) | !PageTail(p) |
+ (p->first_page != page)))
bad_page(page);
+ __ClearPageTail(p);
__ClearPageCompound(p);
}
}
@@ -494,13 +500,18 @@ static inline int free_pages_check(struc
1 << PG_private |
1 << PG_locked |
1 << PG_active |
- 1 << PG_reclaim |
1 << PG_slab |
1 << PG_swapcache |
1 << PG_writeback |
1 << PG_reserved |
1 << PG_buddy ))))
bad_page(page);
+ /*
+ * PageReclaim == PageTail. It is only an error
+ * for PageReclaim to be set if PageCompound is clear.
+ */
+ if (unlikely(!PageCompound(page) && PageReclaim(page)))
+ bad_page(page);
if (PageDirty(page))
__ClearPageDirty(page);
/*
Index: linux-2.6.21-rc5-mm4/mm/slab.c
===================================================================
--- linux-2.6.21-rc5-mm4.orig/mm/slab.c 2007-04-05 13:59:23.000000000 -0700
+++ linux-2.6.21-rc5-mm4/mm/slab.c 2007-04-05 14:00:56.000000000 -0700
@@ -602,8 +602,7 @@ static inline void page_set_cache(struct
static inline struct kmem_cache *page_get_cache(struct page *page)
{
- if (unlikely(PageCompound(page)))
- page = (struct page *)page_private(page);
+ page = compound_head(page);
BUG_ON(!PageSlab(page));
return (struct kmem_cache *)page->lru.next;
}
@@ -615,8 +614,7 @@ static inline void page_set_slab(struct
static inline struct slab *page_get_slab(struct page *page)
{
- if (unlikely(PageCompound(page)))
- page = (struct page *)page_private(page);
+ page = compound_head(page);
BUG_ON(!PageSlab(page));
return (struct slab *)page->lru.prev;
}
Index: linux-2.6.21-rc5-mm4/mm/slub.c
===================================================================
--- linux-2.6.21-rc5-mm4.orig/mm/slub.c 2007-04-05 13:59:24.000000000 -0700
+++ linux-2.6.21-rc5-mm4/mm/slub.c 2007-04-05 14:00:56.000000000 -0700
@@ -1273,9 +1273,7 @@ void kmem_cache_free(struct kmem_cache *
page = virt_to_page(x);
- if (unlikely(PageCompound(page)))
- page = page->first_page;
-
+ page = compound_head(page);
if (unlikely(PageError(page) && (s->flags & SLAB_STORE_USER)))
set_tracking(s, x, 1);
@@ -1286,10 +1284,7 @@ EXPORT_SYMBOL(kmem_cache_free);
/* Figure out on which slab object the object resides */
static struct page *get_object_page(const void *x)
{
- struct page *page = virt_to_page(x);
-
- if (unlikely(PageCompound(page)))
- page = page->first_page;
+ struct page *page = compound_head(virt_to_page(x));
if (!PageSlab(page))
return NULL;
@@ -1896,10 +1891,7 @@ void kfree(const void *x)
if (!x)
return;
- page = virt_to_page(x);
-
- if (unlikely(PageCompound(page)))
- page = page->first_page;
+ page = compound_head(virt_to_page(x));
s = page->slab;
@@ -1935,10 +1927,7 @@ void *krealloc(const void *p, size_t new
return NULL;
}
- page = virt_to_page(p);
-
- if (unlikely(PageCompound(page)))
- page = page->first_page;
+ page = compound_head(virt_to_page(p));
new_cache = get_slab(new_size, flags);
Index: linux-2.6.21-rc5-mm4/mm/swap.c
===================================================================
--- linux-2.6.21-rc5-mm4.orig/mm/swap.c 2007-04-05 13:59:24.000000000 -0700
+++ linux-2.6.21-rc5-mm4/mm/swap.c 2007-04-05 14:00:56.000000000 -0700
@@ -56,7 +56,7 @@ static void fastcall __page_cache_releas
static void put_compound_page(struct page *page)
{
- page = (struct page *)page_private(page);
+ page = compound_head(page);
if (put_page_testzero(page)) {
compound_page_dtor *dtor;
Index: linux-2.6.21-rc5-mm4/arch/ia64/mm/init.c
===================================================================
--- linux-2.6.21-rc5-mm4.orig/arch/ia64/mm/init.c 2007-04-05 13:59:23.000000000 -0700
+++ linux-2.6.21-rc5-mm4/arch/ia64/mm/init.c 2007-04-05 14:00:56.000000000 -0700
@@ -121,7 +121,7 @@ lazy_mmu_prot_update (pte_t pte)
return; /* i-cache is already coherent with d-cache */
if (PageCompound(page)) {
- order = (unsigned long) (page[1].lru.prev);
+ order = compound_order(page);
flush_icache_range(addr, addr + (1UL << order << PAGE_SHIFT));
}
else
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 2/2] Optimize compound_head() by avoiding a shared page flag
2007-04-05 22:36 [PATCH 1/2] Make page->private usable in compound pages V1 Christoph Lameter
@ 2007-04-05 22:36 ` Christoph Lameter
2007-04-07 5:23 ` Andrew Morton
2007-04-05 23:43 ` [PATCH 1/2] Make page->private usable in compound pages V1 Andrew Morton
1 sibling, 1 reply; 19+ messages in thread
From: Christoph Lameter @ 2007-04-05 22:36 UTC (permalink / raw)
To: akpm; +Cc: Hugh Dickins, Nick Piggin, Christoph Lameter, dgc, linux-kernel
Unalias PG_tail for performance reasons
If PG_tail is an alias then we need to check PageCompound before PageTail.
This is particularly bad because the slab and others have to use these tests
in performance critical paths.
This patch uses one of the freed up software suspend flags that is defined
next to PG_compound.
Excerpt from kfree (page = compound_head(page)) before patch:
r33 = pointer to page struct.
0xa000000100170271 <kfree+49>: ld4.acq r14=[r33]
0xa000000100170272 <kfree+50>: nop.i 0x0;;
0xa000000100170280 <kfree+64>: [MIB] nop.m 0x0
0xa000000100170281 <kfree+65>: tbit.z p9,p8=r14,14
0xa000000100170282 <kfree+66>: (p09) br.cond.dptk.few 0xa0000001001702c0 <kfree+128>
0xa000000100170290 <kfree+80>: [MMI] ld4.acq r9=[r33]
0xa000000100170291 <kfree+81>: nop.m 0x0
0xa000000100170292 <kfree+82>: adds r8=16,r33;;
0xa0000001001702a0 <kfree+96>: [MII] nop.m 0x0
0xa0000001001702a1 <kfree+97>: tbit.z p10,p11=r9,17
0xa0000001001702a2 <kfree+98>: nop.i 0x0
0xa0000001001702b0 <kfree+112>: [MMI] nop.m 0x0;;
0xa0000001001702b1 <kfree+113>: (p11) ld8 r33=[r8]
0xa0000001001702b2 <kfree+114>: nop.i 0x0;;
0xa0000001001702c0 <kfree+128>: [MII] ...
After patch:
r34 pointer to page struct
0xa00000010016f541 <kfree+65>: ld4.acq r3=[r34]
0xa00000010016f542 <kfree+66>: nop.i 0x0
0xa00000010016f550 <kfree+80>: [MMI] adds r2=16,r34;;
0xa00000010016f551 <kfree+81>: nop.m 0x0
0xa00000010016f552 <kfree+82>: tbit.z p10,p11=r3,13;;
0xa00000010016f560 <kfree+96>: [MII] (p11) ld8 r34=[r2]
No branch anymore.
Signed-off-by: Christoph Lameter <clameter@sgi.com>
Index: linux-2.6.21-rc5-mm4/include/linux/page-flags.h
===================================================================
--- linux-2.6.21-rc5-mm4.orig/include/linux/page-flags.h 2007-04-05 15:18:33.000000000 -0700
+++ linux-2.6.21-rc5-mm4/include/linux/page-flags.h 2007-04-05 15:18:39.000000000 -0700
@@ -82,6 +82,7 @@
#define PG_private 11 /* If pagecache, has fs-private data */
#define PG_writeback 12 /* Page is under writeback */
+#define PG_tail 13 /* Page is tail of a compound page */
#define PG_compound 14 /* Part of a compound page */
#define PG_swapcache 15 /* Swap page: swp_entry_t in private */
@@ -95,12 +96,6 @@
/* PG_owner_priv_1 users should have descriptive aliases */
#define PG_checked PG_owner_priv_1 /* Used by some filesystems */
-/*
- * Marks tail portion of a compound page. We currently do not reclaim
- * compound pages so we can reuse a flag only used for reclaim here.
- */
-#define PG_tail PG_reclaim
-
#if (BITS_PER_LONG > 32)
/*
* 64-bit-only flags build down from bit 31
@@ -220,10 +215,6 @@ static inline void SetPageUptodate(struc
#define __SetPageCompound(page) __set_bit(PG_compound, &(page)->flags)
#define __ClearPageCompound(page) __clear_bit(PG_compound, &(page)->flags)
-/*
- * Note: PG_tail is an alias of another page flag. The result of PageTail()
- * is only valid if PageCompound(page) is true.
- */
#define PageTail(page) test_bit(PG_tail, &(page)->flags)
#define __SetPageTail(page) __set_bit(PG_tail, &(page)->flags)
#define __ClearPageTail(page) __clear_bit(PG_tail, &(page)->flags)
Index: linux-2.6.21-rc5-mm4/mm/page_alloc.c
===================================================================
--- linux-2.6.21-rc5-mm4.orig/mm/page_alloc.c 2007-04-05 15:18:33.000000000 -0700
+++ linux-2.6.21-rc5-mm4/mm/page_alloc.c 2007-04-05 15:18:39.000000000 -0700
@@ -500,18 +500,13 @@ static inline int free_pages_check(struc
1 << PG_private |
1 << PG_locked |
1 << PG_active |
+ 1 << PG_reclaim |
1 << PG_slab |
1 << PG_swapcache |
1 << PG_writeback |
1 << PG_reserved |
1 << PG_buddy ))))
bad_page(page);
- /*
- * PageReclaim == PageTail. It is only an error
- * for PageReclaim to be set if PageCompound is clear.
- */
- if (unlikely(!PageCompound(page) && PageReclaim(page)))
- bad_page(page);
if (PageDirty(page))
__ClearPageDirty(page);
/*
Index: linux-2.6.21-rc5-mm4/mm/internal.h
===================================================================
--- linux-2.6.21-rc5-mm4.orig/mm/internal.h 2007-04-05 15:18:33.000000000 -0700
+++ linux-2.6.21-rc5-mm4/mm/internal.h 2007-04-05 15:18:39.000000000 -0700
@@ -24,7 +24,7 @@ static inline void set_page_count(struct
*/
static inline void set_page_refcounted(struct page *page)
{
- VM_BUG_ON(PageCompound(page) && PageTail(page));
+ VM_BUG_ON(PageTail(page));
VM_BUG_ON(atomic_read(&page->_count));
set_page_count(page, 1);
}
Index: linux-2.6.21-rc5-mm4/include/linux/mm.h
===================================================================
--- linux-2.6.21-rc5-mm4.orig/include/linux/mm.h 2007-04-05 15:18:51.000000000 -0700
+++ linux-2.6.21-rc5-mm4/include/linux/mm.h 2007-04-05 15:19:09.000000000 -0700
@@ -299,14 +299,7 @@ static inline int get_page_unless_zero(s
static inline struct page *compound_head(struct page *page)
{
- /*
- * We could avoid the PageCompound(page) check if
- * we would not overload PageTail().
- *
- * This check has to be done in several performance critical
- * paths of the slab etc. IMHO PageTail deserves its own flag.
- */
- if (unlikely(PageCompound(page) && PageTail(page)))
+ if (unlikely(PageTail(page)))
return page->first_page;
return page;
}
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/2] Make page->private usable in compound pages V1
2007-04-05 22:36 [PATCH 1/2] Make page->private usable in compound pages V1 Christoph Lameter
2007-04-05 22:36 ` [PATCH 2/2] Optimize compound_head() by avoiding a shared page flag Christoph Lameter
@ 2007-04-05 23:43 ` Andrew Morton
2007-04-06 17:01 ` Christoph Lameter
2007-04-06 19:46 ` Christoph Lameter
1 sibling, 2 replies; 19+ messages in thread
From: Andrew Morton @ 2007-04-05 23:43 UTC (permalink / raw)
To: Christoph Lameter; +Cc: Hugh Dickins, Nick Piggin, dgc, linux-kernel
On Thu, 5 Apr 2007 15:36:51 -0700 (PDT)
Christoph Lameter <clameter@sgi.com> wrote:
> If we add a new flag so that we can distinguish between the
> first page and the tail pages then we can avoid to use page->private
> in the first page. page->private == page for the first page, so there
> is no real information in there.
>
> Freeing up page->private makes the use of compound pages more transparent.
> They become more usable like real pages. Right now we have to be careful f.e.
> if we are going beyond PAGE_SIZE allocations in the slab on i386 because we
> can then no longer use the private field. This is one of the issues that
> cause us not to support debugging for page size slabs in SLAB.
>
> Having page->private available for SLUB would allow more meta information
> in the page struct. I can probably avoid the 16 bit ints that I have in
> there right now.
>
> Also if page->private is available then a compound page may be equipped
> with buffer heads. This may free up the way for filesystems to support
> larger blocks than page size.
>
> We add PageTail as an alias of PageReclaim. Compound pages cannot
> currently be reclaimed. Because of the alias one needs to check
> PageCompound first.
So slub is using compound pages so that it can locate the head page in
higher-order pages, whereas slab uses per-object (or per-order-0-page?)
metadata for that?
I see four instances of
+ page = virt_to_page(p);
+
+ if (unlikely(PageCompound(page)))
+ page = page->first_page;
A new virt_to_head_page() is needed.
Sigh. We're seeing rather a lot of churn to accommodate slub. Do we
actually have any justification for all this? If we end up deciding to
merge slub and to deprecate then remove slab, what would our reasons have
been?
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/2] Make page->private usable in compound pages V1
2007-04-05 23:43 ` [PATCH 1/2] Make page->private usable in compound pages V1 Andrew Morton
@ 2007-04-06 17:01 ` Christoph Lameter
2007-04-06 20:04 ` Andrew Morton
2007-04-06 19:46 ` Christoph Lameter
1 sibling, 1 reply; 19+ messages in thread
From: Christoph Lameter @ 2007-04-06 17:01 UTC (permalink / raw)
To: Andrew Morton; +Cc: Hugh Dickins, Nick Piggin, dgc, linux-kernel
On Thu, 5 Apr 2007, Andrew Morton wrote:
> > We add PageTail as an alias of PageReclaim. Compound pages cannot
> > currently be reclaimed. Because of the alias one needs to check
> > PageCompound first.
>
> So slub is using compound pages so that it can locate the head page in
> higher-order pages, whereas slab uses per-object (or per-order-0-page?)
> metadata for that?
Both SLAB and SLUB use compound pages.
>
> I see four instances of
>
> + page = virt_to_page(p);
> +
> + if (unlikely(PageCompound(page)))
> + page = page->first_page;
>
> A new virt_to_head_page() is needed.
Ok.
> Sigh. We're seeing rather a lot of churn to accommodate slub. Do we
> actually have any justification for all this? If we end up deciding to
> merge slub and to deprecate then remove slab, what would our reasons have
> been?
This is not SLUB specific. SLAB does the same. SLAB does special casing
for page size slabs and does not allow slab debugging because it would
use page->private if debugging would be enabled. Which would get into
trouble on i386.
If you want less SLUB churn on other fronts then I can add the special
casing for PAGE_SIZE slabs back into SLUB. Then we keep all the
inconsistencies in SLAB use in the code.
I'd rather clean up the stuff and then also remove the special casing from
SLAB.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/2] Make page->private usable in compound pages V1
2007-04-05 23:43 ` [PATCH 1/2] Make page->private usable in compound pages V1 Andrew Morton
2007-04-06 17:01 ` Christoph Lameter
@ 2007-04-06 19:46 ` Christoph Lameter
1 sibling, 0 replies; 19+ messages in thread
From: Christoph Lameter @ 2007-04-06 19:46 UTC (permalink / raw)
To: Andrew Morton; +Cc: Hugh Dickins, Nick Piggin, dgc, linux-kernel
On Thu, 5 Apr 2007, Andrew Morton wrote:
> A new virt_to_head_page() is needed.
Add virt_to_head_page and consolidate code in slab and slub.
Signed-off-by: Christoph Lameter <clameter@sgi.com>
Index: linux-2.6.21-rc5-mm4/include/linux/mm.h
===================================================================
--- linux-2.6.21-rc5-mm4.orig/include/linux/mm.h 2007-04-06 19:35:51.000000000 +0000
+++ linux-2.6.21-rc5-mm4/include/linux/mm.h 2007-04-06 19:42:53.000000000 +0000
@@ -316,6 +316,12 @@
atomic_inc(&page->_count);
}
+static inline struct page *virt_to_head_page(const void *x)
+{
+ struct page *page = virt_to_page(x);
+ return compound_head(page);
+}
+
/*
* Setup the page count before being freed into the page allocator for
* the first time (boot or memory hotplug)
Index: linux-2.6.21-rc5-mm4/mm/slab.c
===================================================================
--- linux-2.6.21-rc5-mm4.orig/mm/slab.c 2007-04-06 19:38:58.000000000 +0000
+++ linux-2.6.21-rc5-mm4/mm/slab.c 2007-04-06 19:41:57.000000000 +0000
@@ -614,20 +614,19 @@
static inline struct slab *page_get_slab(struct page *page)
{
- page = compound_head(page);
BUG_ON(!PageSlab(page));
return (struct slab *)page->lru.prev;
}
static inline struct kmem_cache *virt_to_cache(const void *obj)
{
- struct page *page = virt_to_page(obj);
+ struct page *page = virt_to_head_page(obj);
return page_get_cache(page);
}
static inline struct slab *virt_to_slab(const void *obj)
{
- struct page *page = virt_to_page(obj);
+ struct page *page = virt_to_head_page(obj);
return page_get_slab(page);
}
@@ -2884,7 +2883,7 @@
objp -= obj_offset(cachep);
kfree_debugcheck(objp);
- page = virt_to_page(objp);
+ page = virt_to_head_page(objp);
slabp = page_get_slab(page);
@@ -3108,7 +3107,7 @@
struct slab *slabp;
unsigned objnr;
- slabp = page_get_slab(virt_to_page(objp));
+ slabp = page_get_slab(virt_to_head_page(objp));
objnr = (unsigned)(objp - slabp->s_mem) / cachep->buffer_size;
slab_bufctl(slabp)[objnr] = BUFCTL_ACTIVE;
}
Index: linux-2.6.21-rc5-mm4/mm/slub.c
===================================================================
--- linux-2.6.21-rc5-mm4.orig/mm/slub.c 2007-04-06 19:37:49.000000000 +0000
+++ linux-2.6.21-rc5-mm4/mm/slub.c 2007-04-06 19:38:56.000000000 +0000
@@ -1297,9 +1297,7 @@
{
struct page * page;
- page = virt_to_page(x);
-
- page = compound_head(page);
+ page = virt_to_head_page(x);
if (unlikely(PageError(page) && (s->flags & SLAB_STORE_USER)))
set_tracking(s, x, 1);
@@ -1310,7 +1308,7 @@
/* Figure out on which slab object the object resides */
static struct page *get_object_page(const void *x)
{
- struct page *page = compound_head(virt_to_page(x));
+ struct page *page = virt_to_head_page(x);
if (!PageSlab(page))
return NULL;
@@ -1952,7 +1950,7 @@
if (!x)
return;
- page = compound_head(virt_to_page(x));
+ page = virt_to_head_page(x);
s = page->slab;
@@ -1988,7 +1986,7 @@
return NULL;
}
- page = compound_head(virt_to_page(p));
+ page = virt_to_head_page(p);
new_cache = get_slab(new_size, flags);
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/2] Make page->private usable in compound pages V1
2007-04-06 17:01 ` Christoph Lameter
@ 2007-04-06 20:04 ` Andrew Morton
2007-04-06 20:28 ` Christoph Lameter
0 siblings, 1 reply; 19+ messages in thread
From: Andrew Morton @ 2007-04-06 20:04 UTC (permalink / raw)
To: Christoph Lameter; +Cc: Hugh Dickins, Nick Piggin, dgc, linux-kernel
On Fri, 6 Apr 2007 10:01:38 -0700 (PDT)
Christoph Lameter <clameter@sgi.com> wrote:
> On Thu, 5 Apr 2007, Andrew Morton wrote:
>
> > > We add PageTail as an alias of PageReclaim. Compound pages cannot
> > > currently be reclaimed. Because of the alias one needs to check
> > > PageCompound first.
> >
> > So slub is using compound pages so that it can locate the head page in
> > higher-order pages, whereas slab uses per-object (or per-order-0-page?)
> > metadata for that?
>
> Both SLAB and SLUB use compound pages.
Not really. slab sets the page->lru.prev of each constituent page to point
at the controlling slab.
I assume the PageCompound() handling in (for example) page_get_cache() is
for the NOMMU special case.
> >
> > I see four instances of
> >
> > + page = virt_to_page(p);
> > +
> > + if (unlikely(PageCompound(page)))
> > + page = page->first_page;
> >
> > A new virt_to_head_page() is needed.
>
> Ok.
>
> > Sigh. We're seeing rather a lot of churn to accommodate slub. Do we
> > actually have any justification for all this? If we end up deciding to
> > merge slub and to deprecate then remove slab, what would our reasons have
> > been?
>
> This is not SLUB specific. SLAB does the same. SLAB does special casing
> for page size slabs and does not allow slab debugging because it would
> use page->private if debugging would be enabled. Which would get into
> trouble on i386.
>
> If you want less SLUB churn on other fronts then I can add the special
> casing for PAGE_SIZE slabs back into SLUB. Then we keep all the
> inconsistencies in SLAB use in the code.
>
> I'd rather clean up the stuff and then also remove the special casing from
> SLAB.
>
Will slub handle NOMMU anonymous pages appropriately?
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/2] Make page->private usable in compound pages V1
2007-04-06 20:04 ` Andrew Morton
@ 2007-04-06 20:28 ` Christoph Lameter
0 siblings, 0 replies; 19+ messages in thread
From: Christoph Lameter @ 2007-04-06 20:28 UTC (permalink / raw)
To: Andrew Morton; +Cc: Hugh Dickins, Nick Piggin, dgc, linux-kernel
On Fri, 6 Apr 2007, Andrew Morton wrote:
> > Both SLAB and SLUB use compound pages.
>
> Not really. slab sets the page->lru.prev of each constituent page to point
> at the controlling slab.
Uhh.. More slab inconsistencies. page[1]->lru is used for compound
pages in the !MMU case.
>From page_alloc.c:
/*
* Higher-order pages are called "compound pages". They are structured
thusly:
*
* The first tail page's ->lru.next holds the address of the compound
page's
* put_page() function. Its ->lru.prev holds the order of allocation.
* This usage means that zero-order pages may not be compound.
*/
> Will slub handle NOMMU anonymous pages appropriately?
I am not sure how anonymous pages relate to the slab allocators.
SLUB consistently uses compound pages for all higher order allocations.
And those are usually fine for mmu and no mmu even without this
series of patches unless someone pokes around page->private.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] Optimize compound_head() by avoiding a shared page flag
2007-04-05 22:36 ` [PATCH 2/2] Optimize compound_head() by avoiding a shared page flag Christoph Lameter
@ 2007-04-07 5:23 ` Andrew Morton
2007-04-07 22:16 ` Christoph Lameter
0 siblings, 1 reply; 19+ messages in thread
From: Andrew Morton @ 2007-04-07 5:23 UTC (permalink / raw)
To: Christoph Lameter; +Cc: Hugh Dickins, Nick Piggin, dgc, linux-kernel
On Thu, 5 Apr 2007 15:36:57 -0700 (PDT) Christoph Lameter <clameter@sgi.com> wrote:
> Unalias PG_tail for performance reasons
>
> If PG_tail is an alias then we need to check PageCompound before PageTail.
> This is particularly bad because the slab and others have to use these tests
> in performance critical paths.
>
> This patch uses one of the freed up software suspend flags that is defined
> next to PG_compound.
I get a reject from this because it is dependent upon later patches. As
the Official Protector Of Page Flags, I'm going to drop it.
Did you investigate
static inline int page_tail(struct page *page)
{
return ((page->flags & (PG_compound|PG_tail)) == (PG_compound|PG_tail));
}
and
static inline int page_tail(struct page *page)
{
return unlikely(PageCompound(page)) && unlikely(PageTail(page));
}
?
In the latter case we _should_ have a not-taken branch to not-inline code.
If the compiler doesn't do that, we can make the PageTail() test an
out-of-line function. Or make the whole thing an uninlined function.
More work needed, please. I don't expect that a not-taken branch to
not-inline code is worth a new page flag. Especially as it does not
actually reduce the number of branch decisions in the common case.
(I'm assuming in all of this that !PageCompound() is the very common case
with slub. If that is not true, we need to talk).
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] Optimize compound_head() by avoiding a shared page flag
2007-04-07 5:23 ` Andrew Morton
@ 2007-04-07 22:16 ` Christoph Lameter
2007-04-07 22:51 ` Andrew Morton
0 siblings, 1 reply; 19+ messages in thread
From: Christoph Lameter @ 2007-04-07 22:16 UTC (permalink / raw)
To: Andrew Morton; +Cc: Hugh Dickins, Nick Piggin, dgc, linux-kernel
On Fri, 6 Apr 2007, Andrew Morton wrote:
> Did you investigate
>
> static inline int page_tail(struct page *page)
> {
> return ((page->flags & (PG_compound|PG_tail)) == (PG_compound|PG_tail));
> }
The usual test_bit that we are using there uses a volatile reference
so these wont be combined if I check them separately.
A working example of the above would be much uglier:
static inline int page_tail(struct page *page)
{
return ((page->flags & ((1L << PG_compound)|(1L << PG_tail))) ==
((1L << PG_compound)|(1L << PG_tail)));
}
May be this can be cleaned up somehow.
>
> and
>
> static inline int page_tail(struct page *page)
> {
> return unlikely(PageCompound(page)) && unlikely(PageTail(page));
> }
Two volatile references in the bit opes that the compiler cannot combine.
Wont work unless we clean up the bitops first. This means we still have
two branches in the code. Maybe I can make the first one work.
> In the latter case we _should_ have a not-taken branch to not-inline code.
> If the compiler doesn't do that, we can make the PageTail() test an
> out-of-line function. Or make the whole thing an uninlined function.
Still two branches which cannot be optimized in the same way as the single
on on IA64 as shown by the asm that I included.
> More work needed, please. I don't expect that a not-taken branch to
> not-inline code is worth a new page flag. Especially as it does not
> actually reduce the number of branch decisions in the common case.
A new page flag does reduce the number of branches. On several platforms
it eliminates the branch completely since a single instruction can be
conditionally skipped.
> (I'm assuming in all of this that !PageCompound() is the very common case
> with slub. If that is not true, we need to talk).
Yes, it is common for slabs to only have a single page.
The most promising avenue seems to be the simultaneous check for two bits.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] Optimize compound_head() by avoiding a shared page flag
2007-04-07 22:16 ` Christoph Lameter
@ 2007-04-07 22:51 ` Andrew Morton
2007-04-08 0:21 ` Christoph Lameter
0 siblings, 1 reply; 19+ messages in thread
From: Andrew Morton @ 2007-04-07 22:51 UTC (permalink / raw)
To: Christoph Lameter; +Cc: Hugh Dickins, Nick Piggin, dgc, linux-kernel
On Sat, 7 Apr 2007 15:16:17 -0700 (PDT) Christoph Lameter <clameter@sgi.com> wrote:
> On Fri, 6 Apr 2007, Andrew Morton wrote:
>
> > Did you investigate
> >
> > static inline int page_tail(struct page *page)
> > {
> > return ((page->flags & (PG_compound|PG_tail)) == (PG_compound|PG_tail));
> > }
>
> The usual test_bit that we are using there uses a volatile reference
> so these wont be combined if I check them separately.
>
> A working example of the above would be much uglier:
>
> static inline int page_tail(struct page *page)
> {
> return ((page->flags & ((1L << PG_compound)|(1L << PG_tail))) ==
> ((1L << PG_compound)|(1L << PG_tail)));
> }
>
> May be this can be cleaned up somehow.
It might generate better code to do
unsigned long compound;
compound = page->flags & (1 << PG_compound);
if (PG_compound > PG_tail)
return compound & (page->flags << (PG_compound - PG_tail));
else
return compound & (page->flags << (PG_tail - PG_compound));
ie: get the PG_compound flag into `compound', then bitwise-and that with
the PG_tail flag, after shifting it into PG_compound' slot. The return
value will be zero if either bit is clear, (1<<PG_compound) if both are
set. The `if (PG_compound > PG_tail)' will be swallowed by the compiler.
The compiler should turn it all into
(page->flags & N) & (page->flags << M)
Which may or may not be better than (page->flags & N == N), dunno.
Probably not - if the compiler's any good it won't save a branch, I
suspect.
Which is all a ton of fun, but this subversion of the architecture's
freedom to use volatile, memory barriers etc is a worry. We do the same in
page_alloc.c, of course...
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] Optimize compound_head() by avoiding a shared page flag
2007-04-07 22:51 ` Andrew Morton
@ 2007-04-08 0:21 ` Christoph Lameter
2007-04-08 1:25 ` Andrew Morton
0 siblings, 1 reply; 19+ messages in thread
From: Christoph Lameter @ 2007-04-08 0:21 UTC (permalink / raw)
To: Andrew Morton; +Cc: Hugh Dickins, Nick Piggin, dgc, linux-kernel
On Sat, 7 Apr 2007, Andrew Morton wrote:
> Which is all a ton of fun, but this subversion of the architecture's
> freedom to use volatile, memory barriers etc is a worry. We do the same in
> page_alloc.c, of course...
I just tried the approach that we discussed earlier and it was not
nice either. Lets just use a page flag please. This check will be in
several hot code paths. And it may become more important because the file
system folks want to support buffers > page size. Then we may want more
transparent support for huge pages... For all of this page->private gets
in the way.
And I think we curently have 5 or so page flags available?
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] Optimize compound_head() by avoiding a shared page flag
2007-04-08 0:21 ` Christoph Lameter
@ 2007-04-08 1:25 ` Andrew Morton
2007-04-08 1:32 ` Christoph Lameter
0 siblings, 1 reply; 19+ messages in thread
From: Andrew Morton @ 2007-04-08 1:25 UTC (permalink / raw)
To: Christoph Lameter; +Cc: Hugh Dickins, Nick Piggin, dgc, linux-kernel
On Sat, 7 Apr 2007 17:21:38 -0700 (PDT) Christoph Lameter <clameter@sgi.com> wrote:
> On Sat, 7 Apr 2007, Andrew Morton wrote:
>
> > Which is all a ton of fun, but this subversion of the architecture's
> > freedom to use volatile, memory barriers etc is a worry. We do the same in
> > page_alloc.c, of course...
>
> I just tried the approach that we discussed earlier and it was not
> nice either.
We've discussed at least three approaches, so we don't know to what you refer.
> Lets just use a page flag please.
Nope, try harder.
PageCompound is an unlikely case. Back in the old days we would have done
if (PageCompound(page))
goto out_of_line;
back:
do_stuff_with(page);
return;
out_of_line:
if (PageTail(page)) {
page = page_tail(page);
goto back;
}
<do other stuff>
and nowadays we hope that gcc does the above for us. If it doesn't do it
for us, perhaps it needs open-coded help.
Because I don't expect there will be much efficiency difference between the
above and the use of another page flag.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] Optimize compound_head() by avoiding a shared page flag
2007-04-08 1:25 ` Andrew Morton
@ 2007-04-08 1:32 ` Christoph Lameter
2007-04-08 1:48 ` Andrew Morton
0 siblings, 1 reply; 19+ messages in thread
From: Christoph Lameter @ 2007-04-08 1:32 UTC (permalink / raw)
To: Andrew Morton; +Cc: Hugh Dickins, Nick Piggin, dgc, linux-kernel
On Sat, 7 Apr 2007, Andrew Morton wrote:
> > I just tried the approach that we discussed earlier and it was not
> > nice either.
>
> We've discussed at least three approaches, so we don't know to what you refer.
Thats the approach of checking two flags at the same time. In that case
the compiler will generate and "and-immediate" and then a
"compare-immediate" one branch but .... Yuck.
> Because I don't expect there will be much efficiency difference between the
> above and the use of another page flag.
Then we end up with all these small efficiency differences in all
the code paths. I'd rather go for optimal performance in a frequently used
construct like this.
This check is not rare. It is done on every SLAB free and on every
get_page() and put_page(). Lets do the page flag.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] Optimize compound_head() by avoiding a shared page flag
2007-04-08 1:32 ` Christoph Lameter
@ 2007-04-08 1:48 ` Andrew Morton
2007-04-09 17:22 ` Christoph Lameter
2007-04-09 18:09 ` Christoph Lameter
0 siblings, 2 replies; 19+ messages in thread
From: Andrew Morton @ 2007-04-08 1:48 UTC (permalink / raw)
To: Christoph Lameter; +Cc: Hugh Dickins, Nick Piggin, dgc, linux-kernel
On Sat, 7 Apr 2007 18:32:04 -0700 (PDT) Christoph Lameter <clameter@sgi.com> wrote:
> On Sat, 7 Apr 2007, Andrew Morton wrote:
>
> > > I just tried the approach that we discussed earlier and it was not
> > > nice either.
> >
> > We've discussed at least three approaches, so we don't know to what you refer.
>
> Thats the approach of checking two flags at the same time. In that case
> the compiler will generate and "and-immediate" and then a
> "compare-immediate" one branch but .... Yuck.
Right.
movl (%ebx), %eax # <variable>.flags, tmp399
andl $48, %eax #, tmp399
cmpl $48, %eax #, tmp399
je .L265 #,
what's "yuck" about that?
With the single page flag:
movl (%ebx), %eax #* page.521, D.21940
testb $32, %al #, D.21940
jne .L265 #,
So you're talking about saving one sole single silly solitary instruction.
> > Because I don't expect there will be much efficiency difference between the
> > above and the use of another page flag.
>
> Then we end up with all these small efficiency differences in all
> the code paths. I'd rather go for optimal performance in a frequently used
> construct like this.
You can save that worrisome single instruction in the common case by putting the
handling of the uncommon compound pages out of line, as I indicated.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] Optimize compound_head() by avoiding a shared page flag
2007-04-08 1:48 ` Andrew Morton
@ 2007-04-09 17:22 ` Christoph Lameter
2007-04-09 18:09 ` Christoph Lameter
1 sibling, 0 replies; 19+ messages in thread
From: Christoph Lameter @ 2007-04-09 17:22 UTC (permalink / raw)
To: Andrew Morton; +Cc: Hugh Dickins, Nick Piggin, dgc, linux-kernel
On Sat, 7 Apr 2007, Andrew Morton wrote:
> andl $48, %eax #, tmp399
> cmpl $48, %eax #, tmp399
> je .L265 #,
>
> what's "yuck" about that?
>
> With the single page flag:
>
> movl (%ebx), %eax #* page.521, D.21940
> testb $32, %al #, D.21940
> jne .L265 #,
>
> So you're talking about saving one sole single silly solitary instruction.
We are also using another register. The bit instructions can just go in an
test. Ok. Will submit this patch. .... Maybe I can special case 64 bit.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] Optimize compound_head() by avoiding a shared page flag
2007-04-08 1:48 ` Andrew Morton
2007-04-09 17:22 ` Christoph Lameter
@ 2007-04-09 18:09 ` Christoph Lameter
2007-04-09 18:45 ` Andrew Morton
1 sibling, 1 reply; 19+ messages in thread
From: Christoph Lameter @ 2007-04-09 18:09 UTC (permalink / raw)
To: Andrew Morton; +Cc: Hugh Dickins, Nick Piggin, dgc, linux-kernel
Add PageTail / PageHead in order to avoid multiple branches when compound
pages are checked.
The patch adds PageTail(page) and PageHead(page) to check if a page is the
head or the tail of a compound page. This is done by masking the two
bits describing the state of a compound page and then comparing them. So
one comparision and a branch instead of two bit checks and two branches.
---
include/linux/mm.h | 11 ++---------
include/linux/page-flags.h | 28 +++++++++++++++++-----------
mm/page_alloc.c | 10 ++++------
3 files changed, 23 insertions(+), 26 deletions(-)
Index: linux-2.6.21-rc5-mm4/include/linux/mm.h
===================================================================
--- linux-2.6.21-rc5-mm4.orig/include/linux/mm.h 2007-04-09 11:04:15.000000000 -0700
+++ linux-2.6.21-rc5-mm4/include/linux/mm.h 2007-04-09 11:04:31.000000000 -0700
@@ -299,14 +299,7 @@ static inline int get_page_unless_zero(s
static inline struct page *compound_head(struct page *page)
{
- /*
- * We could avoid the PageCompound(page) check if
- * we would not overload PageTail().
- *
- * This check has to be done in several performance critical
- * paths of the slab etc. IMHO PageTail deserves its own flag.
- */
- if (unlikely(PageCompound(page) && PageTail(page)))
+ if (unlikely(PageTail(page)))
return page->first_page;
return page;
}
@@ -363,7 +356,7 @@ static inline compound_page_dtor *get_co
static inline int compound_order(struct page *page)
{
- if (!PageCompound(page) || PageTail(page))
+ if (!PageHead(page))
return 0;
return (unsigned long)page[1].lru.prev;
}
Index: linux-2.6.21-rc5-mm4/include/linux/page-flags.h
===================================================================
--- linux-2.6.21-rc5-mm4.orig/include/linux/page-flags.h 2007-04-09 11:04:10.000000000 -0700
+++ linux-2.6.21-rc5-mm4/include/linux/page-flags.h 2007-04-09 11:04:31.000000000 -0700
@@ -95,12 +95,6 @@
/* PG_owner_priv_1 users should have descriptive aliases */
#define PG_checked PG_owner_priv_1 /* Used by some filesystems */
-/*
- * Marks tail portion of a compound page. We currently do not reclaim
- * compound pages so we can reuse a flag only used for reclaim here.
- */
-#define PG_tail PG_reclaim
-
#if (BITS_PER_LONG > 32)
/*
* 64-bit-only flags build down from bit 31
@@ -221,12 +215,24 @@ static inline void SetPageUptodate(struc
#define __ClearPageCompound(page) __clear_bit(PG_compound, &(page)->flags)
/*
- * Note: PG_tail is an alias of another page flag. The result of PageTail()
- * is only valid if PageCompound(page) is true.
+ * PG_reclaim is used in combination with PG_compound to mark the
+ * head and tail of a compound page
+ *
+ * PG_compound & PG_reclaim => Tail page
+ * PG_compound & ~PG_reclaim => Head page
*/
-#define PageTail(page) test_bit(PG_tail, &(page)->flags)
-#define __SetPageTail(page) __set_bit(PG_tail, &(page)->flags)
-#define __ClearPageTail(page) __clear_bit(PG_tail, &(page)->flags)
+
+#define PG_head_tail_mask ((1L << PG_compound) | (1L << PG_reclaim))
+
+#define PageTail(page) ((page->flags & PG_head_tail_mask) \
+ == PG_head_tail_mask)
+#define __SetPageTail(page) page->flags |= PG_head_tail_mask
+#define __ClearPageTail(page) page->flags ~= PG_head_tail_mask
+
+#define PageHead(page) ((page->flags & PG_head_tail_mask) \
+ == (1L << PG_compound))
+#define __SetPageHead(page) __SetCompoundPage(page)
+#define __ClearPageHead(page) __ClearCompoundPage(page)
#ifdef CONFIG_SWAP
#define PageSwapCache(page) test_bit(PG_swapcache, &(page)->flags)
Index: linux-2.6.21-rc5-mm4/mm/page_alloc.c
===================================================================
--- linux-2.6.21-rc5-mm4.orig/mm/page_alloc.c 2007-04-09 11:04:10.000000000 -0700
+++ linux-2.6.21-rc5-mm4/mm/page_alloc.c 2007-04-09 11:04:31.000000000 -0700
@@ -300,12 +300,11 @@ static void prep_compound_page(struct pa
set_compound_page_dtor(page, free_compound_page);
set_compound_order(page, order);
- __SetPageCompound(page);
+ __SetPageHead(page);
for (i = 1; i < nr_pages; i++) {
struct page *p = page + i;
__SetPageTail(p);
- __SetPageCompound(p);
p->first_page = page;
}
}
@@ -318,17 +317,16 @@ static void destroy_compound_page(struct
if (unlikely(compound_order(page) != order))
bad_page(page);
- if (unlikely(!PageCompound(page)))
+ if (unlikely(!PageHead(page)))
bad_page(page);
- __ClearPageCompound(page);
+ __ClearPageHead(page);
for (i = 1; i < nr_pages; i++) {
struct page *p = page + i;
- if (unlikely(!PageCompound(p) | !PageTail(p) |
+ if (unlikely(!PageTail(p) |
(p->first_page != page)))
bad_page(page);
__ClearPageTail(p);
- __ClearPageCompound(p);
}
}
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] Optimize compound_head() by avoiding a shared page flag
2007-04-09 18:09 ` Christoph Lameter
@ 2007-04-09 18:45 ` Andrew Morton
2007-04-09 18:49 ` Christoph Lameter
2007-04-09 22:05 ` Christoph Lameter
0 siblings, 2 replies; 19+ messages in thread
From: Andrew Morton @ 2007-04-09 18:45 UTC (permalink / raw)
To: Christoph Lameter; +Cc: Hugh Dickins, Nick Piggin, dgc, linux-kernel
On Mon, 9 Apr 2007 11:09:40 -0700 (PDT)
Christoph Lameter <clameter@sgi.com> wrote:
> Add PageTail / PageHead in order to avoid multiple branches when compound
> pages are checked.
>
> The patch adds PageTail(page) and PageHead(page) to check if a page is the
> head or the tail of a compound page. This is done by masking the two
> bits describing the state of a compound page and then comparing them. So
> one comparision and a branch instead of two bit checks and two branches.
>
OK. I'm still a bit concerned about bypassing the bitops synchronisation:
barriers, volatile, etc. We had lengthy ruminations on that a few years
ago, I think when working on free_pages_check().
> @@ -221,12 +215,24 @@ static inline void SetPageUptodate(struc
> #define __ClearPageCompound(page) __clear_bit(PG_compound, &(page)->flags)
>
> /*
> - * Note: PG_tail is an alias of another page flag. The result of PageTail()
> - * is only valid if PageCompound(page) is true.
> + * PG_reclaim is used in combination with PG_compound to mark the
> + * head and tail of a compound page
> + *
> + * PG_compound & PG_reclaim => Tail page
> + * PG_compound & ~PG_reclaim => Head page
> */
> -#define PageTail(page) test_bit(PG_tail, &(page)->flags)
> -#define __SetPageTail(page) __set_bit(PG_tail, &(page)->flags)
> -#define __ClearPageTail(page) __clear_bit(PG_tail, &(page)->flags)
> +
> +#define PG_head_tail_mask ((1L << PG_compound) | (1L << PG_reclaim))
> +
> +#define PageTail(page) ((page->flags & PG_head_tail_mask) \
> + == PG_head_tail_mask)
> +#define __SetPageTail(page) page->flags |= PG_head_tail_mask
> +#define __ClearPageTail(page) page->flags ~= PG_head_tail_mask
hm. The lack of parenthesisation here _might_ be OK, but I haven't
thought it through.
And I'd prefer not to have to, because I know that the do { } while (0)
thing works. As do static inline functions.
> +#define PageHead(page) ((page->flags & PG_head_tail_mask) \
> + == (1L << PG_compound))
> +#define __SetPageHead(page) __SetCompoundPage(page)
> +#define __ClearPageHead(page) __ClearCompoundPage(page)
You meant __SetPageCompound and __ClearPageCompound.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] Optimize compound_head() by avoiding a shared page flag
2007-04-09 18:45 ` Andrew Morton
@ 2007-04-09 18:49 ` Christoph Lameter
2007-04-09 22:05 ` Christoph Lameter
1 sibling, 0 replies; 19+ messages in thread
From: Christoph Lameter @ 2007-04-09 18:49 UTC (permalink / raw)
To: Andrew Morton; +Cc: Hugh Dickins, Nick Piggin, dgc, linux-kernel
On Mon, 9 Apr 2007, Andrew Morton wrote:
> > The patch adds PageTail(page) and PageHead(page) to check if a page is the
> > head or the tail of a compound page. This is done by masking the two
> > bits describing the state of a compound page and then comparing them. So
> > one comparision and a branch instead of two bit checks and two branches.
> >
>
> OK. I'm still a bit concerned about bypassing the bitops synchronisation:
> barriers, volatile, etc. We had lengthy ruminations on that a few years
> ago, I think when working on free_pages_check().
Those are important for bits that are changing while a page is in use.
These are constant.
> > +#define __SetPageTail(page) page->flags |= PG_head_tail_mask
> > +#define __ClearPageTail(page) page->flags ~= PG_head_tail_mask
>
> hm. The lack of parenthesisation here _might_ be OK, but I haven't
> thought it through.
This is a command and not an expression. Ok will inline.
> And I'd prefer not to have to, because I know that the do { } while (0)
> thing works. As do static inline functions.
>
> > +#define PageHead(page) ((page->flags & PG_head_tail_mask) \
> > + == (1L << PG_compound))
> > +#define __SetPageHead(page) __SetCompoundPage(page)
> > +#define __ClearPageHead(page) __ClearCompoundPage(page)
>
> You meant __SetPageCompound and __ClearPageCompound.
Right.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] Optimize compound_head() by avoiding a shared page flag
2007-04-09 18:45 ` Andrew Morton
2007-04-09 18:49 ` Christoph Lameter
@ 2007-04-09 22:05 ` Christoph Lameter
1 sibling, 0 replies; 19+ messages in thread
From: Christoph Lameter @ 2007-04-09 22:05 UTC (permalink / raw)
To: Andrew Morton; +Cc: Hugh Dickins, Nick Piggin, dgc, linux-kernel
Here is another rev on that one. Inlined the two functions but that
required #including <mm_types.h>
Add PageTail / PageHead in order to avoid multiple branches when compound
pages are checked.
The patch adds PageTail(page) and PageHead(page) to check if a page is the
head or the tail of a compound page. This is done by masking the two
bits describing the state of a compound page and then comparing them. So
one comparision and a branch instead of two bit checks and two branches.
---
include/linux/mm.h | 11 ++---------
include/linux/page-flags.h | 28 +++++++++++++++++-----------
mm/page_alloc.c | 10 ++++------
3 files changed, 23 insertions(+), 26 deletions(-)
Index: linux-2.6.21-rc6-mm1/include/linux/mm.h
===================================================================
--- linux-2.6.21-rc6-mm1.orig/include/linux/mm.h 2007-04-09 12:08:33.000000000 -0700
+++ linux-2.6.21-rc6-mm1/include/linux/mm.h 2007-04-09 12:09:50.000000000 -0700
@@ -299,14 +299,7 @@ static inline int get_page_unless_zero(s
static inline struct page *compound_head(struct page *page)
{
- /*
- * We could avoid the PageCompound(page) check if
- * we would not overload PageTail().
- *
- * This check has to be done in several performance critical
- * paths of the slab etc. IMHO PageTail deserves its own flag.
- */
- if (unlikely(PageCompound(page) && PageTail(page)))
+ if (unlikely(PageTail(page)))
return page->first_page;
return page;
}
@@ -363,7 +356,7 @@ static inline compound_page_dtor *get_co
static inline int compound_order(struct page *page)
{
- if (!PageCompound(page) || PageTail(page))
+ if (!PageHead(page))
return 0;
return (unsigned long)page[1].lru.prev;
}
Index: linux-2.6.21-rc6-mm1/include/linux/page-flags.h
===================================================================
--- linux-2.6.21-rc6-mm1.orig/include/linux/page-flags.h 2007-04-09 12:08:33.000000000 -0700
+++ linux-2.6.21-rc6-mm1/include/linux/page-flags.h 2007-04-09 12:13:36.000000000 -0700
@@ -6,6 +6,7 @@
#define PAGE_FLAGS_H
#include <linux/types.h>
+#include <linux/mm_types.h>
/*
* Various page->flags bits:
@@ -95,12 +96,6 @@
/* PG_owner_priv_1 users should have descriptive aliases */
#define PG_checked PG_owner_priv_1 /* Used by some filesystems */
-/*
- * Marks tail portion of a compound page. We currently do not reclaim
- * compound pages so we can reuse a flag only used for reclaim here.
- */
-#define PG_tail PG_reclaim
-
#if (BITS_PER_LONG > 32)
/*
* 64-bit-only flags build down from bit 31
@@ -221,12 +216,32 @@ static inline void SetPageUptodate(struc
#define __ClearPageCompound(page) __clear_bit(PG_compound, &(page)->flags)
/*
- * Note: PG_tail is an alias of another page flag. The result of PageTail()
- * is only valid if PageCompound(page) is true.
+ * PG_reclaim is used in combination with PG_compound to mark the
+ * head and tail of a compound page
+ *
+ * PG_compound & PG_reclaim => Tail page
+ * PG_compound & ~PG_reclaim => Head page
*/
-#define PageTail(page) test_bit(PG_tail, &(page)->flags)
-#define __SetPageTail(page) __set_bit(PG_tail, &(page)->flags)
-#define __ClearPageTail(page) __clear_bit(PG_tail, &(page)->flags)
+
+#define PG_head_tail_mask ((1L << PG_compound) | (1L << PG_reclaim))
+
+#define PageTail(page) ((page->flags & PG_head_tail_mask) \
+ == PG_head_tail_mask)
+
+static inline void __SetPageTail(struct page *page)
+{
+ page->flags |= PG_head_tail_mask;
+}
+
+static inline void __ClearPageTail(struct page *page)
+{
+ page->flags &= ~PG_head_tail_mask;
+}
+
+#define PageHead(page) ((page->flags & PG_head_tail_mask) \
+ == (1L << PG_compound))
+#define __SetPageHead(page) __SetPageCompound(page)
+#define __ClearPageHead(page) __ClearPageCompound(page)
#ifdef CONFIG_SWAP
#define PageSwapCache(page) test_bit(PG_swapcache, &(page)->flags)
Index: linux-2.6.21-rc6-mm1/mm/page_alloc.c
===================================================================
--- linux-2.6.21-rc6-mm1.orig/mm/page_alloc.c 2007-04-09 12:08:33.000000000 -0700
+++ linux-2.6.21-rc6-mm1/mm/page_alloc.c 2007-04-09 12:09:50.000000000 -0700
@@ -300,12 +300,11 @@ static void prep_compound_page(struct pa
set_compound_page_dtor(page, free_compound_page);
set_compound_order(page, order);
- __SetPageCompound(page);
+ __SetPageHead(page);
for (i = 1; i < nr_pages; i++) {
struct page *p = page + i;
__SetPageTail(p);
- __SetPageCompound(p);
p->first_page = page;
}
}
@@ -318,17 +317,16 @@ static void destroy_compound_page(struct
if (unlikely(compound_order(page) != order))
bad_page(page);
- if (unlikely(!PageCompound(page)))
+ if (unlikely(!PageHead(page)))
bad_page(page);
- __ClearPageCompound(page);
+ __ClearPageHead(page);
for (i = 1; i < nr_pages; i++) {
struct page *p = page + i;
- if (unlikely(!PageCompound(p) | !PageTail(p) |
+ if (unlikely(!PageTail(p) |
(p->first_page != page)))
bad_page(page);
__ClearPageTail(p);
- __ClearPageCompound(p);
}
}
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2007-04-09 22:05 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-04-05 22:36 [PATCH 1/2] Make page->private usable in compound pages V1 Christoph Lameter
2007-04-05 22:36 ` [PATCH 2/2] Optimize compound_head() by avoiding a shared page flag Christoph Lameter
2007-04-07 5:23 ` Andrew Morton
2007-04-07 22:16 ` Christoph Lameter
2007-04-07 22:51 ` Andrew Morton
2007-04-08 0:21 ` Christoph Lameter
2007-04-08 1:25 ` Andrew Morton
2007-04-08 1:32 ` Christoph Lameter
2007-04-08 1:48 ` Andrew Morton
2007-04-09 17:22 ` Christoph Lameter
2007-04-09 18:09 ` Christoph Lameter
2007-04-09 18:45 ` Andrew Morton
2007-04-09 18:49 ` Christoph Lameter
2007-04-09 22:05 ` Christoph Lameter
2007-04-05 23:43 ` [PATCH 1/2] Make page->private usable in compound pages V1 Andrew Morton
2007-04-06 17:01 ` Christoph Lameter
2007-04-06 20:04 ` Andrew Morton
2007-04-06 20:28 ` Christoph Lameter
2007-04-06 19:46 ` Christoph Lameter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox