Re: inux-next: Tree for Apr 27 (uml + mm/memcontrol.c)

[David Rientjes]

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-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/