Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1516616 > unrolled thread

[PATCH v3 1/2] memcg: Prevent memcg caches to be both OFF_SLAB & OBJFREELIST_SLAB

Back to article view | Back to linux.kernel

Contents

  [PATCH v3 1/2] memcg: Prevent memcg caches to be both OFF_SLAB & OBJFREELIST_SLAB Thomas Garnier <thgarnie@google.com> - 2016-11-07 22:50 +0100
    Re: [PATCH v3 1/2] memcg: Prevent memcg caches to be both OFF_SLAB  & OBJFREELIST_SLAB Andrew Morton <akpm@linux-foundation.org> - 2016-11-07 23:40 +0100
      Re: [PATCH v3 1/2] memcg: Prevent memcg caches to be both OFF_SLAB & OBJFREELIST_SLAB Thomas Garnier <thgarnie@google.com> - 2016-11-07 23:40 +0100
        Re: [PATCH v3 1/2] memcg: Prevent memcg caches to be both OFF_SLAB  & OBJFREELIST_SLAB Andrew Morton <akpm@linux-foundation.org> - 2016-11-07 23:50 +0100
          Re: [PATCH v3 1/2] memcg: Prevent memcg caches to be both OFF_SLAB & OBJFREELIST_SLAB Thomas Garnier <thgarnie@google.com> - 2016-11-08 05:30 +0100
          Re: [PATCH v3 1/2] memcg: Prevent memcg caches to be both OFF_SLAB  & OBJFREELIST_SLAB Christoph Lameter <cl@linux.com> - 2016-11-09 01:00 +0100

#1516616 — [PATCH v3 1/2] memcg: Prevent memcg caches to be both OFF_SLAB & OBJFREELIST_SLAB

From: Greg Thelen <gthelen@google.com>

While testing OBJFREELIST_SLAB integration with pagealloc, we found a
bug where kmem_cache(sys) would be created with both CFLGS_OFF_SLAB &
CFLGS_OBJFREELIST_SLAB.

The original kmem_cache is created early making OFF_SLAB not possible.
When kmem_cache(sys) is created, OFF_SLAB is possible and if pagealloc
is enabled it will try to enable it first under certain conditions.
Given kmem_cache(sys) reuses the original flag, you can have both flags
at the same time resulting in allocation failures and odd behaviors.

This fix discards allocator specific flags from memcg before calling
create_cache.

Fixes: b03a017bebc4 ("mm/slab: introduce new slab management type, OBJFREELIST_SLAB")
Signed-off-by: Greg Thelen <gthelen@google.com>
Tested-by: Thomas Garnier <thgarnie@google.com>
---
Based on next-20161027
---
 mm/slab_common.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/slab_common.c b/mm/slab_common.c
