Skip to contentSkip to conversation

mm: SLUB fix reclaim_state

PATCH In review [email protected] 5 participants 13 messages May 5, 2009 – May 7, 2009 1 message not in the archive

Subsystems (2)
lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

#1 lore raw

PATCH 1/3 mm: SLUB fix reclaim_state

SLUB does not correctly account reclaim_state.reclaimed_slab, so it will break memory reclaim. Account it like SLAB does.

---
 mm/slub.c |    3 +++
 1 file changed, 3 insertions(+)
mm/slub.c +3 -0
Index: linux-2.6/mm/slub.c
===================================================================
@@ -9,6 +9,7 @@
  */
 
 #include <linux/mm.h>
+#include <linux/swap.h> /* struct reclaim_state */
 #include <linux/module.h>
 #include <linux/bit_spinlock.h>
 #include <linux/interrupt.h>
@@ -1170,6 +1171,8 @@ static void __free_slab(struct kmem_cach
 
 	__ClearPageSlab(page);
 	reset_page_mapcount(page);
+	if (current->reclaim_state)
+		current->reclaim_state->reclaimed_slab += pages;
 	__free_pages(page, order);
 }
 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#3 lore raw
In reply to [email protected]
Quoted 10 lines · On Tue, 2009-05-05 at 19:13 +1000, [email protected] wrote:

plain text document attachment (mm-slub-fix-reclaim_state.patch) SLUB does not correctly account reclaim_state.reclaimed_slab, so it will break memory reclaim. Account it like SLAB does.

I have applied the patch series. I see you have cc'd stable so I assume you want this in 2.6.30, right? This seems like a rather serious bug but I wonder why we've gotten away with it for so long? Is there a test program or a known workload that breaks without this?

Pekka

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#4 lore raw
Quoted 4 lines · Pekka Enberg

I have applied the patch series. I see you have cc'd stable so I assume you want this in 2.6.30, right? This seems like a rather serious bug but I wonder why we've gotten away with it for so long? Is there a test program or a known workload that breaks without this?

You need to have a load that results in extensive slab reclaim so that the slab pages in the reclaim path become a factor.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#5 lore raw
In reply to Pekka Enberg
Quoted 12 lines · Pekka Enberg
Quoted 10 lines · On Tue, 2009-05-05 at 19:13 +1000, [email protected] wrote:

plain text document attachment (mm-slub-fix-reclaim_state.patch) SLUB does not correctly account reclaim_state.reclaimed_slab, so it will break memory reclaim. Account it like SLAB does.

Quoted 2 lines

I have applied the patch series. I see you have cc'd stable so I assume you want this in 2.6.30, right? This seems like a rather serious bug but

Thanks. I think it makes sense to into 2.6.30. Also probably all active .stable kernels.

Quoted 2 lines

I wonder why we've gotten away with it for so long? Is there a test program or a known workload that breaks without this?

Well... it isn't doing what reclaim code wants, and it is differing behaviour between SLAB and SL?B, so I think it is fairly safe to merge these now.

It doesn't look like too much *significant* changes to heuristics, but things will get skewed here and there.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#6 lore raw

Hi Nick,

Quoted 10 lines · Nick Piggin
Quoted 2 lines

I have applied the patch series. I see you have cc'd stable so I assume you want this in 2.6.30, right? This seems like a rather serious bug but

Thanks. I think it makes sense to into 2.6.30. Also probably all active .stable kernels.

Quoted 2 lines

I wonder why we've gotten away with it for so long? Is there a test program or a known workload that breaks without this?

Quoted 6 lines

Well... it isn't doing what reclaim code wants, and it is differing behaviour between SLAB and SL?B, so I think it is fairly safe to merge these now.

It doesn't look like too much *significant* changes to heuristics, but things will get skewed here and there.

Yeah, that's my thinking too. Oh, well, I'll forward it to Linus' way and let the stable guys decide whether they want to take it or not.

Pekka

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#7 lore raw
In reply to Pekka Enberg
Quoted 12 lines · Pekka Enberg
Quoted 10 lines · On Tue, 2009-05-05 at 19:13 +1000, [email protected] wrote:

plain text document attachment (mm-slub-fix-reclaim_state.patch) SLUB does not correctly account reclaim_state.reclaimed_slab, so it will break memory reclaim. Account it like SLAB does.

Quoted 4 lines

I have applied the patch series. I see you have cc'd stable so I assume you want this in 2.6.30, right? This seems like a rather serious bug but I wonder why we've gotten away with it for so long? Is there a test program or a known workload that breaks without this?

We might overreclaim a bit on the LRU lists, but not that much, as we scan them in SWAP_CLUSTER_MAX (32 pages) steps only most of the time and use the same value as a total goal.

