Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933992AbbGUXeQ (ORCPT ); Tue, 21 Jul 2015 19:34:16 -0400 Received: from mail.linuxfoundation.org ([140.211.169.12]:44102 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933618AbbGUXeI (ORCPT ); Tue, 21 Jul 2015 19:34:08 -0400 Date: Tue, 21 Jul 2015 16:34:07 -0700 From: Andrew Morton To: Vladimir Davydov Cc: Andres Lagar-Cavilla , Minchan Kim , Raghavendra K T , Johannes Weiner , Michal Hocko , Greg Thelen , Michel Lespinasse , David Rientjes , Pavel Emelyanov , Cyrill Gorcunov , Jonathan Corbet , , , , , Subject: Re: [PATCH -mm v9 1/8] memcg: add page_cgroup_ino helper Message-Id: <20150721163407.4e198dfcf61eebbbc49731c2@linux-foundation.org> In-Reply-To: References: X-Mailer: Sylpheed 3.4.1 (GTK+ 2.24.23; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1855 Lines: 52 On Sun, 19 Jul 2015 15:31:10 +0300 Vladimir Davydov 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. > > ... > > --- 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? > +{ > + 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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/