RE: [PATCH] slub Discard slab page only when node partials > minimum setting

RE: [PATCH] slub Discard slab page only when node partials > minimum setting

[Alex,Shi]

On Wed, 2011-09-07 at 23:05 +0800, Christoph Lameter wrote:
> On Wed, 7 Sep 2011, Shi, Alex wrote:
> 
> > Oh, seems the deactivate_slab() corrected at linus' tree already, but
> > the unfreeze_partials() just copied from the old version
> > deactivate_slab().
> 
> Ok then the patch is ok.
> 
> Do you also have performance measurements? I am a bit hesitant to merge
> the per cpu partials patchset if there are regressions in the low
> concurrency tests as seem to be indicated by intels latest tests.
> 

My LKP testing system most focus on server platforms. I tested your per
cpu partial set on hackbench and netperf loopback benchmark. hackbench
improve much.

Maybe some IO testing is low concurrency for SLUB, maybe a few jobs
kbuild? or low swap press testing.  I may try them for your patchset in
the near days. 

BTW, some testing results for your PCP SLUB:

for hackbench process testing: 
on WSM-EP, inc ~60%, NHM-EP inc ~25%
on NHM-EX, inc ~200%, core2-EP, inc ~250%. 
on Tigerton-EX, inc 1900%, :) 

for hackbench thread testing: 
on WSM-EP, no clear inc, NHM-EP no clear inc
on NHM-EX, inc 10%, core2-EP, inc ~20%. 
on Tigertion-EX, inc 100%, 

for  netperf loopback testing, no clear performance change. 

RE: [PATCH] slub Discard slab page only when node partials > minimum setting

[Shaohua Li]

On Thu, 2011-09-08 at 08:43 +0800, Shi, Alex wrote:
> On Wed, 2011-09-07 at 23:05 +0800, Christoph Lameter wrote:
> > On Wed, 7 Sep 2011, Shi, Alex wrote:
> > 
> > > Oh, seems the deactivate_slab() corrected at linus' tree already, but
> > > the unfreeze_partials() just copied from the old version
> > > deactivate_slab().
> > 
> > Ok then the patch is ok.
> > 
> > Do you also have performance measurements? I am a bit hesitant to merge
> > the per cpu partials patchset if there are regressions in the low
> > concurrency tests as seem to be indicated by intels latest tests.
> > 
> 
> My LKP testing system most focus on server platforms. I tested your per
> cpu partial set on hackbench and netperf loopback benchmark. hackbench
> improve much.
> 
> Maybe some IO testing is low concurrency for SLUB, maybe a few jobs
> kbuild? or low swap press testing.  I may try them for your patchset in
> the near days. 
> 
> BTW, some testing results for your PCP SLUB:
> 
> for hackbench process testing: 
> on WSM-EP, inc ~60%, NHM-EP inc ~25%
> on NHM-EX, inc ~200%, core2-EP, inc ~250%. 
> on Tigerton-EX, inc 1900%, :) 
> 
> for hackbench thread testing: 
> on WSM-EP, no clear inc, NHM-EP no clear inc
> on NHM-EX, inc 10%, core2-EP, inc ~20%. 
> on Tigertion-EX, inc 100%, 
> 
> for  netperf loopback testing, no clear performance change. 
did you add my patch to add page to partial list tail in the test?
Without it the per-cpu partial list can have more significant impact to
reduce lock contention, so the result isn't precise.

RE: [PATCH] slub Discard slab page only when node partials > minimum setting

[Alex,Shi]

On Thu, 2011-09-08 at 09:34 +0800, Li, Shaohua wrote:
> On Thu, 2011-09-08 at 08:43 +0800, Shi, Alex wrote:
> > On Wed, 2011-09-07 at 23:05 +0800, Christoph Lameter wrote:
> > > On Wed, 7 Sep 2011, Shi, Alex wrote:
> > > 
> > > > Oh, seems the deactivate_slab() corrected at linus' tree already, but
> > > > the unfreeze_partials() just copied from the old version
> > > > deactivate_slab().
> > > 
> > > Ok then the patch is ok.
> > > 
> > > Do you also have performance measurements? I am a bit hesitant to merge
> > > the per cpu partials patchset if there are regressions in the low
> > > concurrency tests as seem to be indicated by intels latest tests.
> > > 
> > 
> > My LKP testing system most focus on server platforms. I tested your per
> > cpu partial set on hackbench and netperf loopback benchmark. hackbench
> > improve much.
> > 
> > Maybe some IO testing is low concurrency for SLUB, maybe a few jobs
> > kbuild? or low swap press testing.  I may try them for your patchset in
> > the near days. 
> > 
> > BTW, some testing results for your PCP SLUB:
> > 
> > for hackbench process testing: 
> > on WSM-EP, inc ~60%, NHM-EP inc ~25%
> > on NHM-EX, inc ~200%, core2-EP, inc ~250%. 
> > on Tigerton-EX, inc 1900%, :) 
> > 
> > for hackbench thread testing: 
> > on WSM-EP, no clear inc, NHM-EP no clear inc
> > on NHM-EX, inc 10%, core2-EP, inc ~20%. 
> > on Tigertion-EX, inc 100%, 
> > 
> > for  netperf loopback testing, no clear performance change. 
> did you add my patch to add page to partial list tail in the test?
> Without it the per-cpu partial list can have more significant impact to
> reduce lock contention, so the result isn't precise.
> 

