slub Discard slab page only when node partials > minimum setting

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.

Quoted 11 lines · 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

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.

Quoted 3 lines

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?

Quoted 6 lines

Quoted 2 lines

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?

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 <[email protected]>

---
 mm/slub.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

mm/slub.c +0 -0

index 6fca71c..7846951 100644

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 <[email protected]>

---
 mm/slub.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

mm/slub.c +1 -2

index ebb3865..8f68757 100644
@@ -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;

Quoted 3 lines · Alex

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.

Quoted 6 lines · Christoph Lameter

Quoted 3 lines · Alex

Oh, seems the deactivate_slab() corrected at linus' tree already, but the unfreeze_partials() just copied from the old version deactivate_slab().

Quoted 6 lines

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.

Quoted 34 lines · Alex

Quoted 6 lines · Christoph Lameter

Quoted 3 lines · Alex

Oh, seems the deactivate_slab() corrected at linus' tree already, but the unfreeze_partials() just copied from the old version deactivate_slab().

Quoted 6 lines

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

Quoted 1 line

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.

Quoted 12 lines

Quoted 12 lines

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

Quoted 5 lines

Quoted 1 line

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.

Quoted 18 lines · Alex,Shi

Quoted 17 lines

Quoted 7 lines

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%, :)

Quoted 6 lines

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.

Quoted 4 lines

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 [email protected] as @kernel.org email forward isn't working.

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.

Quoted 6 lines · Alex,Shi

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?

Quoted 7 lines

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.

Quoted 9 lines · Christoph Lameter

Quoted 6 lines · Alex,Shi

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%, :)

Quoted 1 line

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.

Quoted 3 lines

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.

Quoted 2 lines

Quoted 2 lines

Hmmm... The sizes of the per cpu partial objects could be varied a bit

to

Quoted 12 lines

Quoted 1 line

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.

Quoted 3 lines

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.

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.

Quoted 11 lines · Christoph Lameter

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?

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

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?

Quoted 5 lines · Alex,Shi

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?

Quoted 4 lines

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.

Quoted 2 lines

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

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.

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.

Quoted 5 lines

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.

Quoted 2 lines

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

Quoted 1 line

From my viewpoint, the patch is still helpful on server machines, while no clear

regression finding on desktop machine. So it useful.

Quoted 2 lines · Alex

Quoted 1 line

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!

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 <[email protected]>

---
 mm/slub.c |    2 +-

 1 files changed, 1 insertions(+), 1 deletions(-)

mm/slub.c +0 -0

index b351480..66a5b29 100644

Quoted 19 lines · 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 <[email protected]>

---
 mm/slub.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

mm/slub.c +0 -0

index b351480..66a5b29 100644

mm/slub.c static void unfreeze_partials(struct kmem_cache *s) · line 1960

Quoted 8 lines

mm/slub.c +1 -1

@@ -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 [email protected] email address for now.

Pekka

Quoted 2 lines · Alex,Shi

I found 2 comments error base your per cpu partial patches. Could you like to review for the correction of them?

Great. Thank you.