[PATCH 0/3] explicitly document overloaded page flags

[PATCH 0/3] explicitly document overloaded page flags

[Andy Whitcroft]

With the recent page flag reorganisation we have a single enum which
defines the valid page flags and their values, nice and clear.  However
there are a number of bits which are overloaded by different subsystems.
Firstly there is PG_owner_priv_1 which is used by filesystems and by XEN.
Secondly both SLOB and SLUB use a couple of extra page bits to manage
internal state for pages they own; both overlay other bits.  All of these
"aliases" are scattered about the source making it very hard for a reader
to know if the bits are safe to rely on in all contexts; confusion here
is bad.

As we now have a single place where the bits are clearly assigned it makes
sense to clarify the reuse of bits by making the aliases explicit and
visible with the original bit assignments.  This patch creates explicit
aliases within the enum itself for the overloaded bits and uses those
aliases throughout.

Andrew please consider for -mm.

-apw

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

[PATCH 1/3] page-flags: record page flag overlays explicitly

[Andy Whitcroft]

Some page flags are used for more than one purpose, for example
PG_owner_priv_1.  Currently there are individual accessors for each user,
each built using the common flag name far away from the bit definitions.
This makes it hard to see all possible uses of these bits.

Now that we have a single enum to generate the bit orders it makes sense
to express overlays in the same place.  So create per use aliases for
this bit in the main page-flags enum and use those in the accessors.

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
 include/linux/page-flags.h |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index 590cff3..2cc1fb1 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -96,7 +96,13 @@ enum pageflags {
 #ifdef CONFIG_IA64_UNCACHED_ALLOCATOR
 	PG_uncached,		/* Page has been mapped as uncached */
 #endif
-	__NR_PAGEFLAGS
+	__NR_PAGEFLAGS,
+
+	/* Filesystems */
+	PG_checked = PG_owner_priv_1,
+
+	/* XEN */
+	PG_pinned = PG_owner_priv_1,
 };
 
 #ifndef __GENERATING_BOUNDS_H
@@ -155,8 +161,8 @@ PAGEFLAG(Dirty, dirty) TESTSCFLAG(Dirty, dirty) __CLEARPAGEFLAG(Dirty, dirty)
 PAGEFLAG(LRU, lru) __CLEARPAGEFLAG(LRU, lru)
 PAGEFLAG(Active, active) __CLEARPAGEFLAG(Active, active)
 __PAGEFLAG(Slab, slab)
-PAGEFLAG(Checked, owner_priv_1)		/* Used by some filesystems */
-PAGEFLAG(Pinned, owner_priv_1) TESTSCFLAG(Pinned, owner_priv_1) /* Xen */
+PAGEFLAG(Checked, checked)		/* Used by some filesystems */
+PAGEFLAG(Pinned, pinned) TESTSCFLAG(Pinned, pinned) /* Xen */
 PAGEFLAG(Reserved, reserved) __CLEARPAGEFLAG(Reserved, reserved)
 PAGEFLAG(Private, private) __CLEARPAGEFLAG(Private, private)
 	__SETPAGEFLAG(Private, private)

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

Re: [PATCH 1/3] page-flags: record page flag overlays explicitly

[Christoph Lameter]

On Thu, 15 May 2008, Andy Whitcroft wrote:

> Now that we have a single enum to generate the bit orders it makes sense
> to express overlays in the same place.  So create per use aliases for
> this bit in the main page-flags enum and use those in the accessors.

Well I thought it would be better to have the overlays defined when the 
PAGEFLAGS_xx macro is used. If that is done then every PG_xxx has a unique 
id. The aliasing is then only through PageXXXX() using a PG_yyy

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

[PATCH 2/3] slub: record page flag overlays explicitly

[Andy Whitcroft]

SLUB reuses two page bits for internal purposes, it overlays PG_active
and PG_error.  This is hidden away in slub.c.  Document these overlays
explicitly in the main page-flags enum along with all the others.

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
 include/linux/page-flags.h |    4 ++++
 mm/slub.c                  |    4 ++--
 2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index 2cc1fb1..2e88df6 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -103,6 +103,10 @@ enum pageflags {
 
 	/* XEN */
 	PG_pinned = PG_owner_priv_1,
+
+	/* SLUB */
+	PG_slub_frozen = PG_active,
+	PG_slub_debug = PG_error,
 };
 
 #ifndef __GENERATING_BOUNDS_H
diff --git a/mm/slub.c b/mm/slub.c
index a505a82..fd7c61a 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -102,10 +102,10 @@
  * 			the fast path and disables lockless freelists.
  */
 
-#define FROZEN (1 << PG_active)
+#define FROZEN (1 << PG_slub_frozen)
 
 #ifdef CONFIG_SLUB_DEBUG
-#define SLABDEBUG (1 << PG_error)
+#define SLABDEBUG (1 << PG_slub_debug)
 #else
 #define SLABDEBUG 0
 #endif

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

Re: [PATCH 2/3] slub: record page flag overlays explicitly

[Christoph Lameter]

On Thu, 15 May 2008, Andy Whitcroft wrote:
 
> SLUB reuses two page bits for internal purposes, it overlays PG_active
> and PG_error.  This is hidden away in slub.c.  Document these overlays
> explicitly in the main page-flags enum along with all the others.

Hmmm.. Add the definitions also to page-flags.h?

SLABFLAG(Frozen, PG_active)
SLABFLAG(Debug, PG_error)
?

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

Re: [PATCH 2/3] slub: record page flag overlays explicitly

[KOSAKI Motohiro]

> SLUB reuses two page bits for internal purposes, it overlays PG_active
> and PG_error.  This is hidden away in slub.c.  Document these overlays
> explicitly in the main page-flags enum along with all the others.
> 
> Signed-off-by: Andy Whitcroft <apw@shadowen.org>

Agreed.
I like your approach :)



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

[PATCH 3/3] slob: record page flag overlays explicitly

[Andy Whitcroft]