No, the penberg tree did include your patch on slub/partial head.
Actually PCP won't take that path, so, there is no need for your patch.
I daft a patch to remove some unused code in __slab_free, that related
this, and will send it out later.

But, You reminder me that the compare kernel 3.1-rc2 has a bug. so,
compare to 3.0 kernel, on hackbench process testing, the PCP patchset
just have 5~9% performance on our 4 CPU socket, EX machine, while has
about 2~4% drop on 2 socket EP machines.  :) 

Re: [PATCH] slub Discard slab page only when node partials > minimum setting

[Pekka Enberg]

On Thu, Sep 8, 2011 at 5:24 AM, Alex,Shi <alex.shi@intel.com> wrote:
>> > BTW, some testing results for your PCP SLUB:
>> >
>> > for hackbench process testing:
>> > on WSM-EP, inc ~60%, NHM-EP inc ~25%
>> > on NHM-EX, inc ~200%, core2-EP, inc ~250%.
>> > on Tigerton-EX, inc 1900%, :)
>> >
>> > for hackbench thread testing:
>> > on WSM-EP, no clear inc, NHM-EP no clear inc
>> > on NHM-EX, inc 10%, core2-EP, inc ~20%.
>> > on Tigertion-EX, inc 100%,
>> >
>> > for  netperf loopback testing, no clear performance change.
>> did you add my patch to add page to partial list tail in the test?
>> Without it the per-cpu partial list can have more significant impact to
>> reduce lock contention, so the result isn't precise.
>>
>
> No, the penberg tree did include your patch on slub/partial head.
> Actually PCP won't take that path, so, there is no need for your patch.
> I daft a patch to remove some unused code in __slab_free, that related
> this, and will send it out later.

Which patch is that? Please send me it to penberg@cs.helsinki.fi as
@kernel.org email forward isn't working.

Re: [PATCH] slub Discard slab page only when node partials > minimum setting

[Alex,Shi]

On Thu, 2011-09-15 at 13:40 +0800, Pekka Enberg wrote:
> On Thu, Sep 8, 2011 at 5:24 AM, Alex,Shi <alex.shi@intel.com> wrote:
> >> > BTW, some testing results for your PCP SLUB:
> >> >
> >> > for hackbench process testing:
> >> > on WSM-EP, inc ~60%, NHM-EP inc ~25%
> >> > on NHM-EX, inc ~200%, core2-EP, inc ~250%.
> >> > on Tigerton-EX, inc 1900%, :)
> >> >
> >> > for hackbench thread testing:
> >> > on WSM-EP, no clear inc, NHM-EP no clear inc
> >> > on NHM-EX, inc 10%, core2-EP, inc ~20%.
> >> > on Tigertion-EX, inc 100%,
> >> >
> >> > for  netperf loopback testing, no clear performance change.
> >> did you add my patch to add page to partial list tail in the test?
> >> Without it the per-cpu partial list can have more significant impact to
> >> reduce lock contention, so the result isn't precise.
> >>
> >
> > No, the penberg tree did include your patch on slub/partial head.
> > Actually PCP won't take that path, so, there is no need for your patch.
> > I daft a patch to remove some unused code in __slab_free, that related
> > this, and will send it out later.
> 
> Which patch is that? Please send me it to penberg@cs.helsinki.fi as
> @kernel.org email forward isn't working.


Ops, this thread mentioned 2 patches,
1, shaohua's bug fixing patch, that already in your tree as 'slub/urgent
head', if my memory service me right.

2, [PATCH] slub Discard slab page only when node partials > minimum
setting, that is the following. 

