Re: [PATCH] mm/memcg: remove useless check on page->mem_cgroup
From: Michal Hocko <hidden>
Date: 2020-08-03 08:18:18
Also in:
linux-mm, lkml
On Sat 01-08-20 11:58:41, Alex Shi wrote:
在 2020/7/31 下午11:16, Johannes Weiner 写道:quoted
quoted
if (!entry.val) { memcg_memory_event(memcg, MEMCG_SWAP_FAIL);Uncharged readahead pages are gone, but I'm not 100% sure uncharged pages in general are gone. ISTR that the !page->mem_cgroup check in mem_cgroup_uncharge() prevented a crash - although that is of course a much broader interface, whereas the ones you change should only apply to LRU pages (which are hopefully all charged). Nevertheless, to avoid unnecessary crashes if we discover that we've been wrong, how about leaving the branches for now, but adding a (new) VM_WARN_ON_ONCE_PAGE() to them?
Agreed!
Right, let's see if other unexcepted things happens, and then do actions. So it's the patch:quoted
From 28893cf8e55b98665cce58c0ba6d54aeafb63a62 Mon Sep 17 00:00:00 2001From: Alex Shi <alex.shi-KPsoFbNs7GizrGE5bRqYAgC/G2K4zDHf@public.gmane.org> Date: Sat, 1 Aug 2020 10:43:55 +0800 Subject: [PATCH] mm/memcg: warning on !memcg after readahead page charged Since readahead page is charged on memcg too, in theory we don't have to check this exception now. Before safely remove them all, add a warning for the unexpected !memcg.
I would find it useful to mention since when this assumption holds.
quoted hunk ↗ jump to hunk
Signed-off-by: Alex Shi <alex.shi-KPsoFbNs7GizrGE5bRqYAgC/G2K4zDHf@public.gmane.org> Cc: Johannes Weiner <redacted> Cc: Michal Hocko <redacted> Cc: Vladimir Davydov <redacted> Cc: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org> Cc: cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org Cc: linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org --- include/linux/mmdebug.h | 8 ++++++++ mm/memcontrol.c | 15 ++++++++------- 2 files changed, 16 insertions(+), 7 deletions(-)diff --git a/include/linux/mmdebug.h b/include/linux/mmdebug.h index 2ad72d2c8cc5..639e98a3384e 100644 --- a/include/linux/mmdebug.h +++ b/include/linux/mmdebug.h@@ -37,6 +37,13 @@ BUG(); \ } \ } while (0) +#define VM_WARN_ON_ONCE_PAGE(cond, page) \ + do { \ + if (unlikely(cond)) { \ + dump_page(page, "VM_WARN_ON_ONCE_PAGE(" __stringify(cond)")");\ + WARN_ON_ONCE(cond); \ + } \
This is a bit strange behavior. You dump page for each occasion but warn only once. I would expect either "once" semantic for any output or just dump on each occasion because if the whole point is to reduce to amount of output then the above doesn't serve the purpose. -- Michal Hocko SUSE Labs