SLOB reuses two page bits for internal purposes, it overlays PG_active
and PG_private.  This is hidden away in slob.c.  Document these overlays
explicitly in the main page-flags enum along with all the others.

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
 include/linux/page-flags.h |    4 ++++
 mm/slob.c                  |   12 ++++++------
 2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index 2e88df6..71aec98 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -104,6 +104,10 @@ enum pageflags {
 	/* XEN */
 	PG_pinned = PG_owner_priv_1,
 
+	/* SLOB */
+	PG_slob_page = PG_active,
+	PG_slob_free = PG_private,
+
 	/* SLUB */
 	PG_slub_frozen = PG_active,
 	PG_slub_debug = PG_error,
diff --git a/mm/slob.c b/mm/slob.c
index 6038cba..9bc3147 100644
--- a/mm/slob.c
+++ b/mm/slob.c
@@ -130,17 +130,17 @@ static LIST_HEAD(free_slob_large);
  */
 static inline int slob_page(struct slob_page *sp)
 {
-	return test_bit(PG_active, &sp->flags);
+	return test_bit(PG_slob_page, &sp->flags);
 }
 
 static inline void set_slob_page(struct slob_page *sp)
 {
-	__set_bit(PG_active, &sp->flags);
+	__set_bit(PG_slob_page, &sp->flags);
 }
 
 static inline void clear_slob_page(struct slob_page *sp)
 {
-	__clear_bit(PG_active, &sp->flags);
+	__clear_bit(PG_slob_free, &sp->flags);
 }
 
 /*
@@ -148,19 +148,19 @@ static inline void clear_slob_page(struct slob_page *sp)
  */
 static inline int slob_page_free(struct slob_page *sp)
 {
-	return test_bit(PG_private, &sp->flags);
+	return test_bit(PG_slob_free, &sp->flags);
 }
 
 static void set_slob_page_free(struct slob_page *sp, struct list_head *list)
 {
 	list_add(&sp->list, list);
-	__set_bit(PG_private, &sp->flags);
+	__set_bit(PG_slob_free, &sp->flags);
 }
 
 static inline void clear_slob_page_free(struct slob_page *sp)
 {
 	list_del(&sp->list);
-	__clear_bit(PG_private, &sp->flags);
+	__clear_bit(PG_slob_free, &sp->flags);
 }
 
 #define SLOB_UNIT sizeof(slob_t)

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

Re: [PATCH 3/3] slob: record page flag overlays explicitly

[Andy Whitcroft]

On Thu, May 15, 2008 at 06:20:10PM +0100, Andy Whitcroft wrote:

>  static inline void clear_slob_page(struct slob_page *sp)
>  {
> -	__clear_bit(PG_active, &sp->flags);
> +	__clear_bit(PG_slob_free, &sp->flags);
>  }

Bah, this hunk is wrong.  Seems this is not the latest version.  Will
replace this patch momentarily.

-apw

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

[PATCH] slob: record page flag overlays explicitly v2

[Andy Whitcroft]

SLOB reuses two page bits for internal purposes, it overlays PG_active
and PG_private.  This is hidden away in slob.c.  Document these overlays
explicitly in the main page-flags enum along with all the others.

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
 include/linux/page-flags.h |    4 ++++
 mm/slob.c                  |   12 ++++++------
 2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index 2e88df6..71aec98 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -104,6 +104,10 @@ enum pageflags {
 	/* XEN */
 	PG_pinned = PG_owner_priv_1,
 
+	/* SLOB */
+	PG_slob_page = PG_active,
+	PG_slob_free = PG_private,
+
 	/* SLUB */
 	PG_slub_frozen = PG_active,
 	PG_slub_debug = PG_error,
diff --git a/mm/slob.c b/mm/slob.c
index 6038cba..0aaf54f 100644
--- a/mm/slob.c
+++ b/mm/slob.c
@@ -130,17 +130,17 @@ static LIST_HEAD(free_slob_large);
  */
 static inline int slob_page(struct slob_page *sp)
 {
-	return test_bit(PG_active, &sp->flags);
+	return test_bit(PG_slob_page, &sp->flags);
 }
 
 static inline void set_slob_page(struct slob_page *sp)
 {
-	__set_bit(PG_active, &sp->flags);
+	__set_bit(PG_slob_page, &sp->flags);
 }
 
 static inline void clear_slob_page(struct slob_page *sp)
 {
-	__clear_bit(PG_active, &sp->flags);
+	__clear_bit(PG_slob_page, &sp->flags);
 }
 
 /*
@@ -148,19 +148,19 @@ static inline void clear_slob_page(struct slob_page *sp)
  */
 static inline int slob_page_free(struct slob_page *sp)
 {
-	return test_bit(PG_private, &sp->flags);
+	return test_bit(PG_slob_free, &sp->flags);
 }
 
 static void set_slob_page_free(struct slob_page *sp, struct list_head *list)
 {
 	list_add(&sp->list, list);
-	__set_bit(PG_private, &sp->flags);
+	__set_bit(PG_slob_free, &sp->flags);
 }
 
 static inline void clear_slob_page_free(struct slob_page *sp)
 {
 	list_del(&sp->list);
-	__clear_bit(PG_private, &sp->flags);
+	__clear_bit(PG_slob_free, &sp->flags);
 }
 
 #define SLOB_UNIT sizeof(slob_t)

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

Re: [PATCH 0/3] explicitly document overloaded page flags

[Christoph Lameter]

On Thu, 15 May 2008, Andy Whitcroft wrote:

> With the recent page flag reorganisation we have a single enum which
> defines the valid page flags and their values, nice and clear.  However
> there are a number of bits which are overloaded by different subsystems.
> Firstly there is PG_owner_priv_1 which is used by filesystems and by XEN.
> Secondly both SLOB and SLUB use a couple of extra page bits to manage
> internal state for pages they own; both overlay other bits.  All of these
> "aliases" are scattered about the source making it very hard for a reader
> to know if the bits are safe to rely on in all contexts; confusion here
> is bad.
> 
> As we now have a single place where the bits are clearly assigned it makes
> sense to clarify the reuse of bits by making the aliases explicit and
> visible with the original bit assignments.  This patch creates explicit
> aliases within the enum itself for the overloaded bits and uses those
> aliases throughout.

Ahh. Great! I considered doing that work too but never got around to 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>

[PATCH 0/3] explicitly document overloaded page flags V2

[Andy Whitcroft]

With the recent page flag reorganisation we have a single enum which
defines the valid page flags and their values, nice and clear.  However
there are a number of bits which are overloaded by different subsystems.
Firstly there is PG_owner_priv_1 which is used by filesystems and by XEN.
Secondly both SLOB and SLUB use a couple of extra page bits to manage
internal state for pages they own; both overlay other bits.  All of these
"aliases" are scattered about the source making it very hard for a reader
to know if the bits are safe to rely on in all contexts; confusion here
is bad.

As we now have a single place where the bits are clearly assigned it makes
sense to clarify the reuse of bits by making the aliases explicit and
visible with the original bit assignments.  This patch creates explicit
aliases within the enum itself for the overloaded bits, creates standard
bit accessors PageFoo etc. and uses those throughout.

This version pulls the bit manipulation out to standard named page bit
accessors as suggested by Christoph, it retains the explicit mapping to
the overlayed bits.  A fusion of both ideas.  This has been SLUB and
SLOB have been compile tested on x86_64 only, and SLUB boot tested.
If people feel this is worth doing then I can run a fuller set of testing.

-apw

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

[PATCH 2/3] slub: record page flag overlays explicitly

[Andy Whitcroft]

SLUB reuses two page bits for internal purposes, it overlays PG_active
and PG_error.  This is hidden away in slub.c.  Document these overlays
explicitly in the main page-flags enum along with all the others.

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
 include/linux/page-flags.h |    7 +++++
 mm/slub.c                  |   65 +++++++++++--------------------------------
 2 files changed, 24 insertions(+), 48 deletions(-)
diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index 2cc1fb1..dfd0a26 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -103,6 +103,10 @@ enum pageflags {
 
 	/* XEN */
 	PG_pinned = PG_owner_priv_1,
+
+	/* SLUB */
+	PG_slub_frozen = PG_active,
+	PG_slub_debug = PG_error,
 };
 
 #ifndef __GENERATING_BOUNDS_H
@@ -167,6 +171,9 @@ PAGEFLAG(Reserved, reserved) __CLEARPAGEFLAG(Reserved, reserved)
 PAGEFLAG(Private, private) __CLEARPAGEFLAG(Private, private)
 	__SETPAGEFLAG(Private, private)
 
+__PAGEFLAG(SlubFrozen, slub_frozen)
+__PAGEFLAG(SlubDebug, slub_debug)
+
 /*
  * Only test-and-set exist for PG_writeback.  The unconditional operators are
  * risky: they bypass page accounting.
diff --git a/mm/slub.c b/mm/slub.c
index a505a82..c7f5653 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -102,44 +102,12 @@
  * 			the fast path and disables lockless freelists.
  */
 
-#define FROZEN (1 << PG_active)
-
 #ifdef CONFIG_SLUB_DEBUG
-#define SLABDEBUG (1 << PG_error)
+#define SLABDEBUG 1
 #else
 #define SLABDEBUG 0
 #endif
 
-static inline int SlabFrozen(struct page *page)
-{
-	return page->flags & FROZEN;
-}
-
-static inline void SetSlabFrozen(struct page *page)
-{
-	page->flags |= FROZEN;
-}
-
-static inline void ClearSlabFrozen(struct page *page)
-{
-	page->flags &= ~FROZEN;
-}
-
-static inline int SlabDebug(struct page *page)
-{
-	return page->flags & SLABDEBUG;
-}
-
-static inline void SetSlabDebug(struct page *page)
-{
-	page->flags |= SLABDEBUG;
-}
-
-static inline void ClearSlabDebug(struct page *page)
-{
-	page->flags &= ~SLABDEBUG;
-}
-
 /*
  * Issues still to be resolved:
  *
@@ -972,7 +940,7 @@ static int free_debug_processing(struct kmem_cache *s, struct page *page,
 	}
 
 	/* Special debug activities for freeing objects */
-	if (!SlabFrozen(page) && !page->freelist)
+	if (!PageSlubFrozen(page) && !page->freelist)
 		remove_full(s, page);
 	if (s->flags & SLAB_STORE_USER)
 		set_track(s, object, TRACK_FREE, addr);
@@ -1158,7 +1126,7 @@ static struct page *new_slab(struct kmem_cache *s, gfp_t flags, int node)
 	page->flags |= 1 << PG_slab;
 	if (s->flags & (SLAB_DEBUG_FREE | SLAB_RED_ZONE | SLAB_POISON |
 			SLAB_STORE_USER | SLAB_TRACE))
-		SetSlabDebug(page);
+		__SetPageSlubDebug(page);
 
 	start = page_address(page);
 
@@ -1185,14 +1153,14 @@ static void __free_slab(struct kmem_cache *s, struct page *page)
 	int order = compound_order(page);
 	int pages = 1 << order;
 
-	if (unlikely(SlabDebug(page))) {
+	if (unlikely(SLABDEBUG && PageSlubDebug(page))) {
 		void *p;
 
 		slab_pad_check(s, page);
 		for_each_object(p, s, page_address(page),
 						page->objects)
 			check_object(s, page, p, 0);
-		ClearSlabDebug(page);
+		__ClearPageSlubDebug(page);
 	}
 
 	mod_zone_page_state(page_zone(page),
@@ -1289,7 +1257,7 @@ static inline int lock_and_freeze_slab(struct kmem_cache_node *n,
 	if (slab_trylock(page)) {
 		list_del(&page->lru);
 		n->nr_partial--;
-		SetSlabFrozen(page);
+		__SetPageSlubFrozen(page);
 		return 1;
 	}
 	return 0;
@@ -1399,7 +1367,7 @@ static void unfreeze_slab(struct kmem_cache *s, struct page *page, int tail)
 	struct kmem_cache_node *n = get_node(s, page_to_nid(page));
 	struct kmem_cache_cpu *c = get_cpu_slab(s, smp_processor_id());
 
-	ClearSlabFrozen(page);
+	__ClearPageSlubFrozen(page);
 	if (page->inuse) {
 
 		if (page->freelist) {
@@ -1407,7 +1375,8 @@ static void unfreeze_slab(struct kmem_cache *s, struct page *page, int tail)
 			stat(c, tail ? DEACTIVATE_TO_TAIL : DEACTIVATE_TO_HEAD);
 		} else {
 			stat(c, DEACTIVATE_FULL);
-			if (SlabDebug(page) && (s->flags & SLAB_STORE_USER))
+			if (SLABDEBUG && PageSlubDebug(page) &&
+						(s->flags & SLAB_STORE_USER))
 				add_full(n, page);
 		}
 		slab_unlock(page);
@@ -1560,7 +1529,7 @@ load_freelist:
 	object = c->page->freelist;
 	if (unlikely(!object))
 		goto another_slab;
-	if (unlikely(SlabDebug(c->page)))
+	if (unlikely(SLABDEBUG && PageSlubDebug(c->page)))
 		goto debug;
 
 	c->freelist = object[c->offset];
@@ -1597,7 +1566,7 @@ new_slab:
 		if (c->page)
 			flush_slab(s, c);
 		slab_lock(new);
-		SetSlabFrozen(new);
+		__SetPageSlubFrozen(new);
 		c->page = new;
 		goto load_freelist;
 	}
@@ -1681,7 +1650,7 @@ static void __slab_free(struct kmem_cache *s, struct page *page,
 	stat(c, FREE_SLOWPATH);
 	slab_lock(page);
 
-	if (unlikely(SlabDebug(page)))
+	if (unlikely(SLABDEBUG && PageSlubDebug(page)))
 		goto debug;
 
 checks_ok:
@@ -1689,7 +1658,7 @@ checks_ok:
 	page->freelist = object;
 	page->inuse--;
 
-	if (unlikely(SlabFrozen(page))) {
+	if (unlikely(PageSlubFrozen(page))) {
 		stat(c, FREE_FROZEN);
 		goto out_unlock;
 	}
@@ -3314,12 +3283,12 @@ static void validate_slab_slab(struct kmem_cache *s, struct page *page,
 			s->name, page);
 
 	if (s->flags & DEBUG_DEFAULT_FLAGS) {
-		if (!SlabDebug(page))
-			printk(KERN_ERR "SLUB %s: SlabDebug not set "
+		if (!PageSlubDebug(page))
+			printk(KERN_ERR "SLUB %s: SlubDebug not set "
 				"on slab 0x%p\n", s->name, page);
 	} else {
-		if (SlabDebug(page))
-			printk(KERN_ERR "SLUB %s: SlabDebug set on "
+		if (PageSlubDebug(page))
+			printk(KERN_ERR "SLUB %s: SlubDebug set on "
 				"slab 0x%p\n", s->name, page);
 	}
 }

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

Re: [PATCH 2/3] slub: record page flag overlays explicitly

[KOSAKI Motohiro]

Hi

This patch works well on my box.
but I have one question.

>  	if (s->flags & DEBUG_DEFAULT_FLAGS) {
> -		if (!SlabDebug(page))
> -			printk(KERN_ERR "SLUB %s: SlabDebug not set "
> +		if (!PageSlubDebug(page))
> +			printk(KERN_ERR "SLUB %s: SlubDebug not set "
>  				"on slab 0x%p\n", s->name, page);
>  	} else {
> -		if (SlabDebug(page))
> -			printk(KERN_ERR "SLUB %s: SlabDebug set on "
> +		if (PageSlubDebug(page))
> +			printk(KERN_ERR "SLUB %s: SlubDebug set on "
>  				"slab 0x%p\n", s->name, page);
>  	}
>  }

Why if(SLABDEBUG) check is unnecessary?



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

Re: [PATCH 2/3] slub: record page flag overlays explicitly

[apw]

On Mon, May 26, 2008 at 01:40:44PM +0900, KOSAKI Motohiro wrote:
> Hi
> 
> This patch works well on my box.
> but I have one question.
> 
> >  	if (s->flags & DEBUG_DEFAULT_FLAGS) {
> > -		if (!SlabDebug(page))
> > -			printk(KERN_ERR "SLUB %s: SlabDebug not set "
> > +		if (!PageSlubDebug(page))
> > +			printk(KERN_ERR "SLUB %s: SlubDebug not set "
> >  				"on slab 0x%p\n", s->name, page);
> >  	} else {
> > -		if (SlabDebug(page))
> > -			printk(KERN_ERR "SLUB %s: SlabDebug set on "
> > +		if (PageSlubDebug(page))
> > +			printk(KERN_ERR "SLUB %s: SlubDebug set on "
> >  				"slab 0x%p\n", s->name, page);
> >  	}
> >  }
> 
> Why if(SLABDEBUG) check is unnecessary?

They were unconditional before as well.  SlabDebug would always return
0 before the patch.  The point being, to my reading, that if you asked
for debug on the slab and debug was not compiled in you would still get
told that it was not set; which it cannot without the support.

-apw

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

Re: [PATCH 2/3] slub: record page flag overlays explicitly

[KOSAKI Motohiro]

>> > -           if (SlabDebug(page))
>> > -                   printk(KERN_ERR "SLUB %s: SlabDebug set on "
>> > +           if (PageSlubDebug(page))
>> > +                   printk(KERN_ERR "SLUB %s: SlubDebug set on "
>> >                             "slab 0x%p\n", s->name, page);
>> >     }
>> >  }
>>
>> Why if(SLABDEBUG) check is unnecessary?
>
> They were unconditional before as well.  SlabDebug would always return
> 0 before the patch.  The point being, to my reading, that if you asked
> for debug on the slab and debug was not compiled in you would still get
> told that it was not set; which it cannot without the support.

Thank you explain!

  Tested-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>

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