----------
From: Alex Shi <alex.shi@intel.com>
Date: Tue, 6 Sep 2011 14:46:01 +0800
Subject: [PATCH ] Discard slab page when node partial > mininum partial number

Discarding slab should be done when node partial > min_partial.
Otherwise, node partial slab may eat up all memory.

Signed-off-by: Alex Shi <alex.shi@intel.com>
Acked-by: Christoph Lameter <cl@linux.com>
---
 mm/slub.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index 1348c09..492beab 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1953,7 +1953,7 @@ static void unfreeze_partials(struct kmem_cache *s)
 
 			new.frozen = 0;
 
-			if (!new.inuse && (!n || n->nr_partial < s->min_partial))
+			if (!new.inuse && (!n || n->nr_partial > s->min_partial))
 				m = M_FREE;
 			else {
 				struct kmem_cache_node *n2 = get_node(s,
-- 
1.7.0

RE: [PATCH] slub Discard slab page only when node partials > minimum setting

[Christoph Lameter]

On Thu, 8 Sep 2011, Alex,Shi wrote:

> BTW, some testing results for your PCP SLUB:
>
> for hackbench process testing:
> on WSM-EP, inc ~60%, NHM-EP inc ~25%
> on NHM-EX, inc ~200%, core2-EP, inc ~250%.
> on Tigerton-EX, inc 1900%, :)

There is no minus on tigerton. I hope that is not a regression?
>
> for hackbench thread testing:
> on WSM-EP, no clear inc, NHM-EP no clear inc
> on NHM-EX, inc 10%, core2-EP, inc ~20%.
> on Tigertion-EX, inc 100%,

> for  netperf loopback testing, no clear performance change.

Hmmm... The sizes of the per cpu partial objects could be varied a bit to
see if more would make an impact.

RE: [PATCH] slub Discard slab page only when node partials > minimum setting

[Alex,Shi]

On Fri, 2011-09-09 at 02:37 +0800, Christoph Lameter wrote:
> On Thu, 8 Sep 2011, Alex,Shi wrote:
> 
> > BTW, some testing results for your PCP SLUB:
> >
> > for hackbench process testing:
> > on WSM-EP, inc ~60%, NHM-EP inc ~25%
> > on NHM-EX, inc ~200%, core2-EP, inc ~250%.
> > on Tigerton-EX, inc 1900%, :)
> 
> There is no minus on tigerton. I hope that is not a regression?

Sorry for incorrect usage of '~'. I want use the '~60%' to express
performance increased 'about' 60%, not '-60%'. This usage is unusual in
English.

> >
> > for hackbench thread testing:
> > on WSM-EP, no clear inc, NHM-EP no clear inc
> > on NHM-EX, inc 10%, core2-EP, inc ~20%.
> > on Tigertion-EX, inc 100%,
> 
> > for  netperf loopback testing, no clear performance change.
> 
> Hmmm... The sizes of the per cpu partial objects could be varied a bit to
> see if more would make an impact.


I find almost in one time my kbuilding. 
size 384, was alloced in fastpath about 2900k times
size 176, was alloced in fastpath about 1900k times
size 192, was alloced in fastpath about 500k times
anon_vma, was alloced in fastpath about 560k times 
size 72, was alloced in fastpath about 600k times 
size 512, 256, 128, was alloced in fastpath about more than 100k for
each of them.

I may give you objects size involved in my netperf testing later. 
and which test case do you prefer to? If I have, I may collection data
on them. 

RE: [PATCH] slub Discard slab page only when node partials > minimum setting

[Christoph Lameter]

On Fri, 9 Sep 2011, Alex,Shi wrote:

> On Fri, 2011-09-09 at 02:37 +0800, Christoph Lameter wrote:
> > On Thu, 8 Sep 2011, Alex,Shi wrote:
> >
> > > BTW, some testing results for your PCP SLUB:
> > >
> > > for hackbench process testing:
> > > on WSM-EP, inc ~60%, NHM-EP inc ~25%
> > > on NHM-EX, inc ~200%, core2-EP, inc ~250%.
> > > on Tigerton-EX, inc 1900%, :)
> >
> > There is no minus on tigerton. I hope that is not a regression?
>
> Sorry for incorrect usage of '~'. I want use the '~60%' to express
> performance increased 'about' 60%, not '-60%'. This usage is unusual in
> English.

Oh its ~ not -. Did not see that on my small devices that I used at the
conference.

RE: [PATCH] slub Discard slab page only when node partials > minimum setting

