| Message ID | 20191025232710.4081957-1-guro@fb.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mm: slab: make page_cgroup_ino() to recognize non-compound slab pages properly | expand |
On Fri, Oct 25, 2019 at 04:27:10PM -0700, Roman Gushchin wrote: > page_cgroup_ino() doesn't return a valid memcg pointer for non-compund > slab pages, because it depends on PgHead AND PgSlab flags to be set > to determine the memory cgroup from the kmem_cache. > It's correct for compound pages, but not for generic small pages. Those > don't have PgHead set, so it ends up returning zero. > > Fix this by replacing the condition to PageSlab() && !PageTail(). > > Before this patch: > [root@localhost ~]# ./page-types -c /sys/fs/cgroup/user.slice/user-0.slice/user@0.service/ | grep slab > 0x0000000000000080 38 0 _______S___________________________________ slab > > After this patch: > [root@localhost ~]# ./page-types -c /sys/fs/cgroup/user.slice/user-0.slice/user@0.service/ | grep slab > 0x0000000000000080 147 0 _______S___________________________________ slab > > Fixes: 4d96ba353075 ("mm: memcg/slab: stop setting page->mem_cgroup pointer for slab pages") > Signed-off-by: Roman Gushchin <guro@fb.com> > Cc: Vladimir Davydov <vdavydov.dev@gmail.com> > Cc: Shakeel Butt <shakeelb@google.com> Ping? > --- > mm/memcontrol.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index ea085877c548..00b4188b1bed 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -476,7 +476,7 @@ ino_t page_cgroup_ino(struct page *page) > unsigned long ino = 0; > > rcu_read_lock(); > - if (PageHead(page) && PageSlab(page)) > + if (PageSlab(page) && !PageTail(page)) > memcg = memcg_from_slab_page(page); > else > memcg = READ_ONCE(page->mem_cgroup); > -- > 2.17.1 >
On Fri, Oct 25, 2019 at 4:28 PM Roman Gushchin <guro@fb.com> wrote: > > page_cgroup_ino() doesn't return a valid memcg pointer for non-compund non-compound > slab pages, because it depends on PgHead AND PgSlab flags to be set > to determine the memory cgroup from the kmem_cache. > It's correct for compound pages, but not for generic small pages. Those > don't have PgHead set, so it ends up returning zero. > > Fix this by replacing the condition to PageSlab() && !PageTail(). > > Before this patch: > [root@localhost ~]# ./page-types -c /sys/fs/cgroup/user.slice/user-0.slice/user@0.service/ | grep slab > 0x0000000000000080 38 0 _______S___________________________________ slab > > After this patch: > [root@localhost ~]# ./page-types -c /sys/fs/cgroup/user.slice/user-0.slice/user@0.service/ | grep slab > 0x0000000000000080 147 0 _______S___________________________________ slab > > Fixes: 4d96ba353075 ("mm: memcg/slab: stop setting page->mem_cgroup pointer for slab pages") > Signed-off-by: Roman Gushchin <guro@fb.com> Reviewed-by: Shakeel Butt <shakeelb@google.com> > Cc: Vladimir Davydov <vdavydov.dev@gmail.com> > Cc: Shakeel Butt <shakeelb@google.com> > --- > mm/memcontrol.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index ea085877c548..00b4188b1bed 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -476,7 +476,7 @@ ino_t page_cgroup_ino(struct page *page) > unsigned long ino = 0; > > rcu_read_lock(); > - if (PageHead(page) && PageSlab(page)) > + if (PageSlab(page) && !PageTail(page)) > memcg = memcg_from_slab_page(page); > else > memcg = READ_ONCE(page->mem_cgroup); > -- > 2.17.1 >
Hi Roman, On Fri, Oct 25, 2019 at 04:27:10PM -0700, Roman Gushchin wrote: > page_cgroup_ino() doesn't return a valid memcg pointer for non-compund > slab pages, because it depends on PgHead AND PgSlab flags to be set > to determine the memory cgroup from the kmem_cache. > It's correct for compound pages, but not for generic small pages. Those > don't have PgHead set, so it ends up returning zero. > > Fix this by replacing the condition to PageSlab() && !PageTail(). You may also want to update the comment above memcg_from_slab_page(): * So this function assumes that the page can pass PageHead() and PageSlab() * checks.
On Wed, Oct 30, 2019 at 05:03:14PM -0400, Daniel Jordan wrote: > Hi Roman, > > On Fri, Oct 25, 2019 at 04:27:10PM -0700, Roman Gushchin wrote: > > page_cgroup_ino() doesn't return a valid memcg pointer for non-compund > > slab pages, because it depends on PgHead AND PgSlab flags to be set > > to determine the memory cgroup from the kmem_cache. > > It's correct for compound pages, but not for generic small pages. Those > > don't have PgHead set, so it ends up returning zero. > > > > Fix this by replacing the condition to PageSlab() && !PageTail(). > > You may also want to update the comment above memcg_from_slab_page(): > > * So this function assumes that the page can pass PageHead() and PageSlab() > * checks. Good catch, thank you! Will send v2 in no time.
diff --git a/mm/memcontrol.c b/mm/memcontrol.c index ea085877c548..00b4188b1bed 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -476,7 +476,7 @@ ino_t page_cgroup_ino(struct page *page) unsigned long ino = 0; rcu_read_lock(); - if (PageHead(page) && PageSlab(page)) + if (PageSlab(page) && !PageTail(page)) memcg = memcg_from_slab_page(page); else memcg = READ_ONCE(page->mem_cgroup);
page_cgroup_ino() doesn't return a valid memcg pointer for non-compund slab pages, because it depends on PgHead AND PgSlab flags to be set to determine the memory cgroup from the kmem_cache. It's correct for compound pages, but not for generic small pages. Those don't have PgHead set, so it ends up returning zero. Fix this by replacing the condition to PageSlab() && !PageTail(). Before this patch: [root@localhost ~]# ./page-types -c /sys/fs/cgroup/user.slice/user-0.slice/user@0.service/ | grep slab 0x0000000000000080 38 0 _______S___________________________________ slab After this patch: [root@localhost ~]# ./page-types -c /sys/fs/cgroup/user.slice/user-0.slice/user@0.service/ | grep slab 0x0000000000000080 147 0 _______S___________________________________ slab Fixes: 4d96ba353075 ("mm: memcg/slab: stop setting page->mem_cgroup pointer for slab pages") Signed-off-by: Roman Gushchin <guro@fb.com> Cc: Vladimir Davydov <vdavydov.dev@gmail.com> Cc: Shakeel Butt <shakeelb@google.com> --- mm/memcontrol.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)