linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [RFC] Free up page->private for compound pages
@ 2007-04-05  3:19 Christoph Lameter
  2007-04-05  3:36 ` Nick Piggin
  2007-04-05 15:13 ` Dave Kleikamp
  0 siblings, 2 replies; 14+ messages in thread
From: Christoph Lameter @ 2007-04-05  3:19 UTC (permalink / raw)
  To: linux-mm; +Cc: dgc, npiggin

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 of a compound 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 
SLUB 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.

This is also doing some cleanup of all the functions that look for the 
head of a compound page via compound_head() and provides a function that
determines the order of a compound page.

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-03 23:48:34.000000000 -0700
+++ linux-2.6.21-rc5-mm4/include/linux/mm.h	2007-04-04 18:18:33.000000000 -0700
@@ -297,17 +297,21 @@ static inline int get_page_unless_zero(s
 	return atomic_inc_not_zero(&page->_count);
 }
 
+static inline struct page *compound_head(struct page *page)
+{
+	if (unlikely(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 +348,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-03 23:48:34.000000000 -0700
+++ linux-2.6.21-rc5-mm4/include/linux/page-flags.h	2007-04-04 18:25:47.000000000 -0700
@@ -91,6 +91,7 @@
 #define PG_booked		20	/* Has blocks reserved on-disk */
 
 #define PG_readahead		21	/* Reminder to do read-ahead */
+#define PG_tail			22	/* Tail portion of a compound page */
 
 /* PG_owner_priv_1 users should have descriptive aliases */
 #define PG_checked		PG_owner_priv_1 /* Used by some filesystems */
@@ -214,6 +215,10 @@ static inline void SetPageUptodate(struc
 #define __SetPageCompound(page)	__set_bit(PG_compound, &(page)->flags)
 #define __ClearPageCompound(page) __clear_bit(PG_compound, &(page)->flags)
 
+#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-03-25 15:56:23.000000000 -0700
+++ linux-2.6.21-rc5-mm4/mm/internal.h	2007-04-04 17:56:08.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(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-03 23:48:34.000000000 -0700
+++ linux-2.6.21-rc5-mm4/mm/page_alloc.c	2007-04-04 18:25:21.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,10 +299,12 @@ 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++) {
+	__SetPageCompound(page);
+	set_compound_order(page, order);
+	for (i = 1; i < nr_pages; i++) {
 		struct page *p = page + i;
 
+		__SetPageTail(p);
 		__SetPageCompound(p);
 		set_page_private(p, (unsigned long)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) |
+		if (unlikely(!PageTail(p) | !PageCompound(p) |
 				(page_private(p) != (unsigned long)page)))
 			bad_page(page);
+		__ClearPageTail(p);
 		__ClearPageCompound(p);
 	}
 }
Index: linux-2.6.21-rc5-mm4/mm/slab.c
===================================================================
--- linux-2.6.21-rc5-mm4.orig/mm/slab.c	2007-04-03 23:48:34.000000000 -0700
+++ linux-2.6.21-rc5-mm4/mm/slab.c	2007-04-04 18:05:52.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-04 17:59:30.000000000 -0700
+++ linux-2.6.21-rc5-mm4/mm/slub.c	2007-04-04 18:01:01.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-04 18:01:08.000000000 -0700
+++ linux-2.6.21-rc5-mm4/mm/swap.c	2007-04-04 18:01:40.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-04 18:09:44.000000000 -0700
+++ linux-2.6.21-rc5-mm4/arch/ia64/mm/init.c	2007-04-04 18:15:48.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

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC] Free up page->private for compound pages
  2007-04-05  3:19 [RFC] Free up page->private for compound pages Christoph Lameter
@ 2007-04-05  3:36 ` Nick Piggin
  2007-04-05  3:38   ` Christoph Lameter
  2007-04-05 15:13 ` Dave Kleikamp
  1 sibling, 1 reply; 14+ messages in thread
From: Nick Piggin @ 2007-04-05  3:36 UTC (permalink / raw)
  To: Christoph Lameter; +Cc: linux-mm, dgc

On Wed, Apr 04, 2007 at 08:19:22PM -0700, Christoph Lameter 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 of a compound page. page->private == page for the first 
> page, so there is no real information in there.

Couldn't you use something like PageActive for the head page instead of
taking up a new flag, seeing as we don't put compound pages on the LRU?

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC] Free up page->private for compound pages
  2007-04-05  3:36 ` Nick Piggin
@ 2007-04-05  3:38   ` Christoph Lameter
  2007-04-05  3:57     ` Nick Piggin
  0 siblings, 1 reply; 14+ messages in thread
From: Christoph Lameter @ 2007-04-05  3:38 UTC (permalink / raw)
  To: Nick Piggin; +Cc: linux-mm, dgc

On Thu, 5 Apr 2007, Nick Piggin wrote:

> On Wed, Apr 04, 2007 at 08:19:22PM -0700, Christoph Lameter 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 of a compound page. page->private == page for the first 
> > page, so there is no real information in there.
> 
> Couldn't you use something like PageActive for the head page instead of
> taking up a new flag, seeing as we don't put compound pages on the LRU?

We could set up an alias for now?

If we start supporting compound pages for filesystem then we will have
these pages on the LRU.

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC] Free up page->private for compound pages
  2007-04-05  3:38   ` Christoph Lameter
@ 2007-04-05  3:57     ` Nick Piggin
  2007-04-05  4:04       ` Christoph Lameter
  0 siblings, 1 reply; 14+ messages in thread
From: Nick Piggin @ 2007-04-05  3:57 UTC (permalink / raw)
  To: Christoph Lameter; +Cc: linux-mm, dgc

On Wed, Apr 04, 2007 at 08:38:52PM -0700, Christoph Lameter wrote:
> On Thu, 5 Apr 2007, Nick Piggin wrote:
> 
> > On Wed, Apr 04, 2007 at 08:19:22PM -0700, Christoph Lameter 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 of a compound page. page->private == page for the first 
> > > page, so there is no real information in there.
> > 
> > Couldn't you use something like PageActive for the head page instead of
> > taking up a new flag, seeing as we don't put compound pages on the LRU?
> 
> We could set up an alias for now?

Yeah, definitely use an alias.

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC] Free up page->private for compound pages
  2007-04-05  3:57     ` Nick Piggin
@ 2007-04-05  4:04       ` Christoph Lameter
  2007-04-05  4:25         ` Nick Piggin
  0 siblings, 1 reply; 14+ messages in thread
From: Christoph Lameter @ 2007-04-05  4:04 UTC (permalink / raw)
  To: Nick Piggin; +Cc: linux-mm, dgc

On Thu, 5 Apr 2007, Nick Piggin wrote:

> > > Couldn't you use something like PageActive for the head page instead of
> > > taking up a new flag, seeing as we don't put compound pages on the LRU?
> > 
> > We could set up an alias for now?
> 
> Yeah, definitely use an alias.
> 
Which one? PageActive is in use by the slab and the slab can use compound 
pages.

So PG_reclaim? Its pretty esoteric.


--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC] Free up page->private for compound pages
  2007-04-05  4:04       ` Christoph Lameter
@ 2007-04-05  4:25         ` Nick Piggin
  2007-04-05  4:34           ` Christoph Lameter
  0 siblings, 1 reply; 14+ messages in thread
From: Nick Piggin @ 2007-04-05  4:25 UTC (permalink / raw)
  To: Christoph Lameter; +Cc: linux-mm, dgc

On Wed, Apr 04, 2007 at 09:04:38PM -0700, Christoph Lameter wrote:
> On Thu, 5 Apr 2007, Nick Piggin wrote:
> 
> > > > Couldn't you use something like PageActive for the head page instead of
> > > > taking up a new flag, seeing as we don't put compound pages on the LRU?
> > > 
> > > We could set up an alias for now?
> > 
> > Yeah, definitely use an alias.
> > 
> Which one? PageActive is in use by the slab and the slab can use compound 
> pages.
> 
> So PG_reclaim? Its pretty esoteric.

Yeah, good suggestion.

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC] Free up page->private for compound pages
  2007-04-05  4:25         ` Nick Piggin
@ 2007-04-05  4:34           ` Christoph Lameter
  2007-04-05 14:30             ` Hugh Dickins
  0 siblings, 1 reply; 14+ messages in thread
From: Christoph Lameter @ 2007-04-05  4:34 UTC (permalink / raw)
  To: Nick Piggin; +Cc: linux-mm, dgc

On Thu, 5 Apr 2007, Nick Piggin wrote:

> Yeah, good suggestion.

Fixed up patch:

[RFC] Free up page->private for compound pages V2

V1->V2
	- Use alias to PG_reclaim

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.

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-03 23:48:34.000000000 -0700
+++ linux-2.6.21-rc5-mm4/include/linux/mm.h	2007-04-04 18:18:33.000000000 -0700
@@ -297,17 +297,21 @@ static inline int get_page_unless_zero(s
 	return atomic_inc_not_zero(&page->_count);
 }
 
+static inline struct page *compound_head(struct page *page)
+{
+	if (unlikely(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 +348,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-03 23:48:34.000000000 -0700
+++ linux-2.6.21-rc5-mm4/include/linux/page-flags.h	2007-04-04 21:31:54.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,10 @@ static inline void SetPageUptodate(struc
 #define __SetPageCompound(page)	__set_bit(PG_compound, &(page)->flags)
 #define __ClearPageCompound(page) __clear_bit(PG_compound, &(page)->flags)
 
+#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-03-25 15:56:23.000000000 -0700
+++ linux-2.6.21-rc5-mm4/mm/internal.h	2007-04-04 17:56:08.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(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-03 23:48:34.000000000 -0700
+++ linux-2.6.21-rc5-mm4/mm/page_alloc.c	2007-04-04 18:25:21.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,10 +299,12 @@ 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++) {
+	__SetPageCompound(page);
+	set_compound_order(page, order);
+	for (i = 1; i < nr_pages; i++) {
 		struct page *p = page + i;
 
+		__SetPageTail(p);
 		__SetPageCompound(p);
 		set_page_private(p, (unsigned long)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) |
+		if (unlikely(!PageTail(p) | !PageCompound(p) |
 				(page_private(p) != (unsigned long)page)))
 			bad_page(page);
+		__ClearPageTail(p);
 		__ClearPageCompound(p);
 	}
 }
Index: linux-2.6.21-rc5-mm4/mm/slab.c
===================================================================
--- linux-2.6.21-rc5-mm4.orig/mm/slab.c	2007-04-03 23:48:34.000000000 -0700
+++ linux-2.6.21-rc5-mm4/mm/slab.c	2007-04-04 18:05:52.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-04 17:59:30.000000000 -0700
+++ linux-2.6.21-rc5-mm4/mm/slub.c	2007-04-04 21:27:04.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-04 18:01:08.000000000 -0700
+++ linux-2.6.21-rc5-mm4/mm/swap.c	2007-04-04 18:01:40.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-04 18:09:44.000000000 -0700
+++ linux-2.6.21-rc5-mm4/arch/ia64/mm/init.c	2007-04-04 18:15:48.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
 

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC] Free up page->private for compound pages
  2007-04-05  4:34           ` Christoph Lameter
@ 2007-04-05 14:30             ` Hugh Dickins
  2007-04-05 18:17               ` Christoph Lameter
  0 siblings, 1 reply; 14+ messages in thread
From: Hugh Dickins @ 2007-04-05 14:30 UTC (permalink / raw)
  To: Christoph Lameter; +Cc: Nick Piggin, linux-mm, dgc

On Wed, 4 Apr 2007, Christoph Lameter wrote:
> 
> V1->V2
> 	- Use alias to PG_reclaim
> 
> +static inline struct page *compound_head(struct page *page)
> +{
> +	if (unlikely(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);
>  }

No, you don't want anyone looking at the page_count of a page
currently under reclaim, or doing a get_page on it, to go veering
off through its page->private (page->first_page comes from another
of your patches, not in -mm).  Looks like you need to add a test for
PageCompound in compound_head (what a surprise!), unfortunately.

Hugh

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC] Free up page->private for compound pages
  2007-04-05  3:19 [RFC] Free up page->private for compound pages Christoph Lameter
  2007-04-05  3:36 ` Nick Piggin
@ 2007-04-05 15:13 ` Dave Kleikamp
  1 sibling, 0 replies; 14+ messages in thread
From: Dave Kleikamp @ 2007-04-05 15:13 UTC (permalink / raw)
  To: Christoph Lameter; +Cc: linux-mm, dgc, npiggin

On Wed, 2007-04-04 at 20:19 -0700, Christoph Lameter wrote:

> 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-03 23:48:34.000000000 -0700
> +++ linux-2.6.21-rc5-mm4/include/linux/page-flags.h	2007-04-04 18:25:47.000000000 -0700
> @@ -91,6 +91,7 @@
>  #define PG_booked		20	/* Has blocks reserved on-disk */
> 
>  #define PG_readahead		21	/* Reminder to do read-ahead */
> +#define PG_tail			22	/* Tail portion of a compound page */
> 
>  /* PG_owner_priv_1 users should have descriptive aliases */
>  #define PG_checked		PG_owner_priv_1 /* Used by some filesystems */
> @@ -214,6 +215,10 @@ static inline void SetPageUptodate(struc
>  #define __SetPageCompound(page)	__set_bit(PG_compound, &(page)->flags)
>  #define __ClearPageCompound(page) __clear_bit(PG_compound, &(page)->flags)
> 
> +#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)

Wow, I was planning on adding that exact flag for the work I'm doing
with Page Cache Tails:
http://kernel.org/pub/linux/kernel/people/shaggy/OLS-2006/

I'm working on killing the page flag, but I am still using PageTail() to
test for the special-case page.  No worry.  I'll rename it to something
less ambiguous.

As far as the Page Cache Tail work, I'll try to get some patches out for
review soon.

Shaggy
-- 
David Kleikamp
IBM Linux Technology Center

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC] Free up page->private for compound pages
  2007-04-05 14:30             ` Hugh Dickins
@ 2007-04-05 18:17               ` Christoph Lameter
  2007-04-05 18:40                 ` Hugh Dickins
  0 siblings, 1 reply; 14+ messages in thread
From: Christoph Lameter @ 2007-04-05 18:17 UTC (permalink / raw)
  To: Hugh Dickins; +Cc: Nick Piggin, linux-mm, dgc

On Thu, 5 Apr 2007, Hugh Dickins wrote:

> >  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);
> >  }
> 
> No, you don't want anyone looking at the page_count of a page
> currently under reclaim, or doing a get_page on it, to go veering
> off through its page->private (page->first_page comes from another
> of your patches, not in -mm).  Looks like you need to add a test for
> PageCompound in compound_head (what a surprise!), unfortunately.

Hmmm... Thus we should really have separate page flag and not overload it?

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC] Free up page->private for compound pages
  2007-04-05 18:17               ` Christoph Lameter
@ 2007-04-05 18:40                 ` Hugh Dickins
  2007-04-05 18:58                   ` Christoph Lameter
  0 siblings, 1 reply; 14+ messages in thread
From: Hugh Dickins @ 2007-04-05 18:40 UTC (permalink / raw)
  To: Christoph Lameter; +Cc: Nick Piggin, linux-mm, dgc

On Thu, 5 Apr 2007, Christoph Lameter wrote:
> On Thu, 5 Apr 2007, Hugh Dickins wrote:
> 
> > >  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);
> > >  }
> > 
> > No, you don't want anyone looking at the page_count of a page
> > currently under reclaim, or doing a get_page on it, to go veering
> > off through its page->private (page->first_page comes from another
> > of your patches, not in -mm).  Looks like you need to add a test for
> > PageCompound in compound_head (what a surprise!), unfortunately.
> 
> Hmmm... Thus we should really have separate page flag and not overload it?

Of course that would be more efficient, but is it really something
we'd want to be spending a page flag on?  And it's mainly a codesize
thing, the initial unlikely(PageCompound) tests should keep the main
paths as fast as before, shouldn't they?

But I did wonder whether you could do it differently, but not setting
PageCompound on the first struct page of the compound at all - that
one doesn't need the compound page adjustment, of course, which is
your whole point.

Then in those places which really need to know the first is compounded,
test something like PageCompound(page+1) instead.  "something like"
because that particular test won't work nicely for the very last
struct page in a ... node? (sorry, I don't know the right terminology:
the last struct page in a mem_map-like array).

But if that ends up peppering the code with PageCompound(page) ||
PageCompound(page+1) expressions on fast paths, it'd be a whole lot
worse than the PageCompound(page) && PageTail(page) we're envisaging.

Hugh

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC] Free up page->private for compound pages
  2007-04-05 18:40                 ` Hugh Dickins
@ 2007-04-05 18:58                   ` Christoph Lameter
  2007-04-05 19:50                     ` Hugh Dickins
  0 siblings, 1 reply; 14+ messages in thread
From: Christoph Lameter @ 2007-04-05 18:58 UTC (permalink / raw)
  To: Hugh Dickins; +Cc: Nick Piggin, linux-mm, dgc

On Thu, 5 Apr 2007, Hugh Dickins wrote:

> > > off through its page->private (page->first_page comes from another
> > > of your patches, not in -mm).  Looks like you need to add a test for

Yes its in mm. See the slub patches.

> > > PageCompound in compound_head (what a surprise!), unfortunately.
> > 
> > Hmmm... Thus we should really have separate page flag and not overload it?
> 
> Of course that would be more efficient, but is it really something
> we'd want to be spending a page flag on?  And it's mainly a codesize
> thing, the initial unlikely(PageCompound) tests should keep the main
> paths as fast as before, shouldn't they?

I am not so much worried about performance but more about the availability 
of the page->private field of compound pages.

> But I did wonder whether you could do it differently, but not setting
> PageCompound on the first struct page of the compound at all - that
> one doesn't need the compound page adjustment, of course, which is
> your whole point.

Have not thought about it being a performance improvement. Good point 
though.
 
> Then in those places which really need to know the first is compounded,
> test something like PageCompound(page+1) instead.  "something like"
> because that particular test won't work nicely for the very last
> struct page in a ... node? (sorry, I don't know the right terminology:
> the last struct page in a mem_map-like array).

The last page in a MAX_ORDER block may have issues. In particular if its 
the last MAX_ORDER block in a zone. This going to make sparsemem go 
ballistic.

> But if that ends up peppering the code with PageCompound(page) ||
> PageCompound(page+1) expressions on fast paths, it'd be a whole lot
> worse than the PageCompound(page) && PageTail(page) we're envisaging.

Not sure exactly what you are saying.

The initial proposal was to have


1. Headpage		PageCompound

2. Tail page		PageCompound & PageTail

The PageCompound on each page is necessary for various I/O paths that 
check for compound pages and refuse to do certain things (like dirtying 
etc).

The tail marking is advantages because it exactly marks a page that is

1. Compound

2. Not the head of the compound page

Thus is easy and fast to establish the need to lookup the head page of a 
compound page.

I think we cannot overload the page flag after all because of the page 
count issue you pointed out. Guess I should be cleaning up my 
initial patch and repost it?

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC] Free up page->private for compound pages
  2007-04-05 18:58                   ` Christoph Lameter
@ 2007-04-05 19:50                     ` Hugh Dickins
  2007-04-05 20:07                       ` Christoph Lameter
  0 siblings, 1 reply; 14+ messages in thread
From: Hugh Dickins @ 2007-04-05 19:50 UTC (permalink / raw)
  To: Christoph Lameter; +Cc: Nick Piggin, linux-mm, dgc

On Thu, 5 Apr 2007, Christoph Lameter wrote:
> On Thu, 5 Apr 2007, Hugh Dickins wrote:
> 
> > > > off through its page->private (page->first_page comes from another
> > > > of your patches, not in -mm).  Looks like you need to add a test for
> 
> Yes its in mm. See the slub patches.

So it is, sorry, don't know what tree I was mistakenly looking in then.

> > > > PageCompound in compound_head (what a surprise!), unfortunately.
> > > 
> > > Hmmm... Thus we should really have separate page flag and not overload it?
> > 
> > Of course that would be more efficient, but is it really something
> > we'd want to be spending a page flag on?  And it's mainly a codesize
> > thing, the initial unlikely(PageCompound) tests should keep the main
> > paths as fast as before, shouldn't they?
> 
> I am not so much worried about performance but more about the availability 
> of the page->private field of compound pages.

Yes, I realise that.  I meant
	if (unlikely(PageCompound(page)) && PageTail(page))
shouldn't slow down the !PageCompound fast paths more than the existing
	if (unlikely(PageCompound(page)))
or the
	if (unlikely(PageTail(page)))
you had.

> 
> > But I did wonder whether you could do it differently, but not setting
> > PageCompound on the first struct page of the compound at all - that
> > one doesn't need the compound page adjustment, of course, which is
> > your whole point.
> 
> Have not thought about it being a performance improvement. Good point 
> though.
>  
> > Then in those places which really need to know the first is compounded,
> > test something like PageCompound(page+1) instead.  "something like"
> > because that particular test won't work nicely for the very last
> > struct page in a ... node? (sorry, I don't know the right terminology:
> > the last struct page in a mem_map-like array).
> 
> The last page in a MAX_ORDER block may have issues. In particular if its 
> the last MAX_ORDER block in a zone. This going to make sparsemem go 
> ballistic.

Yes, that was my "something like" point: if the compiler allowed
it (I'm sure not!) you'd probably want PageCompound(page|1) instead,
where that "1" is following pointer arithmetic ;)  If the alignment
of a mem_map is nicely guaranteed.

> 
> > But if that ends up peppering the code with PageCompound(page) ||
> > PageCompound(page+1) expressions on fast paths, it'd be a whole lot
> > worse than the PageCompound(page) && PageTail(page) we're envisaging.
> 
> Not sure exactly what you are saying.
> 
> The initial proposal was to have
> 
> 
> 1. Headpage		PageCompound
> 
> 2. Tail page		PageCompound & PageTail
> 
> The PageCompound on each page is necessary for various I/O paths that 
> check for compound pages and refuse to do certain things (like dirtying 
> etc).

Yes, those set_page_dirty_lock places.  I think those used to be
important, because we put the compound page dtor into page[1].mapping,
and set_page_dirty would go crazy when it mistook that for a struct
address_space*.  When we moved the dtor pointer into page[1].lru.next,
I left those tests alone as a minor optimization: it just didn't need
to bother with the set_page_dirty.  If the first page of the compound
was not marked PageCompound, they could be updated or not, they're just
not important; but never mind, you probably don't want to go that way.

> 
> The tail marking is advantages because it exactly marks a page that is
> 
> 1. Compound
> 
> 2. Not the head of the compound page
> 
> Thus is easy and fast to establish the need to lookup the head page of a 
> compound page.
> 
> I think we cannot overload the page flag after all because of the page 
> count issue you pointed out. Guess I should be cleaning up my 
> initial patch and repost it?

I still think PageTail is not worth its own distinct page flag:
I can understand you drawing back from my page+1 suggestion,
but I don't understand why you're so reluctant to say
	if (unlikely(PageCompound(page)) && PageTail(page))

Hugh

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC] Free up page->private for compound pages
  2007-04-05 19:50                     ` Hugh Dickins
