From: Michal Hocko <mhocko@kernel.org>
To: Yafang Shao <laoar.shao@gmail.com>
Cc: dchinner@redhat.com, akpm@linux-foundation.org,
linux-mm@kvack.org, Roman Gushchin <guro@fb.com>
Subject: Re: [PATCH] mm: verify page type before getting memcg from it
Date: Thu, 16 Jan 2020 16:50:56 +0100 [thread overview]
Message-ID: <20200116155056.GA19428@dhcp22.suse.cz> (raw)
In-Reply-To: <1579183811-1898-1-git-send-email-laoar.shao@gmail.com>
[Cc Roman]
On Thu 16-01-20 09:10:11, Yafang Shao wrote:
> Per disccusion with Dave[1], we always assume we only ever put objects from
> memcg associated slab pages in the list_lru. In list_lru_from_kmem() it
> calls memcg_from_slab_page() which makes no attempt to verify the page is
> actually a slab page. But currently the binder coder (in
> drivers/android/binder_alloc.c) stores normal pages in the list_lru, rather
> than slab objects. The only reason the binder doesn't catch issue is that
> the list_lru is not configured to be memcg aware.
> In order to make it more stable, we should verify the page type before
> getting memcg from it. In this patch, a new helper is introduced and the
> old helper is modified. Now we have two helpers as bellow,
>
> struct mem_cgroup *__memcg_from_slab_page(struct page *page);
> struct mem_cgroup *memcg_from_slab_page(struct page *page);
>
> The first helper is used when we are sure the page is a slab page and also
> a head page, while the second helper is used when we are not sure the page
> type.
>
> [1].
> https://lore.kernel.org/linux-mm/20200106213103.GJ23195@dread.disaster.area/
>
> Suggested-by: Dave Chinner <david@fromorbit.com>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> ---
> mm/memcontrol.c | 7 ++-----
> mm/slab.h | 24 +++++++++++++++++++++++-
> 2 files changed, 25 insertions(+), 6 deletions(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 9bd4ea7..7658b8e 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -460,10 +460,7 @@ ino_t page_cgroup_ino(struct page *page)
> unsigned long ino = 0;
>
> rcu_read_lock();
> - if (PageSlab(page) && !PageTail(page))
> - memcg = memcg_from_slab_page(page);
> - else
> - memcg = READ_ONCE(page->mem_cgroup);
> + memcg = memcg_from_slab_page(page);
> while (memcg && !(memcg->css.flags & CSS_ONLINE))
> memcg = parent_mem_cgroup(memcg);
> if (memcg)
> @@ -748,7 +745,7 @@ void __mod_lruvec_slab_state(void *p, enum node_stat_item idx, int val)
> struct lruvec *lruvec;
>
> rcu_read_lock();
> - memcg = memcg_from_slab_page(page);
> + memcg = __memcg_from_slab_page(page);
>
> /* Untracked pages have no memcg, no lruvec. Update only the node */
> if (!memcg || memcg == root_mem_cgroup) {
> diff --git a/mm/slab.h b/mm/slab.h
> index 7e94700..2444ae4 100644
> --- a/mm/slab.h
> +++ b/mm/slab.h
> @@ -329,7 +329,7 @@ static inline struct kmem_cache *memcg_root_cache(struct kmem_cache *s)
> * The kmem_cache can be reparented asynchronously. The caller must ensure
> * the memcg lifetime, e.g. by taking rcu_read_lock() or cgroup_mutex.
> */
> -static inline struct mem_cgroup *memcg_from_slab_page(struct page *page)
> +static inline struct mem_cgroup *__memcg_from_slab_page(struct page *page)
> {
> struct kmem_cache *s;
>
> @@ -341,6 +341,23 @@ static inline struct mem_cgroup *memcg_from_slab_page(struct page *page)
> }
>
> /*
> + * If we are not sure whether the page can pass PageSlab() && !PageTail(),
> + * we should use this function. That's the difference between this helper
> + * and the above one.
> + */
> +static inline struct mem_cgroup *memcg_from_slab_page(struct page *page)
> +{
> + struct mem_cgroup *memcg;
> +
> + if (PageSlab(page) && !PageTail(page))
> + memcg = __memcg_from_slab_page(page);
> + else
> + memcg = READ_ONCE(page->mem_cgroup);
> +
> + return memcg;
> +}
> +
> +/*
> * Charge the slab page belonging to the non-root kmem_cache.
> * Can be called for non-root kmem_caches only.
> */
> @@ -438,6 +455,11 @@ static inline struct kmem_cache *memcg_root_cache(struct kmem_cache *s)
> return s;
> }
>
> +static inline struct mem_cgroup *__memcg_from_slab_page(struct page *page)
> +{
> + return NULL;
> +}
> +
> static inline struct mem_cgroup *memcg_from_slab_page(struct page *page)
> {
> return NULL;
> --
> 1.8.3.1
>
--
Michal Hocko
SUSE Labs
next prev parent reply other threads:[~2020-01-16 15:51 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-16 14:10 [PATCH] mm: verify page type before getting memcg from it Yafang Shao
2020-01-16 15:50 ` Michal Hocko [this message]
2020-01-16 16:19 ` Roman Gushchin
2020-01-17 1:14 ` Yafang Shao
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=20200116155056.GA19428@dhcp22.suse.cz \
--to=mhocko@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=dchinner@redhat.com \
--cc=guro@fb.com \
--cc=laoar.shao@gmail.com \
--cc=linux-mm@kvack.org \
/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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).