[Alex,Shi]

> > Hmmm... The sizes of the per cpu partial objects could be varied a bit to
> > see if more would make an impact.
> 
> 
> I find almost in one time my kbuilding. 
> size 384, was alloced in fastpath about 2900k times
> size 176, was alloced in fastpath about 1900k times
> size 192, was alloced in fastpath about 500k times
> anon_vma, was alloced in fastpath about 560k times 
> size 72, was alloced in fastpath about 600k times 
> size 512, 256, 128, was alloced in fastpath about more than 100k for
> each of them.
> 
> I may give you objects size involved in my netperf testing later. 
> and which test case do you prefer to? If I have, I may collection data
> on them. 

I write a short script to collect different size object usage of
alloc_fastpath.  The output is following, first column is the object
name and second is the alloc_fastpath called times.

:t-0000448 62693419
:t-0000384 1037746
:at-0000104 191787
:t-0000176 2051053
anon_vma 953578
:t-0000048 2108191
:t-0008192 17858636
:t-0004096 2307039
:t-0002048 21601441
:t-0001024 98409238
:t-0000512 14896189
:t-0000256 96731409
:t-0000128 221045
:t-0000064 149505
:t-0000032 638431
:t-0000192 263488
-----

Above output shows size 448/8192/2048/512/256 are used much. 

So at least both kbuild(with 4 jobs) and netperf loopback (one server on
CPU socket 1, and one client on CPU socket 2) testing have no clear
performance change on our machine
NHM-EP/NHM-EX/WSM-EP/tigerton/core2-EP. 

Re: [PATCH] slub Discard slab page only when node partials > minimum setting

[Alex,Shi]

On Tue, 2011-09-13 at 23:04 +0800, Christoph Lameter wrote:
> Sorry to be that late with a response but my email setup is screwed
> up.
> 
> I was more thinking about the number of slab pages in the partial
> caches rather than the size of the objects itself being an issue. I
> believe that was /sys/kernel/slab/*/cpu_partial.
> 
> That setting could be tuned further before merging. An increase there
> causes additional memory to be caught in the partial list. But it
> reduces the node lock pressure further.
> 

Yeah, I think so. The more cpu partial page, the quicker to getting
slabs. Maybe it's better to considerate the system memory size to set
them. Do you has some plan or suggestions on tunning? 

Re: [PATCH] slub Discard slab page only when node partials > minimum setting

[Alex,Shi]

On Thu, 2011-09-15 at 09:51 +0800, Christoph Lameter wrote:
> I have not had time to get into this. I was hoping you could come up
> with something. 

Thanks! 
Um, let me have some try.

> 
> On Wed, Sep 14, 2011 at 8:32 PM, Alex,Shi <alex.shi@intel.com> wrote:
>         On Tue, 2011-09-13 at 23:04 +0800, Christoph Lameter wrote:
>         > Sorry to be that late with a response but my email setup is
>         screwed
>         > up.
>         >
>         > I was more thinking about the number of slab pages in the
>         partial
>         > caches rather than the size of the objects itself being an
>         issue. I
>         > believe that was /sys/kernel/slab/*/cpu_partial.
>         >
>         > That setting could be tuned further before merging. An
>         increase there
>         > causes additional memory to be caught in the partial list.
>         But it
>         > reduces the node lock pressure further.
>         >
>         
>         
>         Yeah, I think so. The more cpu partial page, the quicker to
>         getting
>         slabs. Maybe it's better to considerate the system memory size
>         to set
>         them. Do you has some plan or suggestions on tunning?
>         
>         
>         
>         
> 

Re: [PATCH] slub Discard slab page only when node partials > minimum setting

[Alex,Shi]

[-- Attachment #1: Type: text/plain, Size: 2249 bytes --]

On Sat, 2011-09-24 at 04:02 +0800, Christoph Lameter wrote:
> On Fri, 23 Sep 2011, Alex,Shi wrote:
> 
> > Just did a little bit work on this. I tested hackbench with difference
> > cpu_partial values. The value set in kmem_cache_open(), tried 1/2, 2
> > times, 8 times, 32 times and 128 times original value. Seems the 8 times
> > value has a slight better performance on almost of my machines,
> > nhm-ex/nhm-ep/wsm-ep.
> 
> Is it really worth it? The higher the value the higher the potential
> memory that is stuck in the per cpu partial pages?

It is hard to find best balance. :) 
> 
> > It needs to do far more test on this tunning. I am going to seek more
> > benchmarks next week. and try tune on different cpu_partial size and
> > code path.
> 
> Thanks for all your efforts.