@ 2007-04-05 20:07                       ` Christoph Lameter
  0 siblings, 0 replies; 14+ messages in thread
From: Christoph Lameter @ 2007-04-05 20:07 UTC (permalink / raw)
  To: Hugh Dickins; +Cc: Nick Piggin, linux-mm, dgc

On Thu, 5 Apr 2007, Hugh Dickins wrote:

> > I am not so much worried about performance but more about the availability 
> > of the page->private field of compound pages.
> 
> Yes, I realise that.  I meant
> 	if (unlikely(PageCompound(page)) && PageTail(page))
> shouldn't slow down the !PageCompound fast paths more than the existing
> 	if (unlikely(PageCompound(page)))
> or the
> 	if (unlikely(PageTail(page)))
> you had.

Right.

> > I think we cannot overload the page flag after all because of the page 
> > count issue you pointed out. Guess I should be cleaning up my 
> > initial patch and repost it?
> 
> I still think PageTail is not worth its own distinct page flag:

Well I think we just killed 2 flags for software suspend. Did I not earn 
at least one by being involved in that project? ;-)

> I can understand you drawing back from my page+1 suggestion,
> but I don't understand why you're so reluctant to say
> 	if (unlikely(PageCompound(page)) && PageTail(page))

Thats fine with me. Ahh.. This would solve the alias issue.... (Lights
going on). Okay we can overload after all. Need to add some comments 
though.


--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2007-04-05 20:07 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-04-05  3:19 [RFC] Free up page->private for compound pages Christoph Lameter
2007-04-05  3:36 ` Nick Piggin
2007-04-05  3:38   ` Christoph Lameter
2007-04-05  3:57     ` Nick Piggin
2007-04-05  4:04       ` Christoph Lameter
2007-04-05  4:25         ` Nick Piggin
2007-04-05  4:34           ` Christoph Lameter
2007-04-05 14:30             ` Hugh Dickins
2007-04-05 18:17               ` Christoph Lameter
2007-04-05 18:40                 ` Hugh Dickins
2007-04-05 18:58                   ` Christoph Lameter
2007-04-05 19:50                     ` Hugh Dickins
2007-04-05 20:07                       ` Christoph Lameter
2007-04-05 15:13 ` Dave Kleikamp

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox