linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Joonsoo Kim <js1304@gmail.com>
To: Pekka Enberg <penberg@kernel.org>
Cc: Christoph Lameter <cl@linux-foundation.org>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	Joonsoo Kim <js1304@gmail.com>
Subject: [PATCH 3/4] slub: refactoring unfreeze_partials()
Date: Sat,  9 Jun 2012 02:23:16 +0900	[thread overview]
Message-ID: <1339176197-13270-3-git-send-email-js1304@gmail.com> (raw)
In-Reply-To: <1339176197-13270-1-git-send-email-js1304@gmail.com>

Current implementation of unfreeze_partials() is so complicated,
but benefit from it is insignificant. In addition many code in
do {} while loop have a bad influence to a fail rate of cmpxchg_double_slab.
Under current implementation which test status of cpu partial slab
and acquire list_lock in do {} while loop,
we don't need to acquire a list_lock and gain a little benefit
when front of the cpu partial slab is to be discarded, but this is a rare case.
In case that add_partial is performed and cmpxchg_double_slab is failed,
remove_partial should be called case by case.

I think that these are disadvantages of current implementation,
so I do refactoring unfreeze_partials().

Minimizing code in do {} while loop introduce a reduced fail rate
of cmpxchg_double_slab. Below is output of 'slabinfo -r kmalloc-256'
when './perf stat -r 33 hackbench 50 process 4000 > /dev/null' is done.

** before **
Cmpxchg_double Looping
------------------------
Locked Cmpxchg Double redos   182685
Unlocked Cmpxchg Double redos 0

** after **
Cmpxchg_double Looping
------------------------
Locked Cmpxchg Double redos   177995
Unlocked Cmpxchg Double redos 1

We can see cmpxchg_double_slab fail rate is improved slightly.

Bolow is output of './perf stat -r 30 hackbench 50 process 4000 > /dev/null'.

** before **
 Performance counter stats for './hackbench 50 process 4000' (30 runs):

     108517.190463 task-clock                #    7.926 CPUs utilized            ( +-  0.24% )
         2,919,550 context-switches          #    0.027 M/sec                    ( +-  3.07% )
           100,774 CPU-migrations            #    0.929 K/sec                    ( +-  4.72% )
           124,201 page-faults               #    0.001 M/sec                    ( +-  0.15% )
   401,500,234,387 cycles                    #    3.700 GHz                      ( +-  0.24% )
   <not supported> stalled-cycles-frontend
   <not supported> stalled-cycles-backend
   250,576,913,354 instructions              #    0.62  insns per cycle          ( +-  0.13% )
    45,934,956,860 branches                  #  423.297 M/sec                    ( +-  0.14% )
       188,219,787 branch-misses             #    0.41% of all branches          ( +-  0.56% )

      13.691837307 seconds time elapsed                                          ( +-  0.24% )

** after **
 Performance counter stats for './hackbench 50 process 4000' (30 runs):

     107784.479767 task-clock                #    7.928 CPUs utilized            ( +-  0.22% )
         2,834,781 context-switches          #    0.026 M/sec                    ( +-  2.33% )
            93,083 CPU-migrations            #    0.864 K/sec                    ( +-  3.45% )
           123,967 page-faults               #    0.001 M/sec                    ( +-  0.15% )
   398,781,421,836 cycles                    #    3.700 GHz                      ( +-  0.22% )
   <not supported> stalled-cycles-frontend
   <not supported> stalled-cycles-backend
   250,189,160,419 instructions              #    0.63  insns per cycle          ( +-  0.09% )
    45,855,370,128 branches                  #  425.436 M/sec                    ( +-  0.10% )
       169,881,248 branch-misses             #    0.37% of all branches          ( +-  0.43% )

      13.596272341 seconds time elapsed                                          ( +-  0.22% )

No regression is found, but rather we can see slightly better result.

Acked-by: Christoph Lameter <cl@linux.com>
Signed-off-by: Joonsoo Kim <js1304@gmail.com>