I am tested aim9/netperf, both of them was said related to memory
allocation, but didn't find performance change with/without PCP. Seems
only hackbench sensitive on this. As to aim9, whichever with ourself
configuration, or with Mel Gorman's aim9 configuration from his mmtest,
both of them has no clear performance change for PCP slub. 

Checking the kernel function call graphic via perf record/perf report,
slab function only be used much in hackbench benchmark. 

I also tried different code path
1, remove the s->cpu_partial limitation, performance drop 30% on
"hackbench 100 process 2000"

2, don't dump cpu partial into node partial, on the contrary, don't fill
cpu partial if it's larger than s->cpu_partial. but no positive
performance change for this, and seems a little bit low on 4 sockets
machines. 

3, don't dump cpu partial into node partial, and only fill cpu partial
in allocation when cpu partial is less then s->cpu_partial. insert free
slab into node partial in __slab_free() directly. No clear performance
change for this.  BTW, actually, this purpose won't reduce the node
partial lock times. 

My experiment patch for new code path 2,3, need to disable VM_BUG_ON
since frozen has a short time incoherence. and may left empty slabs
after slab free. so it just a experiment patch. The attachment is for
code path 2. 

Above is what I did this week for PCP. 

BTW, I will take my one week holiday from tomorrow. e-mail access will
be slow. 


[-- Attachment #2: patch-pcpnodump --]
[-- Type: text/x-patch, Size: 3612 bytes --]

diff --git a/mm/slub.c b/mm/slub.c
index 492beab..372f219 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1613,7 +1613,7 @@ static void *get_partial_node(struct kmem_cache *s,
 	spin_lock(&n->list_lock);
 	list_for_each_entry_safe(page, page2, &n->partial, lru) {
 		void *t = acquire_slab(s, n, page, object == NULL);
-		int available;
+		int available = 1;
 
 		if (!t)
 			continue;
@@ -1623,12 +1623,14 @@ static void *get_partial_node(struct kmem_cache *s,
 			c->node = page_to_nid(page);
 			stat(s, ALLOC_FROM_PARTIAL);
 			object = t;
-			available =  page->objects - page->inuse;
 		} else {
 			page->freelist = t;
 			available = put_cpu_partial(s, page, 0);
+			if(!available)
+				add_partial(n, page, 0);
+				
 		}
-		if (kmem_cache_debug(s) || available > s->cpu_partial / 2)
+		if (kmem_cache_debug(s) || !available)
 			break;
 
 	}
@@ -2017,17 +2019,10 @@ int put_cpu_partial(struct kmem_cache *s, struct page *page, int drain)
 		if (oldpage) {
 			pobjects = oldpage->pobjects;
 			pages = oldpage->pages;
-			if (drain && pobjects > s->cpu_partial) {
-				unsigned long flags;
-				/*
-				 * partial array is full. Move the existing
-				 * set to the per node partial list.
-				 */
-				local_irq_save(flags);
-				unfreeze_partials(s);
-				local_irq_restore(flags);
+			if (pobjects > s->cpu_partial) {
 				pobjects = 0;
-				pages = 0;
+				page->frozen = 0;
+				break;
 			}
 		}
 
@@ -2039,7 +2034,10 @@ int put_cpu_partial(struct kmem_cache *s, struct page *page, int drain)
 		page->next = oldpage;
 
 	} while (this_cpu_cmpxchg(s->cpu_slab->partial, oldpage, page) != oldpage);
-	stat(s, CPU_PARTIAL_FREE);
+
+	if(pobjects)
+		stat(s, CPU_PARTIAL_FREE);
+
 	return pobjects;
 }
 
@@ -2472,6 +2470,7 @@ static void __slab_free(struct kmem_cache *s, struct page *page,
 		new.inuse--;
 		if ((!new.inuse || !prior) && !was_frozen && !n) {
 
+
 			if (!kmem_cache_debug(s) && !prior)
 
 				/*
@@ -2482,7 +2481,7 @@ static void __slab_free(struct kmem_cache *s, struct page *page,
 
 			else { /* Needs to be taken off a list */
 
-	                        n = get_node(s, page_to_nid(page));
+				n = get_node(s, page_to_nid(page));
 				/*
 				 * Speculatively acquire the list_lock.
 				 * If the cmpxchg does not succeed then we may
@@ -2492,8 +2491,8 @@ static void __slab_free(struct kmem_cache *s, struct page *page,
 				 * other processors updating the list of slabs.
 				 */
 				spin_lock_irqsave(&n->list_lock, flags);
-
 			}
+
 		}
 		inuse = new.inuse;
 
@@ -2503,23 +2502,23 @@ static void __slab_free(struct kmem_cache *s, struct page *page,
 		"__slab_free"));
 
 	if (likely(!n)) {
-
-		/*
-		 * If we just froze the page then put it onto the
-		 * per cpu partial list.
-		 */
 		if (new.frozen && !was_frozen)
-			put_cpu_partial(s, page, 1);
+			if (!put_cpu_partial(s, page, 1)){
+				n = get_node(s, page_to_nid(page));
+				spin_lock_irqsave(&n->list_lock, flags);
+				goto get_lock;
+			}
 
 		/*
 		 * The list lock was not taken therefore no list
 		 * activity can be necessary.
 		 */
-                if (was_frozen)
-                        stat(s, FREE_FROZEN);
-                return;
-        }
+		if (was_frozen)
+			stat(s, FREE_FROZEN);
+		return;
+	}
 
