[PATCH v2] slub: correctly bootstrap boot caches

[PATCH v2] slub: correctly bootstrap boot caches

[Glauber Costa]

After we create a boot cache, we may allocate from it until it is bootstraped.
This will move the page from the partial list to the cpu slab list. If this
happens, the loop:

	list_for_each_entry(p, &n->partial, lru)

that we use to scan for all partial pages will yield nothing, and the pages
will keep pointing to the boot cpu cache, which is of course, invalid. To do
that, we should flush the cache to make sure that the cpu slab is back to the
partial list.

Signed-off-by: Glauber Costa <glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
Reported-by:  Steffen Michalke <StMichalke-S0/GAf8tV78@public.gmane.org>
Tested-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org>
Cc: Christoph Lameter <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org>
Cc: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
Cc: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Pekka Enberg <penberg-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

 mm/slub.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)
---
 mm/slub.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/mm/slub.c b/mm/slub.c
index ba2ca53..31a494c 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -3617,6 +3617,12 @@ static struct kmem_cache * __init bootstrap(struct kmem_cache *static_cache)
 
 	memcpy(s, static_cache, kmem_cache->object_size);
 
+	/*
+	 * This runs very early, and only the boot processor is supposed to be
+	 * up.  Even if it weren't true, IRQs are not up so we couldn't fire
+	 * IPIs around.
+	 */
+	__flush_cpu_slab(s, smp_processor_id());
 	for_each_node_state(node, N_NORMAL_MEMORY) {
 		struct kmem_cache_node *n = get_node(s, node);
 		struct page *p;
-- 
1.8.1.2

[PATCH v2] slub: correctly bootstrap boot caches

[Glauber Costa]

After we create a boot cache, we may allocate from it until it is bootstraped.
This will move the page from the partial list to the cpu slab list. If this
happens, the loop:

	list_for_each_entry(p, &n->partial, lru)

that we use to scan for all partial pages will yield nothing, and the pages
will keep pointing to the boot cpu cache, which is of course, invalid. To do
that, we should flush the cache to make sure that the cpu slab is back to the
partial list.

Signed-off-by: Glauber Costa <glommer@parallels.com>
Reported-by:  Steffen Michalke <StMichalke@web.de>
Tested-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: Christoph Lameter <cl@linux.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: Pekka Enberg <penberg@kernel.org>

 mm/slub.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)
---
 mm/slub.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/mm/slub.c b/mm/slub.c
index ba2ca53..31a494c 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -3617,6 +3617,12 @@ static struct kmem_cache * __init bootstrap(struct kmem_cache *static_cache)
 
 	memcpy(s, static_cache, kmem_cache->object_size);
 
+	/*
+	 * This runs very early, and only the boot processor is supposed to be
+	 * up.  Even if it weren't true, IRQs are not up so we couldn't fire
+	 * IPIs around.
+	 */
+	__flush_cpu_slab(s, smp_processor_id());
 	for_each_node_state(node, N_NORMAL_MEMORY) {
 		struct kmem_cache_node *n = get_node(s, node);
 		struct page *p;
-- 
1.8.1.2

[PATCH v2] slub: correctly bootstrap boot caches

[Glauber Costa]

After we create a boot cache, we may allocate from it until it is bootstraped.
This will move the page from the partial list to the cpu slab list. If this
happens, the loop:

	list_for_each_entry(p, &n->partial, lru)

that we use to scan for all partial pages will yield nothing, and the pages
will keep pointing to the boot cpu cache, which is of course, invalid. To do
that, we should flush the cache to make sure that the cpu slab is back to the
partial list.

Signed-off-by: Glauber Costa <glommer@parallels.com>
Reported-by:  Steffen Michalke <StMichalke@web.de>
Tested-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: Christoph Lameter <cl@linux.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: Pekka Enberg <penberg@kernel.org>

 mm/slub.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)
---
 mm/slub.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/mm/slub.c b/mm/slub.c
index ba2ca53..31a494c 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -3617,6 +3617,12 @@ static struct kmem_cache * __init bootstrap(struct kmem_cache *static_cache)
 
 	memcpy(s, static_cache, kmem_cache->object_size);
 
