Re: [PATCH 5/5] [hugetlb] Try to grow pool for MAP_SHARED mappings

Paul Jackson
2007-07-13T20:05:08Z

Adam wrote:
> + /*
> + * I haven't figured out how to incorporate this cpuset bodge into
> + * the dynamic hugetlb pool yet. Hopefully someone more familiar with
> + * cpusets can weigh in on their desired semantics. Maybe we can just
> + * drop this check?
> + *
> if (chg > cpuset_mems_nr(free_huge_pages_node))
> return -ENOMEM;
> + */

I can't figure out the value of this check either -- Ken Chen added it, perhaps
he can comment.

But the cpuset behaviour of this hugetlb stuff looks suspicious to me:
1) The code in alloc_fresh_huge_page() seems to round robin over
the entire system, spreading the hugetlb pages uniformly on all nodes.
If one a task in one small cpuset starts aggressively allocating hugetlb
pages, do you think this will work, Adam -- looks to me like we will end
up calling alloc_fresh_huge_page() many times, most of which will fail to
alloc_pages_node() anything because the 'static nid' clock hand will be
pointing at a node outside of the current tasks cpuset (not in that tasks
mems_allowed). Inefficient, but I guess ok.
2) I don't see what keeps us from picking hugetlb pages off -any- node in the
system, perhaps way outside the current cpuset. We shouldn't be looking for
enough available (free_huge_pages - resv_huge_pages) pages in the whole
system. Rather we should be looking for and reserving enough such pages
that are in the current tasks cpuset (set in its mems_allowed, to be precise)
Folks aren't going to want their hugetlb pages coming from outside their
tasks cpuset.
3) If there is some code I missed (good chance) that enforces the rule that
a task can only get a hugetlb page from a node in its cpuset, then this
uniform global allocation of hugetlb pages, as noted in (1) above, can't
be right. Either it will force all nodes, including many nodes outside
of the current tasks cpuset, to bulk up on free hugetlb pages, just to
get enough of them on nodes allowed by the current tasks cpuset, or else
it will fail to get enough on nodes local to the current tasks cpuset.
I don't understand the logic well enough to know which, but either way
sucks.

--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <pj@sgi.com> 1.925.600.0401