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: Konstantin Khlebnikov <[email protected]>
Cc: Glauber Costa <[email protected]>
Cc: Johannes Weiner <[email protected]>
Cc: Michal Hocko <[email protected]>
Cc: Balbir Singh <[email protected]>
Cc: KAMEZAWA Hiroyuki <[email protected]>
Signed-off-by: Andrey Vagin <[email protected]>
---
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
On Tue 14-05-13 16:38:38, Andrey Vagin 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.
but memcg_update_cache_size is called only for !root caches AFAICS
(check memcg_update_all_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: Konstantin Khlebnikov <[email protected]>
> Cc: Glauber Costa <[email protected]>
> Cc: Johannes Weiner <[email protected]>
> Cc: Michal Hocko <[email protected]>
> Cc: Balbir Singh <[email protected]>
> Cc: KAMEZAWA Hiroyuki <[email protected]>
> Signed-off-by: Andrey Vagin <[email protected]>
> ---
> 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
>
> --
> To unsubscribe from this list: send the line "unsubscribe cgroups" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Michal Hocko
SUSE Labs
On Tue 14-05-13 16:40:31, Michal Hocko wrote:
> On Tue 14-05-13 16:38:38, Andrey Vagin 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.
>
> but memcg_update_cache_size is called only for !root caches AFAICS
> (check memcg_update_all_caches)
Ohh, I am blind. memcg_update_all_caches skips all !root caches.
Then the patch looks correct. If Glauber has nothing against then thise
should be marked for stable (3.9)
--
Michal Hocko
SUSE Labs
On 05/14/2013 06:44 PM, Michal Hocko wrote:
> On Tue 14-05-13 16:40:31, Michal Hocko wrote:
>> On Tue 14-05-13 16:38:38, Andrey Vagin 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.
>>
>> but memcg_update_cache_size is called only for !root caches AFAICS
>> (check memcg_update_all_caches)
>
> Ohh, I am blind. memcg_update_all_caches skips all !root caches.
> Then the patch looks correct. If Glauber has nothing against then thise
> should be marked for stable (3.9)
>
This was recently introduced by the commit that moved the initialization
earlier (15cf17d26e08ee9). It basically moved too much, and I didn't
catch it. If that patch is in 3.9, then yes, this needs to go to stable.
Otherwise it is not affected.
However, I do remember Andrey telling me that he hit this bug in both
3.9 and 3.10-rc1, so yes, stable it is.
On Tue 14-05-13 18:49:07, Glauber Costa wrote:
> On 05/14/2013 06:44 PM, Michal Hocko wrote:
> > On Tue 14-05-13 16:40:31, Michal Hocko wrote:
> >> On Tue 14-05-13 16:38:38, Andrey Vagin 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.
> >>
> >> but memcg_update_cache_size is called only for !root caches AFAICS
> >> (check memcg_update_all_caches)
> >
> > Ohh, I am blind. memcg_update_all_caches skips all !root caches.
> > Then the patch looks correct. If Glauber has nothing against then thise
> > should be marked for stable (3.9)
> >
> This was recently introduced by the commit that moved the initialization
> earlier (15cf17d26e08ee9). It basically moved too much, and I didn't
> catch it. If that patch is in 3.9, then yes, this needs to go to stable.
> Otherwise it is not affected.
git describe tells it was merged in v3.9-rc2~7
> However, I do remember Andrey telling me that he hit this bug in both
> 3.9 and 3.10-rc1, so yes, stable it is.
--
Michal Hocko
SUSE Labs
On Tue 14-05-13 16:38:38, Andrey Vagin 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.
>
> [ 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: Konstantin Khlebnikov <[email protected]>
> Cc: Glauber Costa <[email protected]>
> Cc: Johannes Weiner <[email protected]>
> Cc: Michal Hocko <[email protected]>
> Cc: Balbir Singh <[email protected]>
> Cc: KAMEZAWA Hiroyuki <[email protected]>
> Signed-off-by: Andrey Vagin <[email protected]>
Forgot to add
Reviewed-by: Michal Hocko <[email protected]>
+
Cc: stable # 3.9
Thanks
> ---
> 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
>
> --
> To unsubscribe from this list: send the line "unsubscribe cgroups" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Michal Hocko
SUSE Labs
On Tue, May 14, 2013 at 06:08:59PM +0200, Michal Hocko wrote:
>
> Forgot to add
> Reviewed-by: Michal Hocko <[email protected]>
> +
> Cc: stable # 3.9
>
> Thanks
Who usually picks up such patches?
On 2013/5/22 15:40, Andrew Vagin wrote:
> On Tue, May 14, 2013 at 06:08:59PM +0200, Michal Hocko wrote:
>>
>> Forgot to add
>> Reviewed-by: Michal Hocko <[email protected]>
>> +
>> Cc: stable # 3.9
>>
>> Thanks
>
> Who usually picks up such patches?
The famous AKPM.
On Wed, May 22, 2013 at 03:50:24PM +0800, Li Zefan wrote:
> On 2013/5/22 15:40, Andrew Vagin wrote:
> > On Tue, May 14, 2013 at 06:08:59PM +0200, Michal Hocko wrote:
> >>
> >> Forgot to add
> >> Reviewed-by: Michal Hocko <[email protected]>
> >> +
> >> Cc: stable # 3.9
> >>
> >> Thanks
> >
> > Who usually picks up such patches?
>
> The famous AKPM.
>
Thanks.
get_maintainer.pl doesn't show Andrew in the list of recipients.
$ perl scripts/get_maintainer.pl 0001-memcg-don-t-initialize-kmem-cache-destroying-work-fo.patch
Johannes Weiner <[email protected]> (maintainer:MEMORY RESOURCE C...)
Michal Hocko <[email protected]> (maintainer:MEMORY RESOURCE C...)
Balbir Singh <[email protected]> (maintainer:MEMORY RESOURCE C...)
KAMEZAWA Hiroyuki <[email protected]> (maintainer:MEMORY RESOURCE C...)
[email protected] (open list:MEMORY RESOURCE C...)
[email protected] (open list:MEMORY RESOURCE C...)
[email protected] (open list)
On Wed 22-05-13 11:56:38, Andrew Vagin wrote:
> On Wed, May 22, 2013 at 03:50:24PM +0800, Li Zefan wrote:
> > On 2013/5/22 15:40, Andrew Vagin wrote:
> > > On Tue, May 14, 2013 at 06:08:59PM +0200, Michal Hocko wrote:
> > >>
> > >> Forgot to add
> > >> Reviewed-by: Michal Hocko <[email protected]>
> > >> +
> > >> Cc: stable # 3.9
> > >>
> > >> Thanks
> > >
> > > Who usually picks up such patches?
> >
> > The famous AKPM.
> >
>
> Thanks.
>
> get_maintainer.pl doesn't show Andrew in the list of recipients.
Yes, but most of the mm patches fly via Andrew.
> $ perl scripts/get_maintainer.pl 0001-memcg-don-t-initialize-kmem-cache-destroying-work-fo.patch
> Johannes Weiner <[email protected]> (maintainer:MEMORY RESOURCE C...)
> Michal Hocko <[email protected]> (maintainer:MEMORY RESOURCE C...)
> Balbir Singh <[email protected]> (maintainer:MEMORY RESOURCE C...)
> KAMEZAWA Hiroyuki <[email protected]> (maintainer:MEMORY RESOURCE C...)
> [email protected] (open list:MEMORY RESOURCE C...)
> [email protected] (open list:MEMORY RESOURCE C...)
> [email protected] (open list)
> --
> To unsubscribe from this list: send the line "unsubscribe cgroups" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Michal Hocko
SUSE Labs
On Tue, 14 May 2013 16:38:38 +0400 Andrey Vagin <[email protected]> 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.
That union is a bit dangerous. Perhaps it would be better to do
something like
--- a/include/linux/slab.h~a
+++ a/include/linux/slab.h
@@ -337,15 +337,17 @@ static __always_inline int kmalloc_size(
struct memcg_cache_params {
bool is_root_cache;
union {
- struct kmem_cache *memcg_caches[0];
- struct {
+ struct memcg_root_cache {
+ struct kmem_cache *caches[0];
+ } memcg_root_cache;
+ struct memcg_child_cache {
struct mem_cgroup *memcg;
struct list_head list;
struct kmem_cache *root_cache;
bool dead;
atomic_t nr_pages;
struct work_struct destroy;
- };
+ } memcg_child_cache;
};
};
And then adopt the convention of selecting either memcg_root_cache or
memcg_child_cache at the highest level then passing the more strongly
typed pointer to callees.
On 05/29/2013 04:23 AM, Andrew Morton wrote:
> On Tue, 14 May 2013 16:38:38 +0400 Andrey Vagin <[email protected]> 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.
>
> That union is a bit dangerous. Perhaps it would be better to do
> something like
>
> --- a/include/linux/slab.h~a
> +++ a/include/linux/slab.h
> @@ -337,15 +337,17 @@ static __always_inline int kmalloc_size(
> struct memcg_cache_params {
> bool is_root_cache;
> union {
> - struct kmem_cache *memcg_caches[0];
> - struct {
> + struct memcg_root_cache {
> + struct kmem_cache *caches[0];
> + } memcg_root_cache;
> + struct memcg_child_cache {
> struct mem_cgroup *memcg;
> struct list_head list;
> struct kmem_cache *root_cache;
> bool dead;
> atomic_t nr_pages;
> struct work_struct destroy;
> - };
> + } memcg_child_cache;
> };
> };
>
> And then adopt the convention of selecting either memcg_root_cache or
> memcg_child_cache at the highest level then passing the more strongly
> typed pointer to callees.
>
Since it is already creating problems, yes, I agree.
I will try to cook up something soon.
On 05/29/2013 06:48 AM, Glauber Costa wrote:
> On 05/29/2013 04:23 AM, Andrew Morton wrote:
>> On Tue, 14 May 2013 16:38:38 +0400 Andrey Vagin <[email protected]> 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.
>>
>> That union is a bit dangerous. Perhaps it would be better to do
>> something like
>>
>> --- a/include/linux/slab.h~a
>> +++ a/include/linux/slab.h
>> @@ -337,15 +337,17 @@ static __always_inline int kmalloc_size(
>> struct memcg_cache_params {
>> bool is_root_cache;
>> union {
>> - struct kmem_cache *memcg_caches[0];
>> - struct {
>> + struct memcg_root_cache {
>> + struct kmem_cache *caches[0];
>> + } memcg_root_cache;
>> + struct memcg_child_cache {
>> struct mem_cgroup *memcg;
>> struct list_head list;
>> struct kmem_cache *root_cache;
>> bool dead;
>> atomic_t nr_pages;
>> struct work_struct destroy;
>> - };
>> + } memcg_child_cache;
>> };
>> };
>>
>> And then adopt the convention of selecting either memcg_root_cache or
>> memcg_child_cache at the highest level then passing the more strongly
>> typed pointer to callees.
>>
>
> Since it is already creating problems, yes, I agree.
>
> I will try to cook up something soon.
>
There are other cleanups being requested as well. (Tejun claims that we
would be better off with locks than with barriers for the destruction
path). To avoid conflicting with the current shrinkers work - that is
very massive, and since none of those are pressing, I will try to tackle
both next week (on top of that, if possible)