+	/*
+	 * This runs very early, and only the boot processor is supposed to be
+	 * up.  Even if it weren't true, IRQs are not up so we couldn't fire
+	 * IPIs around.
+	 */
+	__flush_cpu_slab(s, smp_processor_id());
 	for_each_node_state(node, N_NORMAL_MEMORY) {
 		struct kmem_cache_node *n = get_node(s, node);
 		struct page *p;
-- 
1.8.1.2

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

Re: [PATCH v2] slub: correctly bootstrap boot caches

[Christoph Lameter]

On Fri, 22 Feb 2013, Glauber Costa wrote:

> After we create a boot cache, we may allocate from it until it is bootstraped.
> This will move the page from the partial list to the cpu slab list. If this
> happens, the loop:

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>

Re: [PATCH v2] slub: correctly bootstrap boot caches

[Christoph Lameter]

On Fri, 22 Feb 2013, Glauber Costa wrote:

> After we create a boot cache, we may allocate from it until it is bootstraped.
> This will move the page from the partial list to the cpu slab list. If this
> happens, the loop:

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

Re: [PATCH v2] slub: correctly bootstrap boot caches

[Christoph Lameter]

An earlier fix to this is available here:

https://patchwork.kernel.org/patch/1975301/

and

https://lkml.org/lkml/2013/1/15/55

Re: [PATCH v2] slub: correctly bootstrap boot caches

[Christoph Lameter]

An earlier fix to this is available here:

https://patchwork.kernel.org/patch/1975301/

and

https://lkml.org/lkml/2013/1/15/55

Re: [PATCH v2] slub: correctly bootstrap boot caches

[Christoph Lameter]

An earlier fix to this is available here:

https://patchwork.kernel.org/patch/1975301/

and

https://lkml.org/lkml/2013/1/15/55

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

Re: [PATCH v2] slub: correctly bootstrap boot caches

[Christoph Lameter]

Argh. This one was the final version:

https://patchwork.kernel.org/patch/2009521/

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

Re: [PATCH v2] slub: correctly bootstrap boot caches

[Christoph Lameter]

Argh. This one was the final version:

https://patchwork.kernel.org/patch/2009521/

Re: [PATCH v2] slub: correctly bootstrap boot caches

[Glauber Costa]

On 02/22/2013 09:01 PM, Christoph Lameter wrote:
> Argh. This one was the final version:
> 
> https://patchwork.kernel.org/patch/2009521/
> 

It seems it would work. It is all the same to me.
Which one do you prefer?


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

Re: [PATCH v2] slub: correctly bootstrap boot caches

[Glauber Costa]

On 02/22/2013 09:01 PM, Christoph Lameter wrote:
> Argh. This one was the final version:
> 
> https://patchwork.kernel.org/patch/2009521/
> 

It seems it would work. It is all the same to me.
Which one do you prefer?

Re: [PATCH v2] slub: correctly bootstrap boot caches

[Christoph Lameter]

On Fri, 22 Feb 2013, Glauber Costa wrote:

> On 02/22/2013 09:01 PM, Christoph Lameter wrote:
> > Argh. This one was the final version:
> >
> > https://patchwork.kernel.org/patch/2009521/
> >
>
> It seems it would work. It is all the same to me.
> Which one do you prefer?

Flushing seems to be simpler and less code.

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

Re: [PATCH v2] slub: correctly bootstrap boot caches

[Christoph Lameter]

On Fri, 22 Feb 2013, Glauber Costa wrote:

> On 02/22/2013 09:01 PM, Christoph Lameter wrote:
> > Argh. This one was the final version:
> >
> > https://patchwork.kernel.org/patch/2009521/
> >
>
> It seems it would work. It is all the same to me.
> Which one do you prefer?

Flushing seems to be simpler and less code.

Re: [PATCH v2] slub: correctly bootstrap boot caches

[JoonSoo Kim]

2013/2/23 Christoph Lameter <cl@linux.com>:
> On Fri, 22 Feb 2013, Glauber Costa wrote:
>
>> On 02/22/2013 09:01 PM, Christoph Lameter wrote:
>> > Argh. This one was the final version:
>> >
>> > https://patchwork.kernel.org/patch/2009521/
>> >
>>
>> It seems it would work. It is all the same to me.
>> Which one do you prefer?
>
> Flushing seems to be simpler and less code.
>

Hello, Christoph, Glauber.

With flushing, deactivate_slab() occur and it has some overhead to
deactivate objects.
If my patch properly fix this situation, it is better to use mine
which has no overhead.

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>

Re: [PATCH v2] slub: correctly bootstrap boot caches

[JoonSoo Kim]

2013/2/23 Christoph Lameter <cl@linux.com>:
> On Fri, 22 Feb 2013, Glauber Costa wrote:
>
>> On 02/22/2013 09:01 PM, Christoph Lameter wrote:
>> > Argh. This one was the final version:
>> >
>> > https://patchwork.kernel.org/patch/2009521/
>> >
>>
>> It seems it would work. It is all the same to me.
>> Which one do you prefer?
>
> Flushing seems to be simpler and less code.
>

Hello, Christoph, Glauber.

With flushing, deactivate_slab() occur and it has some overhead to
deactivate objects.
If my patch properly fix this situation, it is better to use mine
which has no overhead.

Thanks.

Re: [PATCH v2] slub: correctly bootstrap boot caches

[Christoph Lameter]

On Sat, 23 Feb 2013, JoonSoo Kim wrote:

> With flushing, deactivate_slab() occur and it has some overhead to
> deactivate objects.
> If my patch properly fix this situation, it is better to use mine
> which has no overhead.

Well this occurs during boot and its not that performance critical.

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

Re: [PATCH v2] slub: correctly bootstrap boot caches

[Christoph Lameter]

On Sat, 23 Feb 2013, JoonSoo Kim wrote:

> With flushing, deactivate_slab() occur and it has some overhead to
> deactivate objects.
> If my patch properly fix this situation, it is better to use mine
> which has no overhead.

Well this occurs during boot and its not that performance critical.

Re: [PATCH v2] slub: correctly bootstrap boot caches

[Joonsoo Kim]

Hello, Christoph.

On Sun, Feb 24, 2013 at 12:35:22AM +0000, Christoph Lameter wrote:
> On Sat, 23 Feb 2013, JoonSoo Kim wrote:
> 
> > With flushing, deactivate_slab() occur and it has some overhead to
> > deactivate objects.
> > If my patch properly fix this situation, it is better to use mine
> > which has no overhead.
> 
> Well this occurs during boot and its not that performance critical.

Hmm...
Yes, this is not performance critical place, but why do we use
a sub-optimal solution?

And flushing is abstration for complicated logic, so I think that
my raw implemntation is better for understanding.

But, I have no objection to merge Glauber's one
if you think that is better.

Thanks.

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

Re: [PATCH v2] slub: correctly bootstrap boot caches

[Joonsoo Kim]

Hello, Christoph.

On Sun, Feb 24, 2013 at 12:35:22AM +0000, Christoph Lameter wrote:
> On Sat, 23 Feb 2013, JoonSoo Kim wrote:
> 
> > With flushing, deactivate_slab() occur and it has some overhead to
> > deactivate objects.
> > If my patch properly fix this situation, it is better to use mine
> > which has no overhead.
> 
> Well this occurs during boot and its not that performance critical.

Hmm...
Yes, this is not performance critical place, but why do we use
a sub-optimal solution?

And flushing is abstration for complicated logic, so I think that
my raw implemntation is better for understanding.

But, I have no objection to merge Glauber's one
if you think that is better.

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>

Re: [PATCH v2] slub: correctly bootstrap boot caches

[Joonsoo Kim]

Hello, Christoph.

On Sun, Feb 24, 2013 at 12:35:22AM +0000, Christoph Lameter wrote:
> On Sat, 23 Feb 2013, JoonSoo Kim wrote:
> 
> > With flushing, deactivate_slab() occur and it has some overhead to
> > deactivate objects.
> > If my patch properly fix this situation, it is better to use mine
> > which has no overhead.
> 
> Well this occurs during boot and its not that performance critical.

Hmm...
Yes, this is not performance critical place, but why do we use
a sub-optimal solution?

And flushing is abstration for complicated logic, so I think that
my raw implemntation is better for understanding.

But, I have no objection to merge Glauber's one
if you think that is better.

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>

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

Re: [PATCH v2] slub: correctly bootstrap boot caches

[Pekka Enberg]

On Fri, Feb 22, 2013 at 7:23 PM, Christoph Lameter <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org> wrote:
> On Fri, 22 Feb 2013, Glauber Costa wrote:
>
>> On 02/22/2013 09:01 PM, Christoph Lameter wrote:
>> > Argh. This one was the final version:
>> >
>> > https://patchwork.kernel.org/patch/2009521/
>> >
>>
>> It seems it would work. It is all the same to me.
>> Which one do you prefer?
>
> Flushing seems to be simpler and less code.

Well, I completely lost track of what to apply... Can someone please
send me the final version that everybody agrees on with proper ACKs
and other attributions?

Re: [PATCH v2] slub: correctly bootstrap boot caches

[Pekka Enberg]

On Fri, Feb 22, 2013 at 7:23 PM, Christoph Lameter <cl@linux.com> wrote:
> On Fri, 22 Feb 2013, Glauber Costa wrote:
>
>> On 02/22/2013 09:01 PM, Christoph Lameter wrote:
>> > Argh. This one was the final version:
>> >
>> > https://patchwork.kernel.org/patch/2009521/
>> >
>>
>> It seems it would work. It is all the same to me.
>> Which one do you prefer?
>
> Flushing seems to be simpler and less code.

Well, I completely lost track of what to apply... Can someone please
send me the final version that everybody agrees on with proper ACKs
and other attributions?

Re: [PATCH v2] slub: correctly bootstrap boot caches

[Pekka Enberg]

On Fri, Feb 22, 2013 at 7:23 PM, Christoph Lameter <cl@linux.com> wrote:
> On Fri, 22 Feb 2013, Glauber Costa wrote:
>
>> On 02/22/2013 09:01 PM, Christoph Lameter wrote:
>> > Argh. This one was the final version:
>> >
>> > https://patchwork.kernel.org/patch/2009521/
>> >
>>
>> It seems it would work. It is all the same to me.
>> Which one do you prefer?
>
> Flushing seems to be simpler and less code.

Well, I completely lost track of what to apply... Can someone please
send me the final version that everybody agrees on with proper ACKs
and other attributions?

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

Re: [PATCH v2] slub: correctly bootstrap boot caches

[Glauber Costa]

On 02/27/2013 11:47 AM, Pekka Enberg wrote:
> On Fri, Feb 22, 2013 at 7:23 PM, Christoph Lameter <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org> wrote:
>> On Fri, 22 Feb 2013, Glauber Costa wrote:
>>
>>> On 02/22/2013 09:01 PM, Christoph Lameter wrote:
>>>> Argh. This one was the final version:
>>>>
>>>> https://patchwork.kernel.org/patch/2009521/
>>>>
>>>
>>> It seems it would work. It is all the same to me.
>>> Which one do you prefer?
>>
>> Flushing seems to be simpler and less code.
> 
> Well, I completely lost track of what to apply... Can someone please
> send me the final version that everybody agrees on with proper ACKs
> and other attributions?
> 
You can apply this one as-is with Christoph's ACK.

Re: [PATCH v2] slub: correctly bootstrap boot caches

[Glauber Costa]

On 02/27/2013 11:47 AM, Pekka Enberg wrote:
> On Fri, Feb 22, 2013 at 7:23 PM, Christoph Lameter <cl@linux.com> wrote:
>> On Fri, 22 Feb 2013, Glauber Costa wrote:
>>
>>> On 02/22/2013 09:01 PM, Christoph Lameter wrote:
>>>> Argh. This one was the final version:
>>>>
>>>> https://patchwork.kernel.org/patch/2009521/
>>>>
>>>
>>> It seems it would work. It is all the same to me.
>>> Which one do you prefer?
>>
>> Flushing seems to be simpler and less code.
> 
> Well, I completely lost track of what to apply... Can someone please
> send me the final version that everybody agrees on with proper ACKs
> and other attributions?
> 
You can apply this one as-is with Christoph's ACK.

Re: [PATCH v2] slub: correctly bootstrap boot caches

[Glauber Costa]

On 02/27/2013 11:47 AM, Pekka Enberg wrote:
> On Fri, Feb 22, 2013 at 7:23 PM, Christoph Lameter <cl@linux.com> wrote:
>> On Fri, 22 Feb 2013, Glauber Costa wrote:
>>
>>> On 02/22/2013 09:01 PM, Christoph Lameter wrote:
>>>> Argh. This one was the final version:
>>>>
>>>> https://patchwork.kernel.org/patch/2009521/
>>>>
>>>
>>> It seems it would work. It is all the same to me.
>>> Which one do you prefer?
>>
>> Flushing seems to be simpler and less code.
> 
> Well, I completely lost track of what to apply... Can someone please
> send me the final version that everybody agrees on with proper ACKs
> and other attributions?
> 
You can apply this one as-is with Christoph's ACK.

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

Re: [PATCH v2] slub: correctly bootstrap boot caches

[Christoph Lameter]

On Wed, 27 Feb 2013, Glauber Costa wrote:

> You can apply this one as-is with Christoph's ACK.

Right.

Re: [PATCH v2] slub: correctly bootstrap boot caches

[Christoph Lameter]

On Wed, 27 Feb 2013, Glauber Costa wrote:

> You can apply this one as-is with Christoph's ACK.

Right.

Re: [PATCH v2] slub: correctly bootstrap boot caches

[Christoph Lameter]

On Wed, 27 Feb 2013, Glauber Costa wrote:

> You can apply this one as-is with Christoph's ACK.

Right.

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