So it will rescan the LRU lists if they didn't yield 32 pages, even if slab cache pruning was more successful.

Only hibernation has a larger scan goal and it will then reclaim the extra amount of LRU pages that it failed to account for slab pruning, which might hurt with 15mb worth of reclaimable slab caches.

Hannes

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#8 lore raw
In reply to Pekka Enberg
Quoted 12 lines · Pekka Enberg
Quoted 10 lines · On Tue, 2009-05-05 at 19:13 +1000, [email protected] wrote:

plain text document attachment (mm-slub-fix-reclaim_state.patch) SLUB does not correctly account reclaim_state.reclaimed_slab, so it will break memory reclaim. Account it like SLAB does.

Quoted 4 lines

I have applied the patch series. I see you have cc'd stable so I assume you want this in 2.6.30, right? This seems like a rather serious bug but I wonder why we've gotten away with it for so long? Is there a test program or a known workload that breaks without this?

Appears to me to be less a correctness than a balancing issue. reclaim state is a back channel into the shrink code that says 'yes, this is working'. Without it, things should still work, but possibly not as smoothly.

Signature · 7 lines
Mathematics is the supreme nostalgia of our time.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#9 lore raw

PATCH 2/3 mm: SLOB fix reclaim_state

In reply to a message not in the archive

SLOB does not correctly account reclaim_state.reclaimed_slab, so it will break memory reclaim. Account it like SLAB does.

---
 mm/slob.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
mm/slob.c +4 -1
Index: linux-2.6/mm/slob.c
===================================================================
@@ -60,6 +60,7 @@
 #include <linux/kernel.h>
 #include <linux/slab.h>
 #include <linux/mm.h>
+#include <linux/swap.h> /* struct reclaim_state */
 #include <linux/cache.h>
 #include <linux/init.h>
 #include <linux/module.h>
@@ -255,6 +256,8 @@ static void *slob_new_pages(gfp_t gfp, i
 
 static void slob_free_pages(void *b, int order)
 {
+	if (current->reclaim_state)
+		current->reclaim_state->reclaimed_slab += 1 << order;
 	free_pages((unsigned long)b, order);
 }
 
@@ -407,7 +410,7 @@ static void slob_free(void *block, int s
 		spin_unlock_irqrestore(&slob_lock, flags);
 		clear_slob_page(sp);
 		free_slob_page(sp);
-		free_page((unsigned long)b);
+		slob_free_pages(b, 0);
 		return;
 	}
 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Acked-by on v1 2/3
#11 lore raw

Re: [patch 2/3] mm: SLOB fix reclaim_state

In reply to [email protected]
Quoted 2 lines · On Tue, May 05, 2009 at 07:13:45PM +1000, [email protected] wrote:

SLOB does not correctly account reclaim_state.reclaimed_slab, so it will break memory reclaim. Account it like SLAB does.

Signature · 7 lines
Mathematics is the supreme nostalgia of our time.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#12 lore raw

PATCH 3/3 mm: SLQB fix reclaim_state

In reply to a message not in the archive

SLQB does not correctly account reclaim_state.reclaimed_slab, so it will break memory reclaim. Account it like SLAB does.

---
 mm/slqb.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
mm/slqb.c +6 -2
Index: linux-2.6/mm/slqb.c
===================================================================
@@ -8,6 +8,7 @@
  */
 
 #include <linux/mm.h>
+#include <linux/swap.h> /* struct reclaim_state */
 #include <linux/module.h>
 #include <linux/interrupt.h>
 #include <linux/slab.h>
@@ -178,7 +179,8 @@ static inline struct slqb_page *virt_to_
 	return (struct slqb_page *)p;
 }
 
-static inline void __free_slqb_pages(struct slqb_page *page, unsigned int order)
+static inline void __free_slqb_pages(struct slqb_page *page, unsigned int order,
+					int pages)
 {
 	struct page *p = &page->page;
 
@@ -187,6 +189,8 @@ static inline void __free_slqb_pages(str
 	VM_BUG_ON(!(p->flags & PG_SLQB_BIT));
 	p->flags &= ~PG_SLQB_BIT;
 
+	if (current->reclaim_state)
+		current->reclaim_state->reclaimed_slab += pages;
 	__free_pages(p, order);
 }
 
@@ -1043,7 +1047,7 @@ static void __free_slab(struct kmem_cach
 		NR_SLAB_RECLAIMABLE : NR_SLAB_UNRECLAIMABLE,
 		-pages);
 
-	__free_slqb_pages(page, s->order);
+	__free_slqb_pages(page, s->order, pages);
 }
 
 static void rcu_free_slab(struct rcu_head *h)

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>