+get_lock:
 	/*
 	 * was_frozen may have been set after we acquired the list_lock in
 	 * an earlier loop. So we need to check it here again.
@@ -2536,7 +2535,7 @@ static void __slab_free(struct kmem_cache *s, struct page *page,
 		 */
 		if (unlikely(!prior)) {
 			remove_full(s, page);
-			add_partial(n, page, 0);
+			add_partial(n, page, 1);
 			stat(s, FREE_ADD_PARTIAL);
 		}
 	}

Re: [PATCH] slub Discard slab page only when node partials > minimum setting

[Christoph Lameter]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1243 bytes --]

On Thu, 29 Sep 2011, Alex,Shi wrote:

> > Is it really worth it? The higher the value the higher the potential
> > memory that is stuck in the per cpu partial pages?
>
> It is hard to find best balance. :)

Well then lets err on the side of smaller memory use for now.

> I am tested aim9/netperf, both of them was said related to memory
> allocation, but didn't find performance change with/without PCP. Seems
> only hackbench sensitive on this. As to aim9, whichever with ourself
> configuration, or with Mel Gorman's aim9 configuration from his mmtest,
> both of them has no clear performance change for PCP slub.

AIM9 tests are usually single threaded so I would not expect any
differences. Try AIM7? And concurrent netperfs?

The PCP patch helps only if there is node lock contention. Meaning
simultaneous allocations/frees from multiple processor from the same
cache.

> Checking the kernel function call graphic via perf record/perf report,
> slab function only be used much in hackbench benchmark.

Then the question arises if its worthwhile merging if it only affects this
benchmark.

> Above is what I did this week for PCP.
>
> BTW, I will take my one week holiday from tomorrow. e-mail access will
> be slow.

Have a nice holiday.

