Re: [PATCH -mm v9 1/8] memcg: add page_cgroup_ino helper
flat view
From: Andrew Morton <akpm@linux-foundation.org>
Date: 2015-07-21 23:34:16
Also in:
cgroups, linux-mm, lkml
On Sun, 19 Jul 2015 15:31:10 +0300 Vladimir Davydov [off-list ref] wrote:
This function returns the inode number of the closest online ancestor of the memory cgroup a page is charged to. It is required for exporting information about which page is charged to which cgroup to userspace, which will be introduced by a following patch. ...
quoted hunk ↗ jump to hunk
--- a/mm/memcontrol.c +++ b/mm/memcontrol.c@@ -441,6 +441,29 @@ struct cgroup_subsys_state *mem_cgroup_css_from_page(struct page *page) return &memcg->css; } +/** + * page_cgroup_ino - return inode number of the memcg a page is charged to + * @page: the page + * + * Look up the closest online ancestor of the memory cgroup @page is charged to + * and return its inode number or 0 if @page is not charged to any cgroup. It + * is safe to call this function without holding a reference to @page. + */ +unsigned long page_cgroup_ino(struct page *page)
Shouldn't it return an ino_t?
quoted hunk ↗ jump to hunk
+{ + struct mem_cgroup *memcg; + unsigned long ino = 0; + + rcu_read_lock(); + memcg = READ_ONCE(page->mem_cgroup); + while (memcg && !(memcg->css.flags & CSS_ONLINE)) + memcg = parent_mem_cgroup(memcg); + if (memcg) + ino = cgroup_ino(memcg->css.cgroup); + rcu_read_unlock(); + return ino; +}
The function is racy, isn't it? There's nothing to prevent this inode
from getting torn down and potentially reallocated one nanosecond after
page_cgroup_ino() returns? If so, it is only safely usable by things
which don't care (such as procfs interfaces) and this should be
documented in some fashion.