Message ID | 20180508120402.3159-1-guro@fb.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Tue, May 08, 2018 at 01:04:02PM +0100, Roman Gushchin wrote: > Commit e27be240df53 ("mm: memcg: make sure memory.events is > uptodate when waking pollers") converted most of memcg event > counters to per-memcg atomics, which made them less confusing > for a user. The "oom_kill" counter remained untouched, so now > it behaves differently than other counters (including "oom"). > This adds nothing but confusion. Please, ignore this one. Version 2 is properly rebased on top of the mm tree. Thanks!
Hi Roman, Thank you for the patch! Yet something to improve: [auto build test ERROR on mmotm/master] [also build test ERROR on next-20180508] [cannot apply to v4.17-rc4] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Roman-Gushchin/mm-fix-oom_kill-event-handling/20180509-051754 base: git://git.cmpxchg.org/linux-mmotm.git master config: x86_64-randconfig-x010-201818 (attached as .config) compiler: gcc-7 (Debian 7.3.0-16) 7.3.0 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 All error/warnings (new ones prefixed by >>): In file included from include/linux/swap.h:9:0, from include/linux/suspend.h:5, from arch/x86/kernel/asm-offsets.c:13: include/linux/memcontrol.h: In function 'memcg_memory_event_mm': >> include/linux/memcontrol.h:746:10: error: implicit declaration of function 'mem_cgroup_from_task'; did you mean 'mem_cgroup_from_css'? [-Werror=implicit-function-declaration] memcg = mem_cgroup_from_task(rcu_dereference(mm->owner)); ^~~~~~~~~~~~~~~~~~~~ mem_cgroup_from_css In file included from include/linux/srcu.h:33:0, from include/linux/notifier.h:16, from include/linux/memory_hotplug.h:7, from include/linux/mmzone.h:777, from include/linux/gfp.h:6, from include/linux/slab.h:15, from include/linux/crypto.h:24, from arch/x86/kernel/asm-offsets.c:9: >> include/linux/memcontrol.h:746:49: error: 'struct mm_struct' has no member named 'owner' memcg = mem_cgroup_from_task(rcu_dereference(mm->owner)); ^ include/linux/rcupdate.h:351:10: note: in definition of macro '__rcu_dereference_check' typeof(*p) *________p1 = (typeof(*p) *__force)READ_ONCE(p); \ ^ include/linux/rcupdate.h:546:28: note: in expansion of macro 'rcu_dereference_check' #define rcu_dereference(p) rcu_dereference_check(p, 0) ^~~~~~~~~~~~~~~~~~~~~ >> include/linux/memcontrol.h:746:31: note: in expansion of macro 'rcu_dereference' memcg = mem_cgroup_from_task(rcu_dereference(mm->owner)); ^~~~~~~~~~~~~~~ >> include/linux/memcontrol.h:746:49: error: 'struct mm_struct' has no member named 'owner' memcg = mem_cgroup_from_task(rcu_dereference(mm->owner)); ^ include/linux/rcupdate.h:351:36: note: in definition of macro '__rcu_dereference_check' typeof(*p) *________p1 = (typeof(*p) *__force)READ_ONCE(p); \ ^ include/linux/rcupdate.h:546:28: note: in expansion of macro 'rcu_dereference_check' #define rcu_dereference(p) rcu_dereference_check(p, 0) ^~~~~~~~~~~~~~~~~~~~~ >> include/linux/memcontrol.h:746:31: note: in expansion of macro 'rcu_dereference' memcg = mem_cgroup_from_task(rcu_dereference(mm->owner)); ^~~~~~~~~~~~~~~ In file included from arch/x86/include/asm/atomic.h:5:0, from include/linux/atomic.h:5, from include/linux/crypto.h:20, from arch/x86/kernel/asm-offsets.c:9: >> include/linux/memcontrol.h:746:49: error: 'struct mm_struct' has no member named 'owner' memcg = mem_cgroup_from_task(rcu_dereference(mm->owner)); ^ include/linux/compiler.h:250:17: note: in definition of macro '__READ_ONCE' union { typeof(x) __val; char __c[1]; } __u; \ ^ include/linux/rcupdate.h:351:48: note: in expansion of macro 'READ_ONCE' typeof(*p) *________p1 = (typeof(*p) *__force)READ_ONCE(p); \ ^~~~~~~~~ include/linux/rcupdate.h:488:2: note: in expansion of macro '__rcu_dereference_check' __rcu_dereference_check((p), (c) || rcu_read_lock_held(), __rcu) ^~~~~~~~~~~~~~~~~~~~~~~ include/linux/rcupdate.h:546:28: note: in expansion of macro 'rcu_dereference_check' #define rcu_dereference(p) rcu_dereference_check(p, 0) ^~~~~~~~~~~~~~~~~~~~~ >> include/linux/memcontrol.h:746:31: note: in expansion of macro 'rcu_dereference' memcg = mem_cgroup_from_task(rcu_dereference(mm->owner)); ^~~~~~~~~~~~~~~ >> include/linux/memcontrol.h:746:49: error: 'struct mm_struct' has no member named 'owner' memcg = mem_cgroup_from_task(rcu_dereference(mm->owner)); ^ include/linux/compiler.h:252:22: note: in definition of macro '__READ_ONCE' __read_once_size(&(x), __u.__c, sizeof(x)); \ ^ include/linux/rcupdate.h:351:48: note: in expansion of macro 'READ_ONCE' typeof(*p) *________p1 = (typeof(*p) *__force)READ_ONCE(p); \ ^~~~~~~~~ include/linux/rcupdate.h:488:2: note: in expansion of macro '__rcu_dereference_check' __rcu_dereference_check((p), (c) || rcu_read_lock_held(), __rcu) ^~~~~~~~~~~~~~~~~~~~~~~ include/linux/rcupdate.h:546:28: note: in expansion of macro 'rcu_dereference_check' #define rcu_dereference(p) rcu_dereference_check(p, 0) ^~~~~~~~~~~~~~~~~~~~~ >> include/linux/memcontrol.h:746:31: note: in expansion of macro 'rcu_dereference' memcg = mem_cgroup_from_task(rcu_dereference(mm->owner)); ^~~~~~~~~~~~~~~ >> include/linux/memcontrol.h:746:49: error: 'struct mm_struct' has no member named 'owner' memcg = mem_cgroup_from_task(rcu_dereference(mm->owner)); ^ include/linux/compiler.h:252:42: note: in definition of macro '__READ_ONCE' __read_once_size(&(x), __u.__c, sizeof(x)); \ ^ include/linux/rcupdate.h:351:48: note: in expansion of macro 'READ_ONCE' typeof(*p) *________p1 = (typeof(*p) *__force)READ_ONCE(p); \ ^~~~~~~~~ include/linux/rcupdate.h:488:2: note: in expansion of macro '__rcu_dereference_check' __rcu_dereference_check((p), (c) || rcu_read_lock_held(), __rcu) ^~~~~~~~~~~~~~~~~~~~~~~ include/linux/rcupdate.h:546:28: note: in expansion of macro 'rcu_dereference_check' #define rcu_dereference(p) rcu_dereference_check(p, 0) ^~~~~~~~~~~~~~~~~~~~~ >> include/linux/memcontrol.h:746:31: note: in expansion of macro 'rcu_dereference' memcg = mem_cgroup_from_task(rcu_dereference(mm->owner)); ^~~~~~~~~~~~~~~ >> include/linux/memcontrol.h:746:49: error: 'struct mm_struct' has no member named 'owner' memcg = mem_cgroup_from_task(rcu_dereference(mm->owner)); ^ include/linux/compiler.h:254:30: note: in definition of macro '__READ_ONCE' __read_once_size_nocheck(&(x), __u.__c, sizeof(x)); \ ^ include/linux/rcupdate.h:351:48: note: in expansion of macro 'READ_ONCE' typeof(*p) *________p1 = (typeof(*p) *__force)READ_ONCE(p); \ ^~~~~~~~~ include/linux/rcupdate.h:488:2: note: in expansion of macro '__rcu_dereference_check' __rcu_dereference_check((p), (c) || rcu_read_lock_held(), __rcu) ^~~~~~~~~~~~~~~~~~~~~~~ include/linux/rcupdate.h:546:28: note: in expansion of macro 'rcu_dereference_check' #define rcu_dereference(p) rcu_dereference_check(p, 0) ^~~~~~~~~~~~~~~~~~~~~ >> include/linux/memcontrol.h:746:31: note: in expansion of macro 'rcu_dereference' memcg = mem_cgroup_from_task(rcu_dereference(mm->owner)); ^~~~~~~~~~~~~~~ >> include/linux/memcontrol.h:746:49: error: 'struct mm_struct' has no member named 'owner' memcg = mem_cgroup_from_task(rcu_dereference(mm->owner)); ^ include/linux/compiler.h:254:50: note: in definition of macro '__READ_ONCE' __read_once_size_nocheck(&(x), __u.__c, sizeof(x)); \ ^ include/linux/rcupdate.h:351:48: note: in expansion of macro 'READ_ONCE' typeof(*p) *________p1 = (typeof(*p) *__force)READ_ONCE(p); \ ^~~~~~~~~ include/linux/rcupdate.h:488:2: note: in expansion of macro '__rcu_dereference_check' __rcu_dereference_check((p), (c) || rcu_read_lock_held(), __rcu) ^~~~~~~~~~~~~~~~~~~~~~~ include/linux/rcupdate.h:546:28: note: in expansion of macro 'rcu_dereference_check' #define rcu_dereference(p) rcu_dereference_check(p, 0) ^~~~~~~~~~~~~~~~~~~~~ >> include/linux/memcontrol.h:746:31: note: in expansion of macro 'rcu_dereference' memcg = mem_cgroup_from_task(rcu_dereference(mm->owner)); ^~~~~~~~~~~~~~~ In file included from include/linux/srcu.h:33:0, from include/linux/notifier.h:16, from include/linux/memory_hotplug.h:7, from include/linux/mmzone.h:777, from include/linux/gfp.h:6, from include/linux/slab.h:15, from include/linux/crypto.h:24, from arch/x86/kernel/asm-offsets.c:9: >> include/linux/memcontrol.h:746:49: error: 'struct mm_struct' has no member named 'owner' memcg = mem_cgroup_from_task(rcu_dereference(mm->owner)); ^ include/linux/rcupdate.h:354:12: note: in definition of macro '__rcu_dereference_check' ((typeof(*p) __force __kernel *)(________p1)); \ ^ include/linux/rcupdate.h:546:28: note: in expansion of macro 'rcu_dereference_check' #define rcu_dereference(p) rcu_dereference_check(p, 0) ^~~~~~~~~~~~~~~~~~~~~ >> include/linux/memcontrol.h:746:31: note: in expansion of macro 'rcu_dereference' memcg = mem_cgroup_from_task(rcu_dereference(mm->owner)); ^~~~~~~~~~~~~~~ cc1: some warnings being treated as errors make[2]: *** [arch/x86/kernel/asm-offsets.s] Error 1 make[2]: Target '__build' not remade because of errors. make[1]: *** [prepare0] Error 2 make[1]: Target 'prepare' not remade because of errors. make: *** [sub-make] Error 2 vim +746 include/linux/memcontrol.h 736 737 static inline void memcg_memory_event_mm(struct mm_struct *mm, 738 enum memcg_memory_event event) 739 { 740 struct mem_cgroup *memcg; 741 742 if (mem_cgroup_disabled()) 743 return; 744 745 rcu_read_lock(); > 746 memcg = mem_cgroup_from_task(rcu_dereference(mm->owner)); 747 if (likely(memcg)) 748 memcg_memory_event(memcg, event); 749 rcu_read_unlock(); 750 } 751 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Roman, Thank you for the patch! Yet something to improve: [auto build test ERROR on mmotm/master] [also build test ERROR on next-20180508] [cannot apply to v4.17-rc4] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Roman-Gushchin/mm-fix-oom_kill-event-handling/20180509-051754 base: git://git.cmpxchg.org/linux-mmotm.git master config: i386-randconfig-s1-201818 (attached as .config) compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026 reproduce: # save the attached .config to linux build tree make ARCH=i386 All errors (new ones prefixed by >>): In file included from include/linux/swap.h:9:0, from include/linux/suspend.h:5, from arch/x86/kernel/asm-offsets.c:13: include/linux/memcontrol.h: In function 'memcg_memory_event_mm': >> include/linux/memcontrol.h:746:10: error: implicit declaration of function 'mem_cgroup_from_task' [-Werror=implicit-function-declaration] memcg = mem_cgroup_from_task(rcu_dereference(mm->owner)); ^~~~~~~~~~~~~~~~~~~~ In file included from include/linux/srcu.h:33:0, from include/linux/notifier.h:16, from include/linux/memory_hotplug.h:7, from include/linux/mmzone.h:777, from include/linux/gfp.h:6, from include/linux/slab.h:15, from include/linux/crypto.h:24, from arch/x86/kernel/asm-offsets.c:9: include/linux/memcontrol.h:746:49: error: 'struct mm_struct' has no member named 'owner' memcg = mem_cgroup_from_task(rcu_dereference(mm->owner)); ^ include/linux/rcupdate.h:351:10: note: in definition of macro '__rcu_dereference_check' typeof(*p) *________p1 = (typeof(*p) *__force)READ_ONCE(p); \ ^ include/linux/rcupdate.h:546:28: note: in expansion of macro 'rcu_dereference_check' #define rcu_dereference(p) rcu_dereference_check(p, 0) ^~~~~~~~~~~~~~~~~~~~~ include/linux/memcontrol.h:746:31: note: in expansion of macro 'rcu_dereference' memcg = mem_cgroup_from_task(rcu_dereference(mm->owner)); ^~~~~~~~~~~~~~~ include/linux/memcontrol.h:746:49: error: 'struct mm_struct' has no member named 'owner' memcg = mem_cgroup_from_task(rcu_dereference(mm->owner)); ^ include/linux/rcupdate.h:351:36: note: in definition of macro '__rcu_dereference_check' typeof(*p) *________p1 = (typeof(*p) *__force)READ_ONCE(p); \ ^ include/linux/rcupdate.h:546:28: note: in expansion of macro 'rcu_dereference_check' #define rcu_dereference(p) rcu_dereference_check(p, 0) ^~~~~~~~~~~~~~~~~~~~~ include/linux/memcontrol.h:746:31: note: in expansion of macro 'rcu_dereference' memcg = mem_cgroup_from_task(rcu_dereference(mm->owner)); ^~~~~~~~~~~~~~~ In file included from arch/x86/include/asm/atomic.h:5:0, from include/linux/atomic.h:5, from include/linux/crypto.h:20, from arch/x86/kernel/asm-offsets.c:9: include/linux/memcontrol.h:746:49: error: 'struct mm_struct' has no member named 'owner' memcg = mem_cgroup_from_task(rcu_dereference(mm->owner)); ^ include/linux/compiler.h:250:17: note: in definition of macro '__READ_ONCE' union { typeof(x) __val; char __c[1]; } __u; \ ^ include/linux/rcupdate.h:351:48: note: in expansion of macro 'READ_ONCE' typeof(*p) *________p1 = (typeof(*p) *__force)READ_ONCE(p); \ ^~~~~~~~~ include/linux/rcupdate.h:488:2: note: in expansion of macro '__rcu_dereference_check' __rcu_dereference_check((p), (c) || rcu_read_lock_held(), __rcu) ^~~~~~~~~~~~~~~~~~~~~~~ include/linux/rcupdate.h:546:28: note: in expansion of macro 'rcu_dereference_check' #define rcu_dereference(p) rcu_dereference_check(p, 0) ^~~~~~~~~~~~~~~~~~~~~ include/linux/memcontrol.h:746:31: note: in expansion of macro 'rcu_dereference' memcg = mem_cgroup_from_task(rcu_dereference(mm->owner)); ^~~~~~~~~~~~~~~ include/linux/memcontrol.h:746:49: error: 'struct mm_struct' has no member named 'owner' memcg = mem_cgroup_from_task(rcu_dereference(mm->owner)); ^ include/linux/compiler.h:252:22: note: in definition of macro '__READ_ONCE' __read_once_size(&(x), __u.__c, sizeof(x)); \ ^ include/linux/rcupdate.h:351:48: note: in expansion of macro 'READ_ONCE' typeof(*p) *________p1 = (typeof(*p) *__force)READ_ONCE(p); \ ^~~~~~~~~ include/linux/rcupdate.h:488:2: note: in expansion of macro '__rcu_dereference_check' __rcu_dereference_check((p), (c) || rcu_read_lock_held(), __rcu) ^~~~~~~~~~~~~~~~~~~~~~~ include/linux/rcupdate.h:546:28: note: in expansion of macro 'rcu_dereference_check' #define rcu_dereference(p) rcu_dereference_check(p, 0) ^~~~~~~~~~~~~~~~~~~~~ include/linux/memcontrol.h:746:31: note: in expansion of macro 'rcu_dereference' memcg = mem_cgroup_from_task(rcu_dereference(mm->owner)); ^~~~~~~~~~~~~~~ include/linux/memcontrol.h:746:49: error: 'struct mm_struct' has no member named 'owner' memcg = mem_cgroup_from_task(rcu_dereference(mm->owner)); ^ include/linux/compiler.h:252:42: note: in definition of macro '__READ_ONCE' __read_once_size(&(x), __u.__c, sizeof(x)); \ ^ include/linux/rcupdate.h:351:48: note: in expansion of macro 'READ_ONCE' typeof(*p) *________p1 = (typeof(*p) *__force)READ_ONCE(p); \ ^~~~~~~~~ include/linux/rcupdate.h:488:2: note: in expansion of macro '__rcu_dereference_check' __rcu_dereference_check((p), (c) || rcu_read_lock_held(), __rcu) ^~~~~~~~~~~~~~~~~~~~~~~ include/linux/rcupdate.h:546:28: note: in expansion of macro 'rcu_dereference_check' #define rcu_dereference(p) rcu_dereference_check(p, 0) ^~~~~~~~~~~~~~~~~~~~~ include/linux/memcontrol.h:746:31: note: in expansion of macro 'rcu_dereference' memcg = mem_cgroup_from_task(rcu_dereference(mm->owner)); ^~~~~~~~~~~~~~~ include/linux/memcontrol.h:746:49: error: 'struct mm_struct' has no member named 'owner' memcg = mem_cgroup_from_task(rcu_dereference(mm->owner)); ^ include/linux/compiler.h:254:30: note: in definition of macro '__READ_ONCE' __read_once_size_nocheck(&(x), __u.__c, sizeof(x)); \ ^ include/linux/rcupdate.h:351:48: note: in expansion of macro 'READ_ONCE' typeof(*p) *________p1 = (typeof(*p) *__force)READ_ONCE(p); \ vim +/mem_cgroup_from_task +746 include/linux/memcontrol.h 736 737 static inline void memcg_memory_event_mm(struct mm_struct *mm, 738 enum memcg_memory_event event) 739 { 740 struct mem_cgroup *memcg; 741 742 if (mem_cgroup_disabled()) 743 return; 744 745 rcu_read_lock(); > 746 memcg = mem_cgroup_from_task(rcu_dereference(mm->owner)); 747 if (likely(memcg)) 748 memcg_memory_event(memcg, event); 749 rcu_read_unlock(); 750 } 751 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h index 6cbea2f25a87..caa8b70a85f6 100644 --- a/include/linux/memcontrol.h +++ b/include/linux/memcontrol.h @@ -54,6 +54,7 @@ enum memcg_memory_event { MEMCG_HIGH, MEMCG_MAX, MEMCG_OOM, + MEMCG_OOM_KILL, MEMCG_SWAP_MAX, MEMCG_SWAP_FAIL, MEMCG_NR_MEMORY_EVENTS, @@ -721,11 +722,8 @@ static inline void count_memcg_event_mm(struct mm_struct *mm, rcu_read_lock(); memcg = rcu_dereference(mm->memcg); - if (likely(memcg)) { + if (likely(memcg)) count_memcg_events(memcg, idx, 1); - if (idx == OOM_KILL) - cgroup_file_notify(&memcg->events_file); - } rcu_read_unlock(); } @@ -736,6 +734,21 @@ static inline void memcg_memory_event(struct mem_cgroup *memcg, cgroup_file_notify(&memcg->events_file); } +static inline void memcg_memory_event_mm(struct mm_struct *mm, + enum memcg_memory_event event) +{ + struct mem_cgroup *memcg; + + if (mem_cgroup_disabled()) + return; + + rcu_read_lock(); + memcg = mem_cgroup_from_task(rcu_dereference(mm->owner)); + if (likely(memcg)) + memcg_memory_event(memcg, event); + rcu_read_unlock(); +} + #ifdef CONFIG_TRANSPARENT_HUGEPAGE void mem_cgroup_split_huge_fixup(struct page *head); #endif @@ -757,6 +770,11 @@ static inline void memcg_memory_event(struct mem_cgroup *memcg, { } +static inline void memcg_memory_event_mm(struct mm_struct *mm, + enum memcg_memory_event event) +{ +} + static inline bool mem_cgroup_low(struct mem_cgroup *root, struct mem_cgroup *memcg) { diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 10973671e562..38717630305d 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -3772,7 +3772,8 @@ static int mem_cgroup_oom_control_read(struct seq_file *sf, void *v) seq_printf(sf, "oom_kill_disable %d\n", memcg->oom_kill_disable); seq_printf(sf, "under_oom %d\n", (bool)memcg->under_oom); - seq_printf(sf, "oom_kill %lu\n", memcg_sum_events(memcg, OOM_KILL)); + seq_printf(sf, "oom_kill %lu\n", + atomic_long_read(&memcg->memory_events[MEMCG_OOM_KILL])); return 0; } @@ -5529,7 +5530,8 @@ static int memory_events_show(struct seq_file *m, void *v) atomic_long_read(&memcg->memory_events[MEMCG_MAX])); seq_printf(m, "oom %lu\n", atomic_long_read(&memcg->memory_events[MEMCG_OOM])); - seq_printf(m, "oom_kill %lu\n", memcg_sum_events(memcg, OOM_KILL)); + seq_printf(m, "oom_kill %lu\n", + atomic_long_read(&memcg->memory_events[MEMCG_OOM_KILL])); return 0; } diff --git a/mm/oom_kill.c b/mm/oom_kill.c index 8f7d8dd99e5d..6b74142a1259 100644 --- a/mm/oom_kill.c +++ b/mm/oom_kill.c @@ -868,7 +868,7 @@ static void __oom_kill_process(struct task_struct *victim) /* Raise event before sending signal: task reaper must see this */ count_vm_event(OOM_KILL); - count_memcg_event_mm(mm, OOM_KILL); + memcg_memory_event_mm(mm, MEMCG_OOM_KILL); /* * We should send SIGKILL before granting access to memory reserves
Commit e27be240df53 ("mm: memcg: make sure memory.events is uptodate when waking pollers") converted most of memcg event counters to per-memcg atomics, which made them less confusing for a user. The "oom_kill" counter remained untouched, so now it behaves differently than other counters (including "oom"). This adds nothing but confusion. Let's fix this by adding the MEMCG_OOM_KILL event, and follow the MEMCG_OOM approach. This also removes a hack from count_memcg_event_mm(), introduced earlier specially for the OOM_KILL counter. Signed-off-by: Roman Gushchin <guro@fb.com> Cc: Johannes Weiner <hannes@cmpxchg.org> Cc: Michal Hocko <mhocko@kernel.org> Cc: Vladimir Davydov <vdavydov.dev@gmail.com> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Konstantin Khlebnikov <khlebnikov@yandex-team.ru> Cc: linux-kernel@vger.kernel.org Cc: cgroups@vger.kernel.org Cc: linux-mm@kvack.org --- include/linux/memcontrol.h | 26 ++++++++++++++++++++++---- mm/memcontrol.c | 6 ++++-- mm/oom_kill.c | 2 +- 3 files changed, 27 insertions(+), 7 deletions(-)