* [PATCH v2 1/3] slub: correct to calculate num of acquired objects in get_partial_node()
@ 2013-01-21 8:01 Joonsoo Kim
2013-01-21 8:01 ` [PATCH v2 2/3] slub: correct bootstrap() for kmem_cache, kmem_cache_node Joonsoo Kim
` (5 more replies)
0 siblings, 6 replies; 10+ messages in thread
From: Joonsoo Kim @ 2013-01-21 8:01 UTC (permalink / raw)
To: Pekka Enberg
Cc: Christoph Lameter, js1304, linux-mm, linux-kernel, Joonsoo Kim
There is a subtle bug when calculating a number of acquired objects.
Currently, we calculate "available = page->objects - page->inuse",
after acquire_slab() is called in get_partial_node().
In acquire_slab() with mode = 1, we always set new.inuse = page->objects.
So,
acquire_slab(s, n, page, object == NULL);
if (!object) {
c->page = page;
stat(s, ALLOC_FROM_PARTIAL);
object = t;
available = page->objects - page->inuse;
!!! availabe is always 0 !!!
...
Therfore, "available > s->cpu_partial / 2" is always false and
we always go to second iteration.
This patch correct this problem.
After that, we don't need return value of put_cpu_partial().
So remove it.
v2: calculate nr of objects using new.objects and new.inuse.
It is more accurate way than before.
Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
diff --git a/mm/slub.c b/mm/slub.c
index ba2ca53..7204c74 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1493,7 +1493,7 @@ static inline void remove_partial(struct kmem_cache_node *n,
*/
static inline void *acquire_slab(struct kmem_cache *s,
struct kmem_cache_node *n, struct page *page,
- int mode)
+ int mode, int *objects)
{
void *freelist;
unsigned long counters;
@@ -1507,6 +1507,7 @@ static inline void *acquire_slab(struct kmem_cache *s,
freelist = page->freelist;
counters = page->counters;
new.counters = counters;
+ *objects = new.objects - new.inuse;
if (mode) {
new.inuse = page->objects;
new.freelist = NULL;
@@ -1528,7 +1529,7 @@ static inline void *acquire_slab(struct kmem_cache *s,
return freelist;
}
-static int put_cpu_partial(struct kmem_cache *s, struct page *page, int drain);
+static void put_cpu_partial(struct kmem_cache *s, struct page *page, int drain);
static inline bool pfmemalloc_match(struct page *page, gfp_t gfpflags);
/*
@@ -1539,6 +1540,8 @@ static void *get_partial_node(struct kmem_cache *s, struct kmem_cache_node *n,
{
struct page *page, *page2;
void *object = NULL;
+ int available = 0;
+ int objects;
/*
* Racy check. If we mistakenly see no partial slabs then we
@@ -1552,22 +1555,21 @@ static void *get_partial_node(struct kmem_cache *s, struct kmem_cache_node *n,
spin_lock(&n->list_lock);
list_for_each_entry_safe(page, page2, &n->partial, lru) {
void *t;
- int available;
if (!pfmemalloc_match(page, flags))
continue;
- t = acquire_slab(s, n, page, object == NULL);
+ t = acquire_slab(s, n, page, object == NULL, &objects);
if (!t)
break;
+ available += objects;
if (!object) {
c->page = page;
stat(s, ALLOC_FROM_PARTIAL);
object = t;
- available = page->objects - page->inuse;
} else {
- available = put_cpu_partial(s, page, 0);
+ put_cpu_partial(s, page, 0);
stat(s, CPU_PARTIAL_NODE);
}
if (kmem_cache_debug(s) || available > s->cpu_partial / 2)
@@ -1946,7 +1948,7 @@ static void unfreeze_partials(struct kmem_cache *s,
* If we did not find a slot then simply move all the partials to the
* per node partial list.
*/
-static int put_cpu_partial(struct kmem_cache *s, struct page *page, int drain)
+static void put_cpu_partial(struct kmem_cache *s, struct page *page, int drain)
{
struct page *oldpage;
int pages;
@@ -1984,7 +1986,6 @@ static 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);
- return pobjects;
}
static inline void flush_slab(struct kmem_cache *s, struct kmem_cache_cpu *c)
--
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>
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 2/3] slub: correct bootstrap() for kmem_cache, kmem_cache_node
2013-01-21 8:01 [PATCH v2 1/3] slub: correct to calculate num of acquired objects in get_partial_node() Joonsoo Kim
@ 2013-01-21 8:01 ` Joonsoo Kim
2013-01-23 15:25 ` Christoph Lameter
2013-01-21 8:01 ` [PATCH v2 3/3] slub: add 'likely' macro to inc_slabs_node() Joonsoo Kim
` (4 subsequent siblings)
5 siblings, 1 reply; 10+ messages in thread
From: Joonsoo Kim @ 2013-01-21 8:01 UTC (permalink / raw)
To: Pekka Enberg
Cc: Christoph Lameter, js1304, linux-mm, linux-kernel, Joonsoo Kim
Current implementation of bootstrap() is not sufficient for kmem_cache
and kmem_cache_node.
First, for kmem_cache.
bootstrap() call kmem_cache_zalloc() at first. When kmem_cache_zalloc()
is called, kmem_cache's slab is moved to cpu slab for satisfying kmem_cache
allocation request. In current implementation, we only consider
n->partial slabs, so, we miss this cpu slab for kmem_cache.
Second, for kmem_cache_node.
When slab_state = PARTIAL, create_boot_cache() is called. And then,
kmem_cache_node's slab is moved to cpu slab for satisfying kmem_cache_node
allocation request. So, we also miss this slab.
These didn't make any error previously, because we normally don't free
objects which comes from kmem_cache's first slab and kmem_cache_node's.
Problem will be solved if we consider a cpu slab in bootstrap().
This patch implement it.
v2: don't loop over all processors in bootstrap().
Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
diff --git a/mm/slub.c b/mm/slub.c
index 7204c74..8b95364 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -3614,10 +3614,15 @@ static int slab_memory_callback(struct notifier_block *self,
static struct kmem_cache * __init bootstrap(struct kmem_cache *static_cache)
{
int node;
+ struct kmem_cache_cpu *c;
struct kmem_cache *s = kmem_cache_zalloc(kmem_cache, GFP_NOWAIT);
memcpy(s, static_cache, kmem_cache->object_size);
+ c = this_cpu_ptr(s->cpu_slab);
+ if (c->page)
+ c->page->slab_cache = s;
+
for_each_node_state(node, N_NORMAL_MEMORY) {
struct kmem_cache_node *n = get_node(s, node);
struct page *p;
--
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>
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 3/3] slub: add 'likely' macro to inc_slabs_node()
2013-01-21 8:01 [PATCH v2 1/3] slub: correct to calculate num of acquired objects in get_partial_node() Joonsoo Kim
2013-01-21 8:01 ` [PATCH v2 2/3] slub: correct bootstrap() for kmem_cache, kmem_cache_node Joonsoo Kim
@ 2013-01-21 8:01 ` Joonsoo Kim
2013-01-23 5:15 ` [PATCH v2 1/3] slub: correct to calculate num of acquired objects in get_partial_node() Wanpeng Li
` (3 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Joonsoo Kim @ 2013-01-21 8:01 UTC (permalink / raw)
To: Pekka Enberg
Cc: Christoph Lameter, js1304, linux-mm, linux-kernel, Joonsoo Kim
After boot phase, 'n' always exist.
So add 'likely' macro for helping compiler.
Acked-by: Christoph Lameter <cl@linux.com>
Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
diff --git a/mm/slub.c b/mm/slub.c
index 8b95364..ddbd401 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1005,7 +1005,7 @@ static inline void inc_slabs_node(struct kmem_cache *s, int node, int objects)
* dilemma by deferring the increment of the count during
* bootstrap (see early_kmem_cache_node_alloc).
*/
- if (n) {
+ if (likely(n)) {
atomic_long_inc(&n->nr_slabs);
atomic_long_add(objects, &n->total_objects);
}
--
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>
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/3] slub: correct to calculate num of acquired objects in get_partial_node()
2013-01-21 8:01 [PATCH v2 1/3] slub: correct to calculate num of acquired objects in get_partial_node() Joonsoo Kim
` (2 preceding siblings ...)
2013-01-23 5:15 ` [PATCH v2 1/3] slub: correct to calculate num of acquired objects in get_partial_node() Wanpeng Li
@ 2013-01-23 5:15 ` Wanpeng Li
2013-01-23 15:25 ` Christoph Lameter
2013-03-19 5:10 ` Joonsoo Kim
5 siblings, 0 replies; 10+ messages in thread
From: Wanpeng Li @ 2013-01-23 5:15 UTC (permalink / raw)
To: Joonsoo Kim
Cc: Pekka Enberg, Christoph Lameter, js1304, linux-mm, linux-kernel
On Mon, Jan 21, 2013 at 05:01:25PM +0900, Joonsoo Kim wrote:
>There is a subtle bug when calculating a number of acquired objects.
>
>Currently, we calculate "available = page->objects - page->inuse",
>after acquire_slab() is called in get_partial_node().
>
>In acquire_slab() with mode = 1, we always set new.inuse = page->objects.
>So,
>
> acquire_slab(s, n, page, object == NULL);
>
> if (!object) {
> c->page = page;
> stat(s, ALLOC_FROM_PARTIAL);
> object = t;
> available = page->objects - page->inuse;
>
> !!! availabe is always 0 !!!
> ...
>
>Therfore, "available > s->cpu_partial / 2" is always false and
>we always go to second iteration.
>This patch correct this problem.
>
>After that, we don't need return value of put_cpu_partial().
>So remove it.
>
>v2: calculate nr of objects using new.objects and new.inuse.
>It is more accurate way than before.
>
Reviewed-by: Wanpeng Li <liwanp@linux.vnet.ibm.com>
>Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
>
>diff --git a/mm/slub.c b/mm/slub.c
>index ba2ca53..7204c74 100644
>--- a/mm/slub.c
>+++ b/mm/slub.c
>@@ -1493,7 +1493,7 @@ static inline void remove_partial(struct kmem_cache_node *n,
> */
> static inline void *acquire_slab(struct kmem_cache *s,
> struct kmem_cache_node *n, struct page *page,
>- int mode)
>+ int mode, int *objects)
> {
> void *freelist;
> unsigned long counters;
>@@ -1507,6 +1507,7 @@ static inline void *acquire_slab(struct kmem_cache *s,
> freelist = page->freelist;
> counters = page->counters;
> new.counters = counters;
>+ *objects = new.objects - new.inuse;
> if (mode) {
> new.inuse = page->objects;
> new.freelist = NULL;
>@@ -1528,7 +1529,7 @@ static inline void *acquire_slab(struct kmem_cache *s,
> return freelist;
> }
>
>-static int put_cpu_partial(struct kmem_cache *s, struct page *page, int drain);
>+static void put_cpu_partial(struct kmem_cache *s, struct page *page, int drain);
> static inline bool pfmemalloc_match(struct page *page, gfp_t gfpflags);
>
> /*
>@@ -1539,6 +1540,8 @@ static void *get_partial_node(struct kmem_cache *s, struct kmem_cache_node *n,
> {
> struct page *page, *page2;
> void *object = NULL;
>+ int available = 0;
>+ int objects;
>
> /*
> * Racy check. If we mistakenly see no partial slabs then we
>@@ -1552,22 +1555,21 @@ static void *get_partial_node(struct kmem_cache *s, struct kmem_cache_node *n,
> spin_lock(&n->list_lock);
> list_for_each_entry_safe(page, page2, &n->partial, lru) {
> void *t;
>- int available;
>
> if (!pfmemalloc_match(page, flags))
> continue;
>
>- t = acquire_slab(s, n, page, object == NULL);
>+ t = acquire_slab(s, n, page, object == NULL, &objects);
> if (!t)
> break;
>
>+ available += objects;
> if (!object) {
> c->page = page;
> stat(s, ALLOC_FROM_PARTIAL);
> object = t;
>- available = page->objects - page->inuse;
> } else {
>- available = put_cpu_partial(s, page, 0);
>+ put_cpu_partial(s, page, 0);
> stat(s, CPU_PARTIAL_NODE);
> }
> if (kmem_cache_debug(s) || available > s->cpu_partial / 2)
>@@ -1946,7 +1948,7 @@ static void unfreeze_partials(struct kmem_cache *s,
> * If we did not find a slot then simply move all the partials to the
> * per node partial list.
> */
>-static int put_cpu_partial(struct kmem_cache *s, struct page *page, int drain)
>+static void put_cpu_partial(struct kmem_cache *s, struct page *page, int drain)
> {
> struct page *oldpage;
> int pages;
>@@ -1984,7 +1986,6 @@ static 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);
>- return pobjects;
> }
>
> static inline void flush_slab(struct kmem_cache *s, struct kmem_cache_cpu *c)
>--
>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>
--
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>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/3] slub: correct to calculate num of acquired objects in get_partial_node()
2013-01-21 8:01 [PATCH v2 1/3] slub: correct to calculate num of acquired objects in get_partial_node() Joonsoo Kim
2013-01-21 8:01 ` [PATCH v2 2/3] slub: correct bootstrap() for kmem_cache, kmem_cache_node Joonsoo Kim
2013-01-21 8:01 ` [PATCH v2 3/3] slub: add 'likely' macro to inc_slabs_node() Joonsoo Kim
@ 2013-01-23 5:15 ` Wanpeng Li
2013-01-23 5:15 ` Wanpeng Li
` (2 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Wanpeng Li @ 2013-01-23 5:15 UTC (permalink / raw)
Cc: Pekka Enberg, Christoph Lameter, js1304, linux-mm, linux-kernel,
Joonsoo Kim
On Mon, Jan 21, 2013 at 05:01:25PM +0900, Joonsoo Kim wrote:
>There is a subtle bug when calculating a number of acquired objects.
>
>Currently, we calculate "available = page->objects - page->inuse",
>after acquire_slab() is called in get_partial_node().
>
>In acquire_slab() with mode = 1, we always set new.inuse = page->objects.
>So,
>
> acquire_slab(s, n, page, object == NULL);
>
> if (!object) {
> c->page = page;
> stat(s, ALLOC_FROM_PARTIAL);
> object = t;
> available = page->objects - page->inuse;
>
> !!! availabe is always 0 !!!
> ...
>
>Therfore, "available > s->cpu_partial / 2" is always false and
>we always go to second iteration.
>This patch correct this problem.
>
>After that, we don't need return value of put_cpu_partial().
>So remove it.
>
>v2: calculate nr of objects using new.objects and new.inuse.
>It is more accurate way than before.
>
Reviewed-by: Wanpeng Li <liwanp@linux.vnet.ibm.com>
>Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
>
>diff --git a/mm/slub.c b/mm/slub.c
>index ba2ca53..7204c74 100644
>--- a/mm/slub.c
>+++ b/mm/slub.c
>@@ -1493,7 +1493,7 @@ static inline void remove_partial(struct kmem_cache_node *n,
> */
> static inline void *acquire_slab(struct kmem_cache *s,
> struct kmem_cache_node *n, struct page *page,
>- int mode)
>+ int mode, int *objects)
> {
> void *freelist;
> unsigned long counters;
>@@ -1507,6 +1507,7 @@ static inline void *acquire_slab(struct kmem_cache *s,
> freelist = page->freelist;
> counters = page->counters;
> new.counters = counters;
>+ *objects = new.objects - new.inuse;
> if (mode) {
> new.inuse = page->objects;
> new.freelist = NULL;
>@@ -1528,7 +1529,7 @@ static inline void *acquire_slab(struct kmem_cache *s,
> return freelist;
> }
>
>-static int put_cpu_partial(struct kmem_cache *s, struct page *page, int drain);
>+static void put_cpu_partial(struct kmem_cache *s, struct page *page, int drain);
> static inline bool pfmemalloc_match(struct page *page, gfp_t gfpflags);
>
> /*
>@@ -1539,6 +1540,8 @@ static void *get_partial_node(struct kmem_cache *s, struct kmem_cache_node *n,
> {
> struct page *page, *page2;
> void *object = NULL;
>+ int available = 0;
>+ int objects;
>
> /*
> * Racy check. If we mistakenly see no partial slabs then we
>@@ -1552,22 +1555,21 @@ static void *get_partial_node(struct kmem_cache *s, struct kmem_cache_node *n,
> spin_lock(&n->list_lock);
> list_for_each_entry_safe(page, page2, &n->partial, lru) {
> void *t;
>- int available;
>
> if (!pfmemalloc_match(page, flags))
> continue;
>
>- t = acquire_slab(s, n, page, object == NULL);
>+ t = acquire_slab(s, n, page, object == NULL, &objects);
> if (!t)
> break;
>
>+ available += objects;
> if (!object) {
> c->page = page;
> stat(s, ALLOC_FROM_PARTIAL);
> object = t;
>- available = page->objects - page->inuse;
> } else {
>- available = put_cpu_partial(s, page, 0);
>+ put_cpu_partial(s, page, 0);
> stat(s, CPU_PARTIAL_NODE);
> }
> if (kmem_cache_debug(s) || available > s->cpu_partial / 2)
>@@ -1946,7 +1948,7 @@ static void unfreeze_partials(struct kmem_cache *s,
> * If we did not find a slot then simply move all the partials to the
> * per node partial list.
> */
>-static int put_cpu_partial(struct kmem_cache *s, struct page *page, int drain)
>+static void put_cpu_partial(struct kmem_cache *s, struct page *page, int drain)
> {
> struct page *oldpage;
> int pages;
>@@ -1984,7 +1986,6 @@ static 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);
>- return pobjects;
> }
>
> static inline void flush_slab(struct kmem_cache *s, struct kmem_cache_cpu *c)
>--
>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>
--
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>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/3] slub: correct to calculate num of acquired objects in get_partial_node()
2013-01-21 8:01 [PATCH v2 1/3] slub: correct to calculate num of acquired objects in get_partial_node() Joonsoo Kim
` (3 preceding siblings ...)
2013-01-23 5:15 ` Wanpeng Li
@ 2013-01-23 15:25 ` Christoph Lameter
2013-03-19 5:10 ` Joonsoo Kim
5 siblings, 0 replies; 10+ messages in thread
From: Christoph Lameter @ 2013-01-23 15:25 UTC (permalink / raw)
To: Joonsoo Kim; +Cc: Pekka Enberg, js1304, linux-mm, linux-kernel
On Mon, 21 Jan 2013, Joonsoo Kim wrote:
> v2: calculate nr of objects using new.objects and new.inuse.
> It is more accurate way than before.
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/3] slub: correct bootstrap() for kmem_cache, kmem_cache_node
2013-01-21 8:01 ` [PATCH v2 2/3] slub: correct bootstrap() for kmem_cache, kmem_cache_node Joonsoo Kim
@ 2013-01-23 15:25 ` Christoph Lameter
0 siblings, 0 replies; 10+ messages in thread
From: Christoph Lameter @ 2013-01-23 15:25 UTC (permalink / raw)
To: Joonsoo Kim; +Cc: Pekka Enberg, js1304, linux-mm, linux-kernel
On Mon, 21 Jan 2013, Joonsoo Kim wrote:
> v2: don't loop over all processors in bootstrap().
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/3] slub: correct to calculate num of acquired objects in get_partial_node()
2013-01-21 8:01 [PATCH v2 1/3] slub: correct to calculate num of acquired objects in get_partial_node() Joonsoo Kim
` (4 preceding siblings ...)
2013-01-23 15:25 ` Christoph Lameter
@ 2013-03-19 5:10 ` Joonsoo Kim
2013-04-02 6:43 ` Pekka Enberg
5 siblings, 1 reply; 10+ messages in thread
From: Joonsoo Kim @ 2013-03-19 5:10 UTC (permalink / raw)
To: Pekka Enberg; +Cc: Christoph Lameter, linux-mm, linux-kernel
Hello, Pekka.
Could you pick up 1/3, 3/3?
These are already acked by Christoph.
2/3 is same effect as Glauber's "slub: correctly bootstrap boot caches",
so should skip it.
Thanks.
On Mon, Jan 21, 2013 at 05:01:25PM +0900, Joonsoo Kim wrote:
> There is a subtle bug when calculating a number of acquired objects.
>
> Currently, we calculate "available = page->objects - page->inuse",
> after acquire_slab() is called in get_partial_node().
>
> In acquire_slab() with mode = 1, we always set new.inuse = page->objects.
> So,
>
> acquire_slab(s, n, page, object == NULL);
>
> if (!object) {
> c->page = page;
> stat(s, ALLOC_FROM_PARTIAL);
> object = t;
> available = page->objects - page->inuse;
>
> !!! availabe is always 0 !!!
> ...
>
> Therfore, "available > s->cpu_partial / 2" is always false and
> we always go to second iteration.
> This patch correct this problem.
>
> After that, we don't need return value of put_cpu_partial().
> So remove it.
>
> v2: calculate nr of objects using new.objects and new.inuse.
> It is more accurate way than before.
>
> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
>
> diff --git a/mm/slub.c b/mm/slub.c
> index ba2ca53..7204c74 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -1493,7 +1493,7 @@ static inline void remove_partial(struct kmem_cache_node *n,
> */
> static inline void *acquire_slab(struct kmem_cache *s,
> struct kmem_cache_node *n, struct page *page,
> - int mode)
> + int mode, int *objects)
> {
> void *freelist;
> unsigned long counters;
> @@ -1507,6 +1507,7 @@ static inline void *acquire_slab(struct kmem_cache *s,
> freelist = page->freelist;
> counters = page->counters;
> new.counters = counters;
> + *objects = new.objects - new.inuse;
> if (mode) {
> new.inuse = page->objects;
> new.freelist = NULL;
> @@ -1528,7 +1529,7 @@ static inline void *acquire_slab(struct kmem_cache *s,
> return freelist;
> }
>
> -static int put_cpu_partial(struct kmem_cache *s, struct page *page, int drain);
> +static void put_cpu_partial(struct kmem_cache *s, struct page *page, int drain);
> static inline bool pfmemalloc_match(struct page *page, gfp_t gfpflags);
>
> /*
> @@ -1539,6 +1540,8 @@ static void *get_partial_node(struct kmem_cache *s, struct kmem_cache_node *n,
> {
> struct page *page, *page2;
> void *object = NULL;
> + int available = 0;
> + int objects;
>
> /*
> * Racy check. If we mistakenly see no partial slabs then we
> @@ -1552,22 +1555,21 @@ static void *get_partial_node(struct kmem_cache *s, struct kmem_cache_node *n,
> spin_lock(&n->list_lock);
> list_for_each_entry_safe(page, page2, &n->partial, lru) {
> void *t;
> - int available;
>
> if (!pfmemalloc_match(page, flags))
> continue;
>
> - t = acquire_slab(s, n, page, object == NULL);
> + t = acquire_slab(s, n, page, object == NULL, &objects);
> if (!t)
> break;
>
> + available += objects;
> if (!object) {
> c->page = page;
> stat(s, ALLOC_FROM_PARTIAL);
> object = t;
> - available = page->objects - page->inuse;
> } else {
> - available = put_cpu_partial(s, page, 0);
> + put_cpu_partial(s, page, 0);
> stat(s, CPU_PARTIAL_NODE);
> }
> if (kmem_cache_debug(s) || available > s->cpu_partial / 2)
> @@ -1946,7 +1948,7 @@ static void unfreeze_partials(struct kmem_cache *s,
> * If we did not find a slot then simply move all the partials to the
> * per node partial list.
> */
> -static int put_cpu_partial(struct kmem_cache *s, struct page *page, int drain)
> +static void put_cpu_partial(struct kmem_cache *s, struct page *page, int drain)
> {
> struct page *oldpage;
> int pages;
> @@ -1984,7 +1986,6 @@ static 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);
> - return pobjects;
> }
>
> static inline void flush_slab(struct kmem_cache *s, struct kmem_cache_cpu *c)
> --
> 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>
--
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>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/3] slub: correct to calculate num of acquired objects in get_partial_node()
2013-03-19 5:10 ` Joonsoo Kim
@ 2013-04-02 6:43 ` Pekka Enberg
2013-04-02 19:17 ` Christoph Lameter
0 siblings, 1 reply; 10+ messages in thread
From: Pekka Enberg @ 2013-04-02 6:43 UTC (permalink / raw)
To: Joonsoo Kim; +Cc: Christoph Lameter, linux-mm@kvack.org, LKML
On Tue, Mar 19, 2013 at 7:10 AM, Joonsoo Kim <iamjoonsoo.kim@lge.com> wrote:
> Could you pick up 1/3, 3/3?
> These are already acked by Christoph.
> 2/3 is same effect as Glauber's "slub: correctly bootstrap boot caches",
> so should skip it.
Applied, thanks!
--
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>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/3] slub: correct to calculate num of acquired objects in get_partial_node()
2013-04-02 6:43 ` Pekka Enberg
@ 2013-04-02 19:17 ` Christoph Lameter
0 siblings, 0 replies; 10+ messages in thread
From: Christoph Lameter @ 2013-04-02 19:17 UTC (permalink / raw)
To: Pekka Enberg; +Cc: Joonsoo Kim, linux-mm@kvack.org, LKML
On Tue, 2 Apr 2013, Pekka Enberg wrote:
> On Tue, Mar 19, 2013 at 7:10 AM, Joonsoo Kim <iamjoonsoo.kim@lge.com> wrote:
> > Could you pick up 1/3, 3/3?
> > These are already acked by Christoph.
> > 2/3 is same effect as Glauber's "slub: correctly bootstrap boot caches",
> > so should skip it.
>
> Applied, thanks!
Could you also put in
1. The fixes for the hotpath using preempt/enable/disable that were
discussed with the RT folks a couple of months ago.
2. The fixes from the slab next branch.
--
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>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2013-04-02 19:17 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-21 8:01 [PATCH v2 1/3] slub: correct to calculate num of acquired objects in get_partial_node() Joonsoo Kim
2013-01-21 8:01 ` [PATCH v2 2/3] slub: correct bootstrap() for kmem_cache, kmem_cache_node Joonsoo Kim
2013-01-23 15:25 ` Christoph Lameter
2013-01-21 8:01 ` [PATCH v2 3/3] slub: add 'likely' macro to inc_slabs_node() Joonsoo Kim
2013-01-23 5:15 ` [PATCH v2 1/3] slub: correct to calculate num of acquired objects in get_partial_node() Wanpeng Li
2013-01-23 5:15 ` Wanpeng Li
2013-01-23 15:25 ` Christoph Lameter
2013-03-19 5:10 ` Joonsoo Kim
2013-04-02 6:43 ` Pekka Enberg
2013-04-02 19:17 ` Christoph Lameter
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).