[-- Attachment #2: Type: TEXT/X-PATCH, Size: 3745 bytes --]

diff --git a/mm/slub.c b/mm/slub.c

index 492beab..372f219 100644

--- a/mm/slub.c

+++ b/mm/slub.c

@@ -1613,7 +1613,7 @@ static void *get_partial_node(struct kmem_cache *s,

 	spin_lock(&n->list_lock);

 	list_for_each_entry_safe(page, page2, &n->partial, lru) {

 		void *t = acquire_slab(s, n, page, object == NULL);

-		int available;

+		int available = 1;

 

 		if (!t)

 			continue;

@@ -1623,12 +1623,14 @@ static void *get_partial_node(struct kmem_cache *s,

 			c->node = page_to_nid(page);

 			stat(s, ALLOC_FROM_PARTIAL);

 			object = t;

-			available =  page->objects - page->inuse;

 		} else {

 			page->freelist = t;

 			available = put_cpu_partial(s, page, 0);

+			if(!available)

+				add_partial(n, page, 0);

+				

 		}

-		if (kmem_cache_debug(s) || available > s->cpu_partial / 2)

+		if (kmem_cache_debug(s) || !available)

 			break;

 

 	}

@@ -2017,17 +2019,10 @@ int put_cpu_partial(struct kmem_cache *s, struct page *page, int drain)

 		if (oldpage) {

 			pobjects = oldpage->pobjects;

 			pages = oldpage->pages;

-			if (drain && pobjects > s->cpu_partial) {

-				unsigned long flags;

-				/*

-				 * partial array is full. Move the existing

-				 * set to the per node partial list.

-				 */

-				local_irq_save(flags);

-				unfreeze_partials(s);

-				local_irq_restore(flags);

+			if (pobjects > s->cpu_partial) {

 				pobjects = 0;

-				pages = 0;

+				page->frozen = 0;

+				break;

 			}

 		}

 

@@ -2039,7 +2034,10 @@ int put_cpu_partial(struct kmem_cache *s, struct page *page, int drain)

 		page->next = oldpage;

 

 	} while (this_cpu_cmpxchg(s->cpu_slab->partial, oldpage, page) != oldpage);

-	stat(s, CPU_PARTIAL_FREE);

+

+	if(pobjects)

+		stat(s, CPU_PARTIAL_FREE);

+

 	return pobjects;

 }

 

@@ -2472,6 +2470,7 @@ static void __slab_free(struct kmem_cache *s, struct page *page,

 		new.inuse--;

 		if ((!new.inuse || !prior) && !was_frozen && !n) {

 

+

 			if (!kmem_cache_debug(s) && !prior)

 

 				/*

@@ -2482,7 +2481,7 @@ static void __slab_free(struct kmem_cache *s, struct page *page,

 

 			else { /* Needs to be taken off a list */

 

-	                        n = get_node(s, page_to_nid(page));

+				n = get_node(s, page_to_nid(page));

 				/*

 				 * Speculatively acquire the list_lock.

 				 * If the cmpxchg does not succeed then we may

@@ -2492,8 +2491,8 @@ static void __slab_free(struct kmem_cache *s, struct page *page,

 				 * other processors updating the list of slabs.

 				 */

 				spin_lock_irqsave(&n->list_lock, flags);

-

 			}

+

 		}

 		inuse = new.inuse;

 

@@ -2503,23 +2502,23 @@ static void __slab_free(struct kmem_cache *s, struct page *page,

 		"__slab_free"));

 

 	if (likely(!n)) {

-

-		/*

-		 * If we just froze the page then put it onto the

-		 * per cpu partial list.

-		 */

 		if (new.frozen && !was_frozen)

-			put_cpu_partial(s, page, 1);

+			if (!put_cpu_partial(s, page, 1)){

+				n = get_node(s, page_to_nid(page));

+				spin_lock_irqsave(&n->list_lock, flags);

+				goto get_lock;

+			}

 

 		/*

 		 * The list lock was not taken therefore no list

 		 * activity can be necessary.

 		 */

-                if (was_frozen)

-                        stat(s, FREE_FROZEN);

-                return;

-        }

+		if (was_frozen)

+			stat(s, FREE_FROZEN);

+		return;

+	}

 

+get_lock:

 	/*

 	 * was_frozen may have been set after we acquired the list_lock in

 	 * an earlier loop. So we need to check it here again.

@@ -2536,7 +2535,7 @@ static void __slab_free(struct kmem_cache *s, struct page *page,

 		 */

 		if (unlikely(!prior)) {

 			remove_full(s, page);

-			add_partial(n, page, 0);

+			add_partial(n, page, 1);

 			stat(s, FREE_ADD_PARTIAL);

 		}

 	}

RE: [PATCH] slub Discard slab page only when node partials > minimum setting

[Shi, Alex]

> > I am tested aim9/netperf, both of them was said related to memory
> > allocation, but didn't find performance change with/without PCP. Seems
> > only hackbench sensitive on this. As to aim9, whichever with ourself
> > configuration, or with Mel Gorman's aim9 configuration from his
> > mmtest, both of them has no clear performance change for PCP slub.
> 
> AIM9 tests are usually single threaded so I would not expect any differences.
> Try AIM7? And concurrent netperfs?

I used aim7+aim9 patch, and setup 2000 process run concurrently. But aim9 
can't have big press on slab in fact. 
As to concurrent netperf, I'd like try it after vacation, if you can wait. :) 
> 
> The PCP patch helps only if there is node lock contention. Meaning
> simultaneous allocations/frees from multiple processor from the same cache.
> 
> > Checking the kernel function call graphic via perf record/perf report,
> > slab function only be used much in hackbench benchmark.
> 
> Then the question arises if its worthwhile merging if it only affects this
> benchmark.
> 

>From my viewpoint, the patch is still helpful on server machines, while no clear 
regression finding on desktop machine. So it useful. 

RE: [PATCH] slub Discard slab page only when node partials > minimum setting

[Christoph Lameter]

On Sun, 2 Oct 2011, Shi, Alex wrote:

