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

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

[Alex,Shi]

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



--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH] slub: code optimze in get_partial_node()

[Alex,Shi]

I find a way to reduce a variable in get_partial_node(). That is also
helpful for code understanding. :)

This patch base on 'slub/partial' head of penberg's tree.

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

diff --git a/mm/slub.c b/mm/slub.c
index ebb3865..8f68757 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1600,7 +1600,6 @@ static void *get_partial_node(struct kmem_cache *s,
 {
 	struct page *page, *page2;
 	void *object = NULL;
-	int count = 0;
 
 	/*
 	 * Racy check. If we mistakenly see no partial slabs then we
@@ -1613,17 +1612,16 @@ 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, count == 0);
+		void *t = acquire_slab(s, n, page, object == NULL);
 		int available;
 
 		if (!t)
 			break;
 
-		if (!count) {
+		if (!object) {
 			c->page = page;
 			c->node = page_to_nid(page);
 			stat(s, ALLOC_FROM_PARTIAL);
-			count++;
 			object = t;
 			available =  page->objects - page->inuse;
 		} else {
-- 
1.7.0



--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 2/2] slub: continue to seek slab in node partial if met a null page

[Alex,Shi]

In the per cpu partial slub, we may add a full page into node partial
list. like the following scenario:

	cpu1     		        	cpu2 
    in unfreeze_partials	           in __slab_alloc
	...
   add_partial(n, page, 1);
					alloced from cpu partial, and 
					set frozen = 1.
   second cmpxchg_double_slab()
   set frozen = 0

If it happen, we'd better to skip the full page and to seek next slab in
node partial instead of jump to other nodes.

This patch base on 'slub/partial' head of penberg's tree, and following
code optimize of get_partial_node() patch. 

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 8f68757..6fca71c 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1616,7 +1616,7 @@ static void *get_partial_node(struct kmem_cache *s,
 		int available;
 
 		if (!t)
-			break;
+			continue;
 
 		if (!object) {
 			c->page = page;
-- 
1.7.0



--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 2/2] slub: continue to seek slab in node partial if met a null page

[Christoph Lameter]

On Wed, 7 Sep 2011, Alex,Shi wrote:

> In the per cpu partial slub, we may add a full page into node partial
> list. like the following scenario:
>
> 	cpu1     		        	cpu2
>     in unfreeze_partials	           in __slab_alloc
> 	...
>    add_partial(n, page, 1);
> 					alloced from cpu partial, and
> 					set frozen = 1.
>    second cmpxchg_double_slab()
>    set frozen = 0

This scenario cannot happen as the frozen state confers ownership to a
cpu (like the cpu slabs). The cpu partial lists are different from the per
node partial lists and a slab on the per node partial lists should never
have the frozen bit set.

> If it happen, we'd better to skip the full page and to seek next slab in
> node partial instead of jump to other nodes.

But I agree that the patch can be beneficial if acquire slab ever returns
a full page. That should not happen though. Is this theoretical or do you
have actual tests that show that this occurs?

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 2/2] slub: continue to seek slab in node partial if met a null page

[Alex,Shi]

On Wed, 2011-09-07 at 23:01 +0800, Christoph Lameter wrote:
> On Wed, 7 Sep 2011, Alex,Shi wrote:
> 
> > In the per cpu partial slub, we may add a full page into node partial
> > list. like the following scenario:
> >
> > 	cpu1     		        	cpu2
> >     in unfreeze_partials	           in __slab_alloc
> > 	...
> >    add_partial(n, page, 1);
> > 					alloced from cpu partial, and
> > 					set frozen = 1.
> >    second cmpxchg_double_slab()
> >    set frozen = 0
> 
> This scenario cannot happen as the frozen state confers ownership to a
> cpu (like the cpu slabs). The cpu partial lists are different from the per
> node partial lists and a slab on the per node partial lists should never
> have the frozen bit set.

oh, sorry, I am wrong here. 
Firstly, since unfreeze_partials only drain self cpu partial slabs, and
__slab_alloc also only check self cpu partial. So, above scenario won't
happen.  
Secondly, add_partial mean got the node list_lock already, so if
__slab_alloc try to alloc from the node partial, it won't get the
list_lock before cmpxchg finished. 

> 
> > If it happen, we'd better to skip the full page and to seek next slab in
> > node partial instead of jump to other nodes.
> 
> But I agree that the patch can be beneficial if acquire slab ever returns
> a full page. That should not happen though. Is this theoretical or do you
> have actual tests that show that this occurs?

I didn't find a real case for this now. So, do you still like to pick up
this as a defense for future more lockless usage? 



--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 2/2] slub: continue to seek slab in node partial if met a null page

[Christoph Lameter]

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

> > > If it happen, we'd better to skip the full page and to seek next slab in
> > > node partial instead of jump to other nodes.
> >
> > But I agree that the patch can be beneficial if acquire slab ever returns
> > a full page. That should not happen though. Is this theoretical or do you
> > have actual tests that show that this occurs?
>
> I didn't find a real case for this now. So, do you still like to pick up
> this as a defense for future more lockless usage?

I am at a conference and its a bit difficult to see the state of affairs
right now. Lets first defer it until I can see how this would otherwise
impact things.

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[rfc ] slub: unfreeze full page if it's in node partial

[Alex,Shi]

In the per cpu partial slub, we may add a full page into node partial
list. like the following scenario:

        cpu1                                    cpu2 
    in unfreeze_partials                   in __slab_alloc
        ...
   add_partial(n, page, 1);
                                        alloced from cpu partial, and 
                                        set frozen = 1.
   second cmpxchg_double_slab()
   set frozen = 0


At that time, maybe we'd better to unfreeze it in acquire_slab(). That
let it in 'full list' mode, frozen=0 and freelist = NULL, same as we did
in __slab_alloc()


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 6fca71c..7846951 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1579,7 +1579,7 @@ static inline void *acquire_slab(struct kmem_cache *s,
 			new.inuse = page->objects;
 
 		VM_BUG_ON(new.frozen);
-		new.frozen = 1;
+		new.frozen = freelist != NULL;
 
 	} while (!__cmpxchg_double_slab(s, page,
 			freelist, counters,
-- 
1.7.0



--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [rfc ] slub: unfreeze full page if it's in node partial

[Alex,Shi]

Forget to point, It also base on per cpu partial patches. 

On Wed, 2011-09-07 at 10:56 +0800, Alex,Shi wrote:
> In the per cpu partial slub, we may add a full page into node partial
> list. like the following scenario:
> 
>         cpu1                                    cpu2 
>     in unfreeze_partials                   in __slab_alloc
>         ...
>    add_partial(n, page, 1);
>                                         alloced from cpu partial, and 
>                                         set frozen = 1.
>    second cmpxchg_double_slab()
>    set frozen = 0
> 
> 
> At that time, maybe we'd better to unfreeze it in acquire_slab(). That
> let it in 'full list' mode, frozen=0 and freelist = NULL, same as we did
> in __slab_alloc()
> 
> 
> 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 6fca71c..7846951 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -1579,7 +1579,7 @@ static inline void *acquire_slab(struct kmem_cache *s,
>  			new.inuse = page->objects;
>  
>  		VM_BUG_ON(new.frozen);
> -		new.frozen = 1;
> +		new.frozen = freelist != NULL;
>  
>  	} while (!__cmpxchg_double_slab(s, page,
>  			freelist, counters,


--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slub: code optimze in get_partial_node()

[Christoph Lameter]

On Wed, 7 Sep 2011, Alex,Shi wrote:

> I find a way to reduce a variable in get_partial_node(). That is also
> helpful for code understanding. :)
>
> This patch base on 'slub/partial' head of penberg's tree.

Acked-by: Christoph Lameter <cl@linux.com>

>
> Signed-off-by: Alex Shi <alex.shi@intel.com>
> ---
>  mm/slub.c |    6 ++----
>  1 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/mm/slub.c b/mm/slub.c
> index ebb3865..8f68757 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -1600,7 +1600,6 @@ static void *get_partial_node(struct kmem_cache *s,
>  {
>  	struct page *page, *page2;
>  	void *object = NULL;
> -	int count = 0;
>
>  	/*
>  	 * Racy check. If we mistakenly see no partial slabs then we
> @@ -1613,17 +1612,16 @@ 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, count == 0);
> +		void *t = acquire_slab(s, n, page, object == NULL);
>  		int available;
>
>  		if (!t)
>  			break;
>
> -		if (!count) {
> +		if (!object) {
>  			c->page = page;
>  			c->node = page_to_nid(page);
>  			stat(s, ALLOC_FROM_PARTIAL);
> -			count++;
>  			object = t;
>  			available =  page->objects - page->inuse;
>  		} 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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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. 



--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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.

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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.  :) 





--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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.

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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




--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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.

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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. 

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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.

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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. 





--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Christoph Lameter]

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

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.

On Tue, Sep 13, 2011 at 3:29 AM, Alex,Shi <alex.shi@intel.com> wrote:

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

[-- Attachment #2: Type: text/html, Size: 2536 bytes --]

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? 




--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Christoph Lameter]

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

I have not had time to get into this. I was hoping you could come up with
something.

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

[-- Attachment #2: Type: text/html, Size: 1259 bytes --]

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


--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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. 


--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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.

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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. 

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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.


--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Christoph Lameter]

Acked-by: Christoph Lameter <cl@linux.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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Pekka Enberg]

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

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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





--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slub: correct comments error for per cpu partial

[Alex,Shi]

On Wed, 2011-09-07 at 08:14 +0800, Christoph Lameter wrote:
> On Mon, 5 Sep 2011, Alex,Shi wrote:
> 
> > I found 2 comments error base your per cpu partial patches. Could you
> > like to review for the correction of them?
> 
> Great. Thank you.
Thanks for review. I try to add a little bit formal commit info, but
seems hard to say more. :) how about the following?  

=========

Correct 2 comments errors for per cpu partial patches. 

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

diff --git a/include/linux/slub_def.h b/include/linux/slub_def.h
index 4890ef7..a32bcfd 100644
--- a/include/linux/slub_def.h
+++ b/include/linux/slub_def.h
@@ -82,7 +82,7 @@ struct kmem_cache {
 	int size;		/* The size of an object including meta data */
 	int objsize;		/* The size of an object without meta data */
 	int offset;		/* Free pointer offset. */
-	int cpu_partial;	/* Number of per cpu partial pages to keep around */
+	int cpu_partial;	/* Number of per cpu partial objects to keep around */
 	struct kmem_cache_order_objects oo;
 
 	/* Allocation and freeing of slabs */
diff --git a/mm/slub.c b/mm/slub.c
index 0e286ac..ebb3865 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -3086,7 +3086,7 @@ static int kmem_cache_open(struct kmem_cache *s,
 	 *
 	 * A) The number of objects from per cpu partial slabs dumped to the
 	 *    per node list when we reach the limit.
-	 * B) The number of objects in partial partial slabs to extract from the
+	 * B) The number of objects in cpu partial slabs to extract from the
 	 *    per node list when we run out of per cpu objects. We only fetch 50%
 	 *    to keep some capacity around for frees.
 	 */
-- 
1.7.0




--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>