diff --git a/mm/slub.c b/mm/slub.c
index 686ed90..b5f2108 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1886,18 +1886,24 @@ redo:
  */
 static void unfreeze_partials(struct kmem_cache *s)
 {
-	struct kmem_cache_node *n = NULL;
+	struct kmem_cache_node *n = NULL, *n2 = NULL;
 	struct kmem_cache_cpu *c = this_cpu_ptr(s->cpu_slab);
 	struct page *page, *discard_page = NULL;
 
 	while ((page = c->partial)) {
-		enum slab_modes { M_PARTIAL, M_FREE };
-		enum slab_modes l, m;
 		struct page new;
 		struct page old;
 
 		c->partial = page->next;
-		l = M_FREE;
+
+		n2 = get_node(s, page_to_nid(page));
+		if (n != n2) {
+			if (n)
+				spin_unlock(&n->list_lock);
+
+			n = n2;
+			spin_lock(&n->list_lock);
+		}
 
 		do {
 
@@ -1910,43 +1916,17 @@ static void unfreeze_partials(struct kmem_cache *s)
 
 			new.frozen = 0;
 
-			if (!new.inuse && (!n || n->nr_partial > s->min_partial))
-				m = M_FREE;
-			else {
-				struct kmem_cache_node *n2 = get_node(s,
-							page_to_nid(page));
-
-				m = M_PARTIAL;
-				if (n != n2) {
-					if (n)
-						spin_unlock(&n->list_lock);
-
-					n = n2;
-					spin_lock(&n->list_lock);
-				}
-			}
-
-			if (l != m) {
-				if (l == M_PARTIAL) {
-					remove_partial(n, page);
-					stat(s, FREE_REMOVE_PARTIAL);
-				} else {
-					add_partial(n, page,
-						DEACTIVATE_TO_TAIL);
-					stat(s, FREE_ADD_PARTIAL);
-				}
-
-				l = m;
-			}
-
 		} while (!__cmpxchg_double_slab(s, page,
 				old.freelist, old.counters,
 				new.freelist, new.counters,
 				"unfreezing slab"));
 
-		if (m == M_FREE) {
+		if (unlikely(!new.inuse && n->nr_partial > s->min_partial)) {
 			page->next = discard_page;
 			discard_page = page;
+		} else {
+			add_partial(n, page, DEACTIVATE_TO_TAIL);
+			stat(s, FREE_ADD_PARTIAL);
 		}
 	}
 
-- 
1.7.9.5

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

  parent reply	other threads:[~2012-06-08 17:25 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <yes>
2012-06-08 17:23 ` [PATCH 1/4] slub: change declare of get_slab() to inline at all times Joonsoo Kim
2012-06-08 17:23   ` [PATCH 2/4] slub: use __cmpxchg_double_slab() at interrupt disabled place Joonsoo Kim
2012-06-08 17:23   ` Joonsoo Kim [this message]
2012-06-20  7:19     ` [PATCH 3/4] slub: refactoring unfreeze_partials() Pekka Enberg
2012-06-08 17:23   ` [PATCH 4/4] slub: deactivate freelist of kmem_cache_cpu all at once in deactivate_slab() Joonsoo Kim
2012-06-08 19:04     ` Christoph Lameter
2012-06-10 10:27       ` JoonSoo Kim
2012-06-22 18:34         ` JoonSoo Kim
2012-06-08 19:02   ` [PATCH 1/4] slub: change declare of get_slab() to inline at all times Christoph Lameter
2012-06-09 15:57     ` JoonSoo Kim
2012-06-11 15:04       ` Christoph Lameter
2012-06-22 18:22 ` [PATCH 1/3] slub: prefetch next freelist pointer in __slab_alloc() Joonsoo Kim
2012-06-22 18:22   ` [PATCH 2/3] slub: reduce failure of this_cpu_cmpxchg in put_cpu_partial() after unfreezing Joonsoo Kim
2012-07-04 13:05     ` Pekka Enberg
2012-07-05 14:20       ` Christoph Lameter
2012-08-16  7:06     ` Pekka Enberg
2012-06-22 18:22   ` [PATCH 3/3] slub: release a lock if freeing object with a lock is failed in __slab_free() Joonsoo Kim
2012-07-04 13:10     ` Pekka Enberg
2012-07-04 14:48       ` JoonSoo Kim
2012-07-05 14:26     ` Christoph Lameter
2012-07-06 14:19       ` JoonSoo Kim
2012-07-06 14:34         ` Christoph Lameter
2012-07-06 14:59           ` JoonSoo Kim
2012-07-06 15:10             ` Christoph Lameter
2012-07-08 16:19               ` JoonSoo Kim
2012-06-22 18:45   ` [PATCH 1/3 v2] slub: prefetch next freelist pointer in __slab_alloc() Joonsoo Kim
2012-07-04 12:58     ` JoonSoo Kim
2012-07-04 13:00     ` Pekka Enberg
2012-07-04 14:30       ` JoonSoo Kim
2012-07-04 15:08         ` Pekka Enberg
2012-07-04 15:26           ` Eric Dumazet
2012-07-04 15:48             ` JoonSoo Kim
2012-07-04 16:15               ` Eric Dumazet
2012-07-04 16:24                 ` JoonSoo Kim
2012-07-04 15:45           ` JoonSoo Kim
2012-07-04 15:59             ` Pekka Enberg
2012-07-04 16:04               ` JoonSoo Kim

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1339176197-13270-3-git-send-email-js1304@gmail.com \
    --to=js1304@gmail.com \
    --cc=cl@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=penberg@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).