> >From my viewpoint, the patch is still helpful on server machines, while no clear
> regression finding on desktop machine. So it useful.

Ok. We still have a few weeks it seems before the next merge phase.

RE: [PATCH] slub Discard slab page only when node partials > minimum setting

[Alex,Shi]

On Mon, 2011-10-03 at 23:21 +0800, Christoph Lameter wrote:
> On Sun, 2 Oct 2011, Shi, Alex wrote:
> 
> > >From my viewpoint, the patch is still helpful on server machines, while no clear
> > regression finding on desktop machine. So it useful.
> 
> Ok. We still have a few weeks it seems before the next merge phase.
> 

I tested multi-threads loopback netperf on our machines, no clear
regression or improvement find on our NHM-EP/NHM-EX/WSM-EP machine. 

but on our 4 sockets tigerton machine with 2048 clients, performance
increase 20% for TCP_RR subcase, and 10% for TCP_STREAM32 subcase;
and on our 2 sockets harpertown machine, the TCP_RR subcase increase
15%. 

The data is quite good! 

Don't find performance change on UDP_STREAM1 and UDP_RR1 subcase. 

RE: [PATCH] slub Discard slab page only when node partials > minimum setting

[Christoph Lameter]

On Sun, 9 Oct 2011, Alex,Shi wrote:

> I tested multi-threads loopback netperf on our machines, no clear
> regression or improvement find on our NHM-EP/NHM-EX/WSM-EP machine.

What are your criteria for a regressio?

> but on our 4 sockets tigerton machine with 2048 clients, performance
> increase 20% for TCP_RR subcase, and 10% for TCP_STREAM32 subcase;
> and on our 2 sockets harpertown machine, the TCP_RR subcase increase
> 15%.
>
> The data is quite good!

Sounds encouraging.

Re: [PATCH] slub Discard slab page only when node partials > minimum setting

[Alex,Shi]

On Thu, 2011-09-15 at 13:48 +0800, Pekka Enberg wrote:
> On Wed, Sep 7, 2011 at 4:03 AM, Alex,Shi <alex.shi@intel.com> wrote:
> > Unfreeze_partials may try to discard slab page, the discarding condition
> > should be 'when node partials number > minimum partial number setting',
> > not '<' in current code.
> >
> > This patch base on penberg's tree's 'slub/partial' head.
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/penberg/slab-2.6.git
> >
> > Signed-off-by: Alex Shi <alex.shi@intel.com>
> >
> > ---
> >  mm/slub.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/mm/slub.c b/mm/slub.c
> > index b351480..66a5b29 100644
> > --- a/mm/slub.c
> > +++ b/mm/slub.c
> > @@ -1954,7 +1954,7 @@ static void unfreeze_partials(struct kmem_cache *s)
> >
> >                        new.frozen = 0;
> >
> > -                       if (!new.inuse && (!n || n->nr_partial < s->min_partial))
> > +                       if (!new.inuse && (!n || n->nr_partial > s->min_partial))
> >                                m = M_FREE;
> >                        else {
> >                                struct kmem_cache_node *n2 = get_node(s,
> 
> Can you please resend the patch with Christoph's ACK and a better
> explanation why the condition needs to be flipped. A reference to
> commit 81107188f123e3c2217ac2f2feb2a1147904c62f ("slub: Fix partial
> count comparison confusion") is probably sufficient.
> 
> P.S. Please use the penberg@cs.helsinki.fi email address for now.
> 
>                         Pekka

Is the following OK? Pekka. :) 

==========
From: Alex Shi <alex.shi@intel.com>
Date: Tue, 6 Sep 2011 14:46:01 +0800
Subject: [PATCH ] Discard slab page when node partial > mininum partial number

Unfreeze_partials will try to discard empty slab pages when the slab
node partial number is greater than s->min_partial, not less than
s->min_partial. Otherwise the empty slab page will keep growing and eat
up all system memory.

Signed-off-by: Alex Shi <alex.shi@intel.com>
Acked-by: Christoph Lameter <cl@linux.com>
---
 mm/slub.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index 1348c09..492beab 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1953,7 +1953,7 @@ static void unfreeze_partials(struct kmem_cache *s)
 
 			new.frozen = 0;
 
-			if (!new.inuse && (!n || n->nr_partial < s->min_partial))
+			if (!new.inuse && (!n || n->nr_partial > s->min_partial))
 				m = M_FREE;
 			else {
 				struct kmem_cache_node *n2 = get_node(s,
-- 
1.7.0