On Fri, 27 Apr 2012, Andrew Morton wrote: > Seems reasonable. But the CONFIG_HUGETLB_PAGE=y, > CONFIG_MEM_RES_CTLR_HUGETLB=n combination will cause unneeded code > generation and space consumption in memcontrol.c. > > I wonder if we can additionally do, within memcontrol.c: > > /* > * Nice comment goes here > */ > #ifdef CONFIG_MEM_RES_CTLR_HUGETLB > #define HUGE_MAX_HSTATE_FOO HUGE_MAX_HSTATE > #else > #define HUGE_MAX_HSTATE_FOO 0 > #endif > > and s/HUGE_MAX_HSTATE/HUGE_MAX_HSTATE_FOO/ in that file. > I haven't looked at the hugetlb memcg controller in-depth (yet), but I really think we should start considering breaking things like this off into its own cgroup. The hugetlb extension seems like something that could be easily separtated, but perhaps I'm saying "easily" because I haven't looked at the implementation. mm/memcontrol.c in linux-next is 5877 lines and, if history is any guide, it's going to continue growing. If the hugetlb usage isn't charged against the memcg's memory.usage_in_bytes like thp is, then I really think it should be its own cgroup. From the hugetlb perspective absent any cgroups, things like hstates (since we're talking about HUGE_MAX_HSTATE) are global resources and so you'd need to preallocate these on the command line or via sysfs before you could mmap them. So if my assumption that the hugetlb memcg controller is only governing these global resources and charging a set of tasks for what they use, then it really has no business in mm/memcontrol.c to begin with, in my opinion. -- To unsubscribe from this list: send the line "unsubscribe linux-next" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
