From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Jeons Subject: Re: [PATCH] mm/hugetlb: create hugetlb cgroup file in hugetlb_init Date: Wed, 12 Dec 2012 23:12:14 -0600 Message-ID: <1355375534.1567.1.camel@kernel.cn.ibm.com> References: <50C83F97.3040009@huawei.com> <20121212101917.GD32081@dhcp22.suse.cz> <50C85FFD.10305@huawei.com> <20121212112329.GE32081@dhcp22.suse.cz> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20121212112329.GE32081-2MMpYkNvuYDjFM9bn6wA6Q@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: containers-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Errors-To: containers-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org To: Michal Hocko Cc: Liujiang , containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org, dhillf-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org, Jianguo Wu , cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Hanjun Guo , tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, Xishi Qiu , Andrew Morton , Jiang Liu , aneesh.kumar-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org List-Id: containers.vger.kernel.org On Wed, 2012-12-12 at 12:23 +0100, Michal Hocko wrote: > On Wed 12-12-12 18:44:13, Xishi Qiu wrote: > > On 2012/12/12 18:19, Michal Hocko wrote: > > > > > On Wed 12-12-12 16:25:59, Jianguo Wu wrote: > > >> Build kernel with CONFIG_HUGETLBFS=y,CONFIG_HUGETLB_PAGE=y > > >> and CONFIG_CGROUP_HUGETLB=y, then specify hugepagesz=xx boot option, > > >> system will boot fail. > > >> > > >> This failure is caused by following code path: > > >> setup_hugepagesz > > >> hugetlb_add_hstate > > >> hugetlb_cgroup_file_init > > >> cgroup_add_cftypes > > >> kzalloc <--slab is *not available* yet > > >> > > >> For this path, slab is not available yet, so memory allocated will be > > >> failed, and cause WARN_ON() in hugetlb_cgroup_file_init(). > > >> > > >> So I move hugetlb_cgroup_file_init() into hugetlb_init(). > > > > > > I do not think this is a good idea. hugetlb_init is in __init section as > > > well so what guarantees that the slab is initialized by then? Isn't this > > > just a good ordering that makes this working? > > > > Hi Michal, > > > > __initcall functions will be called in > > start_kernel() > > rest_init() // -> slab is already > > kernel_init() > > kernel_init_freeable() > > do_basic_setup() > > do_initcalls() > > > > and setup_hugepagesz() will be called in > > start_kernel() > > parse_early_param() // -> before mm_init() -> kmem_cache_init() > > > > Is this right? > > Yes this is right. I just noticed that kmem_cache_init_late is an __init > function as well and didn't realize it is called directly. Sorry about > the confusion. > Anyway I still think it would be a better idea to move the call into the > hugetlb_cgroup_create callback where it is more logical IMO but now that > I'm looking at other controllers (blk and kmem.tcp) they all do this from > init calls as well. So it doesn't make sense to have hugetlb behave > differently. Which callback in cgroup_subsys should hugetlb_cgroup_create associated? Currently, there is no such callback. > > So > Acked-by: Michal Hocko > > Thanks!