index 71f0b28..329b038 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -533,8 +533,8 @@ void memcg_create_kmem_cache(struct mem_cgroup *memcg,
 
 	s = create_cache(cache_name, root_cache->object_size,
 			 root_cache->size, root_cache->align,
-			 root_cache->flags, root_cache->ctor,
-			 memcg, root_cache);
+			 root_cache->flags & CACHE_CREATE_MASK,
+			 root_cache->ctor, memcg, root_cache);
 	/*
 	 * If we could not create a memcg cache, do not complain, because
 	 * that's not critical at all as we can always proceed with the root
-- 
2.8.0.rc3.226.g39d4020

[toc] | [next] | [standalone]

#1516633 — Re: [PATCH v3 1/2] memcg: Prevent memcg caches to be both OFF_SLAB & OBJFREELIST_SLAB

On Mon,  7 Nov 2016 13:11:14 -0800 Thomas Garnier <thgarnie@google.com> wrote:

> From: Greg Thelen <gthelen@google.com>
> 
> While testing OBJFREELIST_SLAB integration with pagealloc, we found a
> bug where kmem_cache(sys) would be created with both CFLGS_OFF_SLAB &
> CFLGS_OBJFREELIST_SLAB.
> 
> The original kmem_cache is created early making OFF_SLAB not possible.
> When kmem_cache(sys) is created, OFF_SLAB is possible and if pagealloc
> is enabled it will try to enable it first under certain conditions.
> Given kmem_cache(sys) reuses the original flag, you can have both flags
> at the same time resulting in allocation failures and odd behaviors.

Can we please have a better description of the problems which this bug
causes?  Without this info it's unclear to me which kernel version(s)
need the fix.

Given that the bug is 6 months old I'm assuming "not very urgent".

> This fix discards allocator specific flags from memcg before calling
> create_cache.
> 
> Fixes: b03a017bebc4 ("mm/slab: introduce new slab management type, OBJFREELIST_SLAB")
> Signed-off-by: Greg Thelen <gthelen@google.com>
> Tested-by: Thomas Garnier <thgarnie@google.com>

This should have had your signed-off-by, as you were on the delivery
path.  I've made that change.

[toc] | [prev] | [next] | [standalone]

#1516640

On Mon, Nov 7, 2016 at 2:19 PM, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Mon,  7 Nov 2016 13:11:14 -0800 Thomas Garnier <thgarnie@google.com> wrote:
>
>> From: Greg Thelen <gthelen@google.com>
>>
>> While testing OBJFREELIST_SLAB integration with pagealloc, we found a
>> bug where kmem_cache(sys) would be created with both CFLGS_OFF_SLAB &
>> CFLGS_OBJFREELIST_SLAB.
>>
>> The original kmem_cache is created early making OFF_SLAB not possible.
>> When kmem_cache(sys) is created, OFF_SLAB is possible and if pagealloc
>> is enabled it will try to enable it first under certain conditions.
>> Given kmem_cache(sys) reuses the original flag, you can have both flags
>> at the same time resulting in allocation failures and odd behaviors.
>
> Can we please have a better description of the problems which this bug
> causes?  Without this info it's unclear to me which kernel version(s)
> need the fix.
>
> Given that the bug is 6 months old I'm assuming "not very urgent".
>

I will add more details and send another round.

>> This fix discards allocator specific flags from memcg before calling
>> create_cache.
>>
>> Fixes: b03a017bebc4 ("mm/slab: introduce new slab management type, OBJFREELIST_SLAB")
>> Signed-off-by: Greg Thelen <gthelen@google.com>
>> Tested-by: Thomas Garnier <thgarnie@google.com>
>
> This should have had your signed-off-by, as you were on the delivery
> path.  I've made that change.

Thanks Andrew.


-- 
Thomas

[toc] | [prev] | [next] | [standalone]

#1516643 — Re: [PATCH v3 1/2] memcg: Prevent memcg caches to be both OFF_SLAB & OBJFREELIST_SLAB

On Mon, 7 Nov 2016 14:32:56 -0800 Thomas Garnier <thgarnie@google.com> wrote:

> On Mon, Nov 7, 2016 at 2:19 PM, Andrew Morton <akpm@linux-foundation.org> wrote:
> > On Mon,  7 Nov 2016 13:11:14 -0800 Thomas Garnier <thgarnie@google.com> wrote:
> >
> >> From: Greg Thelen <gthelen@google.com>
> >>
> >> While testing OBJFREELIST_SLAB integration with pagealloc, we found a
> >> bug where kmem_cache(sys) would be created with both CFLGS_OFF_SLAB &
> >> CFLGS_OBJFREELIST_SLAB.
> >>
> >> The original kmem_cache is created early making OFF_SLAB not possible.
> >> When kmem_cache(sys) is created, OFF_SLAB is possible and if pagealloc
> >> is enabled it will try to enable it first under certain conditions.
> >> Given kmem_cache(sys) reuses the original flag, you can have both flags
> >> at the same time resulting in allocation failures and odd behaviors.
> >
> > Can we please have a better description of the problems which this bug
> > causes?  Without this info it's unclear to me which kernel version(s)
> > need the fix.
> >
> > Given that the bug is 6 months old I'm assuming "not very urgent".
> >
> 
> I will add more details and send another round.

Please simply send the additional changelog text in this thread -
processing an entire v4 patch just for a changelog fiddle is rather
heavyweight.

[toc] | [prev] | [next] | [standalone]

#1516772

On Mon, Nov 7, 2016 at 2:49 PM, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Mon, 7 Nov 2016 14:32:56 -0800 Thomas Garnier <thgarnie@google.com> wrote:
>
>> On Mon, Nov 7, 2016 at 2:19 PM, Andrew Morton <akpm@linux-foundation.org> wrote:
>> > On Mon,  7 Nov 2016 13:11:14 -0800 Thomas Garnier <thgarnie@google.com> wrote:
>> >
>> >> From: Greg Thelen <gthelen@google.com>
>> >>
>> >> While testing OBJFREELIST_SLAB integration with pagealloc, we found a
>> >> bug where kmem_cache(sys) would be created with both CFLGS_OFF_SLAB &
>> >> CFLGS_OBJFREELIST_SLAB.
>> >>
>> >> The original kmem_cache is created early making OFF_SLAB not possible.
>> >> When kmem_cache(sys) is created, OFF_SLAB is possible and if pagealloc
>> >> is enabled it will try to enable it first under certain conditions.
>> >> Given kmem_cache(sys) reuses the original flag, you can have both flags
>> >> at the same time resulting in allocation failures and odd behaviors.
>> >
>> > Can we please have a better description of the problems which this bug
>> > causes?  Without this info it's unclear to me which kernel version(s)
>> > need the fix.
>> >
>> > Given that the bug is 6 months old I'm assuming "not very urgent".
>> >
>>
>> I will add more details and send another round.
>
> Please simply send the additional changelog text in this thread -
> processing an entire v4 patch just for a changelog fiddle is rather
> heavyweight.
>

Got it, here is the diff of the previous commit message:

9,10c9
< CFLGS_OBJFREELIST_SLAB. When it happened, critical allocations needed
< for loading drivers or creating new caches will fail.
---
> CFLGS_OBJFREELIST_SLAB.
16c15
< at the same time.
---
> at the same time resulting in allocation failures and odd behaviors.
21,23d19
< The bug exists since 4.6-rc1 and affects testing debug pagealloc
< configurations.
<
26d21
< Signed-off-by: Thomas Garnier <thgarnie@google.com>

-- 
Thomas

[toc] | [prev] | [next] | [standalone]

#1517657 — Re: [PATCH v3 1/2] memcg: Prevent memcg caches to be both OFF_SLAB & OBJFREELIST_SLAB

On Mon, 7 Nov 2016, Andrew Morton wrote:

> > I will add more details and send another round.
>
> Please simply send the additional changelog text in this thread -
> processing an entire v4 patch just for a changelog fiddle is rather
> heavyweight.

I think this patch is good for future cleanup. We have had a case here
where an internal flag was passed to kmem_cache_create that caused issues
later. This should not happen. We need to guard against this in the
future.

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web