* [PATCH] memcg: don't initialize kmem-cache destroying work for root caches
@ 2013-05-22 8:09 Andrey Vagin
0 siblings, 0 replies; 6+ messages in thread
From: Andrey Vagin @ 2013-05-22 8:09 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-kernel, linux-mm, cgroups, Andrey Vagin, stable,
Konstantin Khlebnikov, Glauber Costa, Johannes Weiner,
Balbir Singh, KAMEZAWA Hiroyuki
struct memcg_cache_params has a union. Different parts of this union are
used for root and non-root caches. A part with destroying work is used only
for non-root caches.
[ 115.096202] BUG: unable to handle kernel paging request at 0000000fffffffe0
[ 115.096785] IP: [<ffffffff8116b641>] kmem_cache_alloc+0x41/0x1f0
[ 115.097024] PGD 7ace1067 PUD 0
[ 115.097024] Oops: 0000 [#4] SMP
[ 115.097024] Modules linked in: netlink_diag af_packet_diag udp_diag tcp_diag inet_diag unix_diag ip6table_filter ip6_tables i2c_piix4 virtio_net virtio_balloon microcode i2c_core pcspkr floppy
[ 115.097024] CPU: 0 PID: 1929 Comm: lt-vzctl Tainted: G D 3.10.0-rc1+ #2
[ 115.097024] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[ 115.097024] task: ffff88007b5aaee0 ti: ffff88007bf0c000 task.ti: ffff88007bf0c000
[ 115.097024] RIP: 0010<ffffffff8116b641>] [<ffffffff8116b641>] kmem_cache_alloc+0x41/0x1f0
[ 115.097024] RSP: 0018:ffff88007bf0de68 EFLAGS: 00010202
[ 115.097024] RAX: 0000000fffffffe0 RBX: 00007fff4014f200 RCX: 0000000000000300
[ 115.097024] RDX: 0000000000000005 RSI: 00000000000000d0 RDI: ffff88007d001300
[ 115.097024] RBP: ffff88007bf0dea8 R08: 00007f849c3141b7 R09: ffffffff8118e100
[ 115.097024] R10: 0000000000000001 R11: 0000000000000246 R12: 00000000000000d0
[ 115.097024] R13: 0000000fffffffe0 R14: ffff88007d001300 R15: 0000000000001000
[ 115.097024] FS: 00007f849cbb8b40(0000) GS:ffff88007fc00000(0000) knlGS:0000000000000000
[ 115.097024] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 115.097024] CR2: 0000000fffffffe0 CR3: 000000007bc38000 CR4: 00000000000006f0
[ 115.097024] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 115.097024] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[ 115.097024] Stack:
[ 115.097024] ffffffff8118e100 ffffffff81149ea1 0000000000000008 00007fff4014f200
[ 115.097024] 00007fff4014f200 0000000000000000 0000000000000000 0000000000001000
[ 115.097024] ffff88007bf0dee8 ffffffff8118e100 ffff880037598e00 00007fff4014f200
[ 115.097024] Call Trace:
[ 115.097024] [<ffffffff8118e100>] ? getname_flags.part.34+0x30/0x140
[ 115.097024] [<ffffffff81149ea1>] ? vma_rb_erase+0x121/0x210
[ 115.097024] [<ffffffff8118e100>] getname_flags.part.34+0x30/0x140
[ 115.097024] [<ffffffff8118e248>] getname+0x38/0x60
[ 115.097024] [<ffffffff81181d55>] do_sys_open+0xc5/0x1e0
[ 115.097024] [<ffffffff81181e92>] SyS_open+0x22/0x30
[ 115.097024] [<ffffffff8161cb82>] system_call_fastpath+0x16/0x1b
[ 115.097024] Code: f4 53 48 83 ec 18 8b 05 8e 53 b7 00 4c 8b 4d 08 21 f0 a8 10 74 0d 4c 89 4d c0 e8 1b 76 4a 00 4c 8b 4d c0 e9 92 00 00 00 4d 89 f5 <4d> 8b 45 00 65 4c 03 04 25 48 cd 00 00 49 8b 50 08 4d 8b 38 49
[ 115.097024] RIP [<ffffffff8116b641>] kmem_cache_alloc+0x41/0x1f0
[ 115.097024] RSP <ffff88007bf0de68>
[ 115.097024] CR2: 0000000fffffffe0
[ 115.121352] ---[ end trace 16bb8e8408b97d0e ]---
Cc: stable@vger.kernel.org # 3.9
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Konstantin Khlebnikov <khlebnikov@openvz.org>
Cc: Glauber Costa <glommer@parallels.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Balbir Singh <bsingharora@gmail.com>
Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Reviewed-by: Michal Hocko <mhocko@suse.cz>
Signed-off-by: Andrey Vagin <avagin@openvz.org>
---
mm/memcontrol.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index cb1c9de..764b9e4 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3141,8 +3141,6 @@ int memcg_update_cache_size(struct kmem_cache *s, int num_groups)
return -ENOMEM;
}
- INIT_WORK(&s->memcg_params->destroy,
- kmem_cache_destroy_work_func);
s->memcg_params->is_root_cache = true;
/*
--
1.8.1.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] memcg: don't initialize kmem-cache destroying work for root caches
@ 2013-08-05 16:09 Andrey Vagin
2013-08-05 20:05 ` Andrew Morton
0 siblings, 1 reply; 6+ messages in thread
From: Andrey Vagin @ 2013-08-05 16:09 UTC (permalink / raw)
To: linux-mm
Cc: cgroups, linux-kernel, Andrey Vagin, Glauber Costa,
Johannes Weiner, Michal Hocko, Balbir Singh, KAMEZAWA Hiroyuki,
Andrew Morton, Konstantin Khlebnikov, stable
struct memcg_cache_params has a union. Different parts of this union
are used for root and non-root caches. A part with destroying work is
used only for non-root caches.
I fixed the same problem in another place v3.9-rc1-16204-gf101a94, but
didn't notice this one.
Cc: Glauber Costa <glommer@openvz.org>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Michal Hocko <mhocko@suse.cz>
Cc: Balbir Singh <bsingharora@gmail.com>
Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Konstantin Khlebnikov <khlebnikov@openvz.org>
Cc: <stable@vger.kernel.org> [3.9.x]
Signed-off-by: Andrey Vagin <avagin@openvz.org>
---
mm/memcontrol.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index c290a1c..c5792a5 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3195,11 +3195,11 @@ int memcg_register_cache(struct mem_cgroup *memcg, struct kmem_cache *s,
if (!s->memcg_params)
return -ENOMEM;
- INIT_WORK(&s->memcg_params->destroy,
- kmem_cache_destroy_work_func);
if (memcg) {
s->memcg_params->memcg = memcg;
s->memcg_params->root_cache = root_cache;
+ INIT_WORK(&s->memcg_params->destroy,
+ kmem_cache_destroy_work_func);
} else
s->memcg_params->is_root_cache = true;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] memcg: don't initialize kmem-cache destroying work for root caches
2013-08-05 16:09 Andrey Vagin
@ 2013-08-05 20:05 ` Andrew Morton
2013-08-05 21:01 ` Andrew Vagin
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2013-08-05 20:05 UTC (permalink / raw)
To: Andrey Vagin
Cc: linux-mm, cgroups, linux-kernel, Glauber Costa, Johannes Weiner,
Michal Hocko, Balbir Singh, KAMEZAWA Hiroyuki,
Konstantin Khlebnikov, stable
On Mon, 5 Aug 2013 20:09:40 +0400 Andrey Vagin <avagin@openvz.org> wrote:
> struct memcg_cache_params has a union. Different parts of this union
> are used for root and non-root caches. A part with destroying work is
> used only for non-root caches.
>
> I fixed the same problem in another place v3.9-rc1-16204-gf101a94, but
> didn't notice this one.
>
> Cc: <stable@vger.kernel.org> [3.9.x]
hm, why the cc:stable?
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -3195,11 +3195,11 @@ int memcg_register_cache(struct mem_cgroup *memcg, struct kmem_cache *s,
> if (!s->memcg_params)
> return -ENOMEM;
>
> - INIT_WORK(&s->memcg_params->destroy,
> - kmem_cache_destroy_work_func);
> if (memcg) {
> s->memcg_params->memcg = memcg;
> s->memcg_params->root_cache = root_cache;
> + INIT_WORK(&s->memcg_params->destroy,
> + kmem_cache_destroy_work_func);
> } else
> s->memcg_params->is_root_cache = true;
So the bug here is that we'll scribble on some entries in
memcg_caches[]. Those scribbles may or may not be within the part of
that array which is actually used. If there's code which expects
memcg_caches[] entries to be zeroed at initialisation then yes, we have
a problem.
But I rather doubt whether this bug was causing runtime problems?
Presently memcg_register_cache() allocates too much memory for the
memcg_caches[] array. If that was fixed then this INIT_WORK() might
scribble into unknown memory, which is of course serious.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] memcg: don't initialize kmem-cache destroying work for root caches
2013-08-05 20:05 ` Andrew Morton
@ 2013-08-05 21:01 ` Andrew Vagin
2013-08-05 21:16 ` Andrew Morton
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Vagin @ 2013-08-05 21:01 UTC (permalink / raw)
To: Andrew Morton
Cc: Andrey Vagin, linux-mm, cgroups, linux-kernel, Glauber Costa,
Johannes Weiner, Michal Hocko, Balbir Singh, KAMEZAWA Hiroyuki,
Konstantin Khlebnikov, stable
On Mon, Aug 05, 2013 at 01:05:30PM -0700, Andrew Morton wrote:
> On Mon, 5 Aug 2013 20:09:40 +0400 Andrey Vagin <avagin@openvz.org> wrote:
>
> > struct memcg_cache_params has a union. Different parts of this union
> > are used for root and non-root caches. A part with destroying work is
> > used only for non-root caches.
> >
> > I fixed the same problem in another place v3.9-rc1-16204-gf101a94, but
> > didn't notice this one.
> >
> > Cc: <stable@vger.kernel.org> [3.9.x]
>
> hm, why the cc:stable?
Because this patch fixes the kernel panic:
[ 46.848187] BUG: unable to handle kernel paging request at 000000fffffffeb8
[ 46.849026] IP: [<ffffffff811a484c>] kmem_cache_destroy_memcg_children+0x6c/0xc0
[ 46.849092] PGD 0
[ 46.849092] Oops: 0000 [#1] SMP
[ 46.849092] Modules linked in: vzethdev vznetdev pio_direct pfmt_raw pfmt_ploop1 ploop simfs ipt_MASQUERADE nf_conntrack_netbios_ns nf_conntrack_broadcast ip6table_mangle ip6t_REJECT nf_conntrack_ipv6 nf_defrag_ipv6 iptable_nat nf_nat_ipv4 nf_nat iptable_mangle nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack ebtable_filter ebtables ip6table_filter ip6_tables vzevent microcode joydev pcspkr virtio_balloon i2c_piix4 i2c_core virtio_net virtio_blk floppy
[ 46.849092] CPU 0
[ 46.849092] Pid: 6, comm: kworker/u:0 ve: 0 Not tainted 3.9.4+ #42 ovz.2.4 Red Hat KVM
[ 46.849092] RIP: 0010:[<ffffffff811a484c>] [<ffffffff811a484c>] kmem_cache_destroy_memcg_children+0x6c/0xc0
[ 46.849092] RSP: 0018:ffff88007c7dfcd8 EFLAGS: 00010206
[ 46.849092] RAX: ffff88007b65f180 RBX: 000000fffffffe00 RCX: 0000000000000004
[ 46.849092] RDX: 0000000000000005 RSI: ffff88007fc17cc8 RDI: ffffffff81c5e5a0
[ 46.849092] RBP: ffff88007c7dfcf8 R08: ffffea0001ee1b20 R09: 0000000000000000
[ 46.849092] R10: ffff88007fbe5fe0 R11: 0000000000000000 R12: 0000000000000005
[ 46.849092] R13: ffff88007b8c2400 R14: 0000000000000000 R15: ffff88007c008005
[ 46.849092] FS: 0000000000000000(0000) GS:ffff88007fc00000(0000) knlGS:0000000000000000
[ 46.849092] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[ 46.849092] CR2: 000000fffffffeb8 CR3: 00000000375a3000 CR4: 00000000000006f0
[ 46.849092] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 46.849092] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[ 46.849092] Process kworker/u:0 (pid: 6, ve: 0, threadinfo ffff88007c7de000, task ffff88007c7e8000)
[ 46.849092] Stack:
[ 46.849092] ffff88007bc90870 ffff88007b8c2400 ffff88007bc90000 ffff88007bc90870
[ 46.849092] ffff88007c7dfd18 ffffffff81166a14 0000000080000003 ffff88007bc90000
[ 46.849092] ffff88007c7dfd48 ffffffffa007b3c8 ffff88007c7dfd48 ffff88007bc90000
[ 46.849092] Call Trace:
[ 46.849092] [<ffffffff81166a14>] kmem_cache_destroy+0x14/0xf0
[ 46.849092] [<ffffffffa007b3c8>] nf_conntrack_cleanup_net+0xf8/0x120 [nf_conntrack]
[ 46.849092] [<ffffffffa007d221>] nf_conntrack_pernet_exit+0x41/0x50 [nf_conntrack]
[ 46.849092] [<ffffffff81536779>] ops_exit_list+0x39/0x60
[ 46.849092] [<ffffffff81536cfb>] cleanup_net+0xfb/0x200
[ 46.849092] [<ffffffff8107f43b>] process_one_work+0x17b/0x3d0
[ 46.849092] [<ffffffff81082639>] worker_thread+0x119/0x380
[ 46.849092] [<ffffffff81082520>] ? manage_workers+0x350/0x350
[ 46.849092] [<ffffffff8108796e>] kthread+0xce/0xe0
[ 46.849092] [<ffffffff810878a0>] ? kthread_freezable_should_stop+0x70/0x70
[ 46.849092] [<ffffffff8163faec>] ret_from_fork+0x7c/0xb0
[ 46.849092] [<ffffffff810878a0>] ? kthread_freezable_should_stop+0x70/0x70
[ 46.849092] Code: fb 48 00 8b 1d 6e f0 1d 01 85 db 7e 4e 45 31 e4 0f 1f 80 00 00 00 00 49 8b 85 b8 00 00 00 49 63 d4 48 8b 5c d0 08 48 85 db 74 23 <48> 8b 83 b8 00 00 00 c6 40 28 00 48 8b bb b8 00 00 00 48 83 c7
This bug was added by v3.9-rc1-221-g15cf17d
>
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
> > @@ -3195,11 +3195,11 @@ int memcg_register_cache(struct mem_cgroup *memcg, struct kmem_cache *s,
> > if (!s->memcg_params)
> > return -ENOMEM;
> >
> > - INIT_WORK(&s->memcg_params->destroy,
> > - kmem_cache_destroy_work_func);
> > if (memcg) {
> > s->memcg_params->memcg = memcg;
> > s->memcg_params->root_cache = root_cache;
> > + INIT_WORK(&s->memcg_params->destroy,
> > + kmem_cache_destroy_work_func);
> > } else
> > s->memcg_params->is_root_cache = true;
>
> So the bug here is that we'll scribble on some entries in
> memcg_caches[]. Those scribbles may or may not be within the part of
> that array which is actually used. If there's code which expects
> memcg_caches[] entries to be zeroed at initialisation then yes, we have
> a problem.
INIT_WORK() sets s->memcg_params->memcg_caches[5] to 0xfffffffe00.
Look at kmem_cache_destroy_memcg_children()
for (i = 0; i < memcg_limited_groups_array_size; i++) {
c = s->memcg_params->memcg_caches[i];
if (!c)
continue;
...
c->memcg_params->dead = false;
This code tries dereference 0xfffffffe00->dead and the kernel panics
>
> But I rather doubt whether this bug was causing runtime problems?
>
>
> Presently memcg_register_cache() allocates too much memory for the
> memcg_caches[] array. If that was fixed then this INIT_WORK() might
> scribble into unknown memory, which is of course serious.
Looks like you find another bug:
struct memcg_cache_params {
bool is_root_cache;
union {
struct kmem_cache *memcg_caches[0];
struct {
struct mem_cgroup *memcg;
struct list_head list;
struct kmem_cache *root_cache;
bool dead;
atomic_t nr_pages;
struct work_struct destroy;
};
};
};
The size of this strcture is 80 bytes, then look at memcg_register_cache()
size_t size = sizeof(struct memcg_cache_params);
if (!memcg_kmem_enabled())
return 0;
if (!memcg)
size += memcg_limited_groups_array_size * sizeof(void *);
s->memcg_params = kzalloc(size, GFP_KERNEL);
Actually it allocates too much memory. It allocates memory as if struct
memcg_cache_params would have been written without union.
Actually you already suggested to rework this code
https://lkml.org/lkml/2013/5/28/585
Thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] memcg: don't initialize kmem-cache destroying work for root caches
2013-08-05 21:01 ` Andrew Vagin
@ 2013-08-05 21:16 ` Andrew Morton
2013-08-05 21:41 ` Andrew Vagin
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2013-08-05 21:16 UTC (permalink / raw)
To: Andrew Vagin
Cc: Andrey Vagin, linux-mm, cgroups, linux-kernel, Glauber Costa,
Johannes Weiner, Michal Hocko, Balbir Singh, KAMEZAWA Hiroyuki,
Konstantin Khlebnikov, stable
On Tue, 6 Aug 2013 01:01:28 +0400 Andrew Vagin <avagin@parallels.com> wrote:
> On Mon, Aug 05, 2013 at 01:05:30PM -0700, Andrew Morton wrote:
> > On Mon, 5 Aug 2013 20:09:40 +0400 Andrey Vagin <avagin@openvz.org> wrote:
> >
> > > struct memcg_cache_params has a union. Different parts of this union
> > > are used for root and non-root caches. A part with destroying work is
> > > used only for non-root caches.
> > >
> > > I fixed the same problem in another place v3.9-rc1-16204-gf101a94, but
> > > didn't notice this one.
> > >
> > > Cc: <stable@vger.kernel.org> [3.9.x]
> >
> > hm, why the cc:stable?
>
> Because this patch fixes the kernel panic:
>
> [ 46.848187] BUG: unable to handle kernel paging request at 000000fffffffeb8
> [ 46.849026] IP: [<ffffffff811a484c>] kmem_cache_destroy_memcg_children+0x6c/0xc0
> [ 46.849092] PGD 0
> [ 46.849092] Oops: 0000 [#1] SMP
OK, pretty soon we'll have a changelog!
What does one do to trigger this oops? The bug has been there since
3.9, so the means-of-triggering must be quite special?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] memcg: don't initialize kmem-cache destroying work for root caches
2013-08-05 21:16 ` Andrew Morton
@ 2013-08-05 21:41 ` Andrew Vagin
0 siblings, 0 replies; 6+ messages in thread
From: Andrew Vagin @ 2013-08-05 21:41 UTC (permalink / raw)
To: Andrew Morton
Cc: Andrey Vagin, linux-mm, cgroups, linux-kernel, Glauber Costa,
Johannes Weiner, Michal Hocko, Balbir Singh, KAMEZAWA Hiroyuki,
Konstantin Khlebnikov, stable
On Mon, Aug 05, 2013 at 02:16:09PM -0700, Andrew Morton wrote:
> On Tue, 6 Aug 2013 01:01:28 +0400 Andrew Vagin <avagin@parallels.com> wrote:
>
> > On Mon, Aug 05, 2013 at 01:05:30PM -0700, Andrew Morton wrote:
> > > On Mon, 5 Aug 2013 20:09:40 +0400 Andrey Vagin <avagin@openvz.org> wrote:
> > >
> > > > struct memcg_cache_params has a union. Different parts of this union
> > > > are used for root and non-root caches. A part with destroying work is
> > > > used only for non-root caches.
> > > >
> > > > I fixed the same problem in another place v3.9-rc1-16204-gf101a94, but
> > > > didn't notice this one.
> > > >
> > > > Cc: <stable@vger.kernel.org> [3.9.x]
> > >
> > > hm, why the cc:stable?
> >
> > Because this patch fixes the kernel panic:
> >
> > [ 46.848187] BUG: unable to handle kernel paging request at 000000fffffffeb8
> > [ 46.849026] IP: [<ffffffff811a484c>] kmem_cache_destroy_memcg_children+0x6c/0xc0
> > [ 46.849092] PGD 0
> > [ 46.849092] Oops: 0000 [#1] SMP
>
> OK, pretty soon we'll have a changelog!
Sorry, probably I had to write all these in the initial commit message. I
just thought that this patch is an additional part of v3.9-rc1-16204-gf101a94.
>
> What does one do to trigger this oops? The bug has been there since
> 3.9, so the means-of-triggering must be quite special?
I don't think that so many people use cgroups with limits of the kernel memory.
I use the vzctl utility to operate with containers. vzctl limits the
kernel memory of containers by default. A container should be started
and stoped a few times (five or four) to reproduce the bug.
And one more thing is that nf_conntrack should be loaded. It creates
a new kmem_cache for each network namespace.
Thanks
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-08-05 21:41 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-22 8:09 [PATCH] memcg: don't initialize kmem-cache destroying work for root caches Andrey Vagin
-- strict thread matches above, loose matches on Subject: below --
2013-08-05 16:09 Andrey Vagin
2013-08-05 20:05 ` Andrew Morton
2013-08-05 21:01 ` Andrew Vagin
2013-08-05 21:16 ` Andrew Morton
2013-08-05 21:41 ` Andrew Vagin
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).