From: Michal Hocko <mhocko@suse.com> To: Alex Shi <alex.shi@linux.alibaba.com> Cc: Johannes Weiner <hannes@cmpxchg.org>, Vladimir Davydov <vdavydov.dev@gmail.com>, Andrew Morton <akpm@linux-foundation.org>, cgroups@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] mm/memcg: remove useless check on page->mem_cgroup Date: Mon, 10 Aug 2020 11:55:00 +0200 [thread overview] Message-ID: <20200810095500.GE4773@dhcp22.suse.cz> (raw) In-Reply-To: <92dd8e68-8095-72c5-0144-2a084e4d5993@linux.alibaba.com> On Wed 05-08-20 20:28:33, Alex Shi wrote: [...] > >From 2ca3e87fd3878ab729551682ad083a70f15bb3fc Mon Sep 17 00:00:00 2001 > From: Alex Shi <alex.shi@linux.alibaba.com> > Date: Sat, 1 Aug 2020 10:43:55 +0800 > Subject: [PATCH v3] 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. > > Signed-off-by: Alex Shi <alex.shi@linux.alibaba.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: cgroups@vger.kernel.org > Cc: linux-mm@kvack.org > Cc: linux-kernel@vger.kernel.org Looks good to me. I am not familiar with the section tweaks but that should be ok. Acked-by: Michal Hocko <mhocko@suse.com> Once you collect more feedback, please send both patches so that they do not get lost in this thread. Thanks! > --- > include/linux/mmdebug.h | 13 +++++++++++++ > mm/memcontrol.c | 15 ++++++++------- > 2 files changed, 21 insertions(+), 7 deletions(-) > > diff --git a/include/linux/mmdebug.h b/include/linux/mmdebug.h > index 2ad72d2c8cc5..4ed52879ce55 100644 > --- a/include/linux/mmdebug.h > +++ b/include/linux/mmdebug.h > @@ -37,6 +37,18 @@ > BUG(); \ > } \ > } while (0) > +#define VM_WARN_ON_ONCE_PAGE(cond, page) ({ \ > + static bool __section(.data.once) __warned; \ > + int __ret_warn_once = !!(cond); \ > + \ > + if (unlikely(__ret_warn_once && !__warned)) { \ > + dump_page(page, "VM_WARN_ON_ONCE_PAGE(" __stringify(cond)")");\ > + __warned = true; \ > + WARN_ON(1); \ > + } \ > + unlikely(__ret_warn_once); \ > +}) > + > #define VM_WARN_ON(cond) (void)WARN_ON(cond) > #define VM_WARN_ON_ONCE(cond) (void)WARN_ON_ONCE(cond) > #define VM_WARN_ONCE(cond, format...) (void)WARN_ONCE(cond, format) > @@ -48,6 +60,7 @@ > #define VM_BUG_ON_MM(cond, mm) VM_BUG_ON(cond) > #define VM_WARN_ON(cond) BUILD_BUG_ON_INVALID(cond) > #define VM_WARN_ON_ONCE(cond) BUILD_BUG_ON_INVALID(cond) > +#define VM_WARN_ON_ONCE_PAGE(cond, page) BUILD_BUG_ON_INVALID(cond) > #define VM_WARN_ONCE(cond, format...) BUILD_BUG_ON_INVALID(cond) > #define VM_WARN(cond, format...) BUILD_BUG_ON_INVALID(cond) > #endif > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 130093bdf74b..299382fc55a9 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -1322,10 +1322,8 @@ struct lruvec *mem_cgroup_page_lruvec(struct page *page, struct pglist_data *pgd > } > > memcg = page->mem_cgroup; > - /* > - * Swapcache readahead pages are added to the LRU - and > - * possibly migrated - before they are charged. > - */ > + /* Readahead page is charged too, to see if other page uncharged */ > + VM_WARN_ON_ONCE_PAGE(!memcg, page); > if (!memcg) > memcg = root_mem_cgroup; > > @@ -6906,8 +6904,9 @@ void mem_cgroup_migrate(struct page *oldpage, struct page *newpage) > if (newpage->mem_cgroup) > return; > > - /* Swapcache readahead pages can get replaced before being charged */ > memcg = oldpage->mem_cgroup; > + /* Readahead page is charged too, to see if other page uncharged */ > + VM_WARN_ON_ONCE_PAGE(!memcg, oldpage); > if (!memcg) > return; > > @@ -7104,7 +7103,8 @@ void mem_cgroup_swapout(struct page *page, swp_entry_t entry) > > memcg = page->mem_cgroup; > > - /* Readahead page, never charged */ > + /* Readahead page is charged too, to see if other page uncharged */ > + VM_WARN_ON_ONCE_PAGE(!memcg, page); > if (!memcg) > return; > > @@ -7168,7 +7168,8 @@ int mem_cgroup_try_charge_swap(struct page *page, swp_entry_t entry) > > memcg = page->mem_cgroup; > > - /* Readahead page, never charged */ > + /* Readahead page is charged too, to see if other page uncharged */ > + VM_WARN_ON_ONCE_PAGE(!memcg, page); > if (!memcg) > return 0; > > -- > 1.8.3.1 -- Michal Hocko SUSE Labs
WARNING: multiple messages have this Message-ID (diff)
From: Michal Hocko <mhocko-IBi9RG/b67k@public.gmane.org> To: Alex Shi <alex.shi-KPsoFbNs7GizrGE5bRqYAgC/G2K4zDHf@public.gmane.org> Cc: Johannes Weiner <hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org>, Vladimir Davydov <vdavydov.dev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>, cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org Subject: Re: [PATCH] mm/memcg: remove useless check on page->mem_cgroup Date: Mon, 10 Aug 2020 11:55:00 +0200 [thread overview] Message-ID: <20200810095500.GE4773@dhcp22.suse.cz> (raw) In-Reply-To: <92dd8e68-8095-72c5-0144-2a084e4d5993-KPsoFbNs7GizrGE5bRqYAgC/G2K4zDHf@public.gmane.org> On Wed 05-08-20 20:28:33, Alex Shi wrote: [...] > >From 2ca3e87fd3878ab729551682ad083a70f15bb3fc Mon Sep 17 00:00:00 2001 > From: Alex Shi <alex.shi-KPsoFbNs7GizrGE5bRqYAgC/G2K4zDHf@public.gmane.org> > Date: Sat, 1 Aug 2020 10:43:55 +0800 > Subject: [PATCH v3] 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. > > Signed-off-by: Alex Shi <alex.shi-KPsoFbNs7GizrGE5bRqYAgC/G2K4zDHf@public.gmane.org> > Cc: Johannes Weiner <hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org> > Cc: Michal Hocko <mhocko-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> > Cc: Vladimir Davydov <vdavydov.dev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > 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 Looks good to me. I am not familiar with the section tweaks but that should be ok. Acked-by: Michal Hocko <mhocko-IBi9RG/b67k@public.gmane.org> Once you collect more feedback, please send both patches so that they do not get lost in this thread. Thanks! > --- > include/linux/mmdebug.h | 13 +++++++++++++ > mm/memcontrol.c | 15 ++++++++------- > 2 files changed, 21 insertions(+), 7 deletions(-) > > diff --git a/include/linux/mmdebug.h b/include/linux/mmdebug.h > index 2ad72d2c8cc5..4ed52879ce55 100644 > --- a/include/linux/mmdebug.h > +++ b/include/linux/mmdebug.h > @@ -37,6 +37,18 @@ > BUG(); \ > } \ > } while (0) > +#define VM_WARN_ON_ONCE_PAGE(cond, page) ({ \ > + static bool __section(.data.once) __warned; \ > + int __ret_warn_once = !!(cond); \ > + \ > + if (unlikely(__ret_warn_once && !__warned)) { \ > + dump_page(page, "VM_WARN_ON_ONCE_PAGE(" __stringify(cond)")");\ > + __warned = true; \ > + WARN_ON(1); \ > + } \ > + unlikely(__ret_warn_once); \ > +}) > + > #define VM_WARN_ON(cond) (void)WARN_ON(cond) > #define VM_WARN_ON_ONCE(cond) (void)WARN_ON_ONCE(cond) > #define VM_WARN_ONCE(cond, format...) (void)WARN_ONCE(cond, format) > @@ -48,6 +60,7 @@ > #define VM_BUG_ON_MM(cond, mm) VM_BUG_ON(cond) > #define VM_WARN_ON(cond) BUILD_BUG_ON_INVALID(cond) > #define VM_WARN_ON_ONCE(cond) BUILD_BUG_ON_INVALID(cond) > +#define VM_WARN_ON_ONCE_PAGE(cond, page) BUILD_BUG_ON_INVALID(cond) > #define VM_WARN_ONCE(cond, format...) BUILD_BUG_ON_INVALID(cond) > #define VM_WARN(cond, format...) BUILD_BUG_ON_INVALID(cond) > #endif > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 130093bdf74b..299382fc55a9 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -1322,10 +1322,8 @@ struct lruvec *mem_cgroup_page_lruvec(struct page *page, struct pglist_data *pgd > } > > memcg = page->mem_cgroup; > - /* > - * Swapcache readahead pages are added to the LRU - and > - * possibly migrated - before they are charged. > - */ > + /* Readahead page is charged too, to see if other page uncharged */ > + VM_WARN_ON_ONCE_PAGE(!memcg, page); > if (!memcg) > memcg = root_mem_cgroup; > > @@ -6906,8 +6904,9 @@ void mem_cgroup_migrate(struct page *oldpage, struct page *newpage) > if (newpage->mem_cgroup) > return; > > - /* Swapcache readahead pages can get replaced before being charged */ > memcg = oldpage->mem_cgroup; > + /* Readahead page is charged too, to see if other page uncharged */ > + VM_WARN_ON_ONCE_PAGE(!memcg, oldpage); > if (!memcg) > return; > > @@ -7104,7 +7103,8 @@ void mem_cgroup_swapout(struct page *page, swp_entry_t entry) > > memcg = page->mem_cgroup; > > - /* Readahead page, never charged */ > + /* Readahead page is charged too, to see if other page uncharged */ > + VM_WARN_ON_ONCE_PAGE(!memcg, page); > if (!memcg) > return; > > @@ -7168,7 +7168,8 @@ int mem_cgroup_try_charge_swap(struct page *page, swp_entry_t entry) > > memcg = page->mem_cgroup; > > - /* Readahead page, never charged */ > + /* Readahead page is charged too, to see if other page uncharged */ > + VM_WARN_ON_ONCE_PAGE(!memcg, page); > if (!memcg) > return 0; > > -- > 1.8.3.1 -- Michal Hocko SUSE Labs
next prev parent reply other threads:[~2020-08-10 9:55 UTC|newest] Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-07-31 3:34 [PATCH] mm/memcg: remove useless check on page->mem_cgroup Alex Shi 2020-07-31 3:34 ` Alex Shi 2020-07-31 15:16 ` Johannes Weiner 2020-07-31 15:16 ` Johannes Weiner 2020-08-01 3:58 ` Alex Shi 2020-08-01 3:58 ` Alex Shi 2020-08-03 8:18 ` Michal Hocko 2020-08-03 8:18 ` Michal Hocko 2020-08-04 7:35 ` Alex Shi 2020-08-04 7:35 ` Alex Shi 2020-08-05 12:28 ` Alex Shi 2020-08-05 13:02 ` Alex Shi 2020-08-08 13:22 ` Alex Shi 2020-08-08 13:22 ` Alex Shi 2020-08-08 21:43 ` Roman Gushchin 2020-08-08 21:43 ` Roman Gushchin 2020-08-10 7:44 ` Alex Shi 2020-08-10 7:44 ` Alex Shi 2020-08-10 9:52 ` Michal Hocko 2020-08-10 9:52 ` Michal Hocko 2020-08-10 9:53 ` Michal Hocko 2020-08-10 9:53 ` Michal Hocko 2020-08-10 9:55 ` Michal Hocko [this message] 2020-08-10 9:55 ` Michal Hocko 2020-08-10 12:29 ` Alex Shi 2020-08-10 12:29 ` Alex Shi
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200810095500.GE4773@dhcp22.suse.cz \ --to=mhocko@suse.com \ --cc=akpm@linux-foundation.org \ --cc=alex.shi@linux.alibaba.com \ --cc=cgroups@vger.kernel.org \ --cc=hannes@cmpxchg.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=vdavydov.dev@gmail.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.