Re: [PATCH 3/5] Use per hstate nodes_allowed to constrain huge page allocation

[Lee Schermerhorn <Lee.Schermerhorn@hp.com>]

On Wed, 2009-06-17 at 14:39 +0100, Mel Gorman wrote:
> On Tue, Jun 16, 2009 at 09:53:01AM -0400, Lee Schermerhorn wrote:
> > [PATCH 3/5] Use per hstate nodes_allowed to constrain huge page allocation
> > 
> > Against:  17may09 mmotm
> > 
> > Select only nodes from the per hstate nodes_allowed mask when
> > promoting surplus pages to persistent or when allocating fresh
> > huge pages to the pool.
> > 
> > Note that alloc_buddy_huge_page() still uses task policy to allocate
> > surplus huge pages.  This could be changed.
> > 
> > Signed-off-by: Lee Schermerhorn <lee.schermerhorn@hp.com>
> > 
> >  mm/hugetlb.c |   23 ++++++++++++++++++-----
> >  1 file changed, 18 insertions(+), 5 deletions(-)
> > 
> > Index: linux-2.6.30-rc8-mmotm-090603-1633/mm/hugetlb.c
> > ===================================================================
> > --- linux-2.6.30-rc8-mmotm-090603-1633.orig/mm/hugetlb.c	2009-06-04 12:59:32.000000000 -0400
> > +++ linux-2.6.30-rc8-mmotm-090603-1633/mm/hugetlb.c	2009-06-04 12:59:33.000000000 -0400
> > @@ -637,9 +637,9 @@ static struct page *alloc_fresh_huge_pag
> >  static int hstate_next_node(struct hstate *h)
> >  {
> >  	int next_nid;
> > -	next_nid = next_node(h->hugetlb_next_nid, node_online_map);
> > +	next_nid = next_node(h->hugetlb_next_nid, *h->nodes_allowed);
> >  	if (next_nid == MAX_NUMNODES)
> > -		next_nid = first_node(node_online_map);
> > +		next_nid = first_node(*h->nodes_allowed);
> >  	h->hugetlb_next_nid = next_nid;
> >  	return next_nid;
> >  }
> > @@ -652,6 +652,11 @@ static int alloc_fresh_huge_page(struct 
> >  	int ret = 0;
> >  
> >  	start_nid = h->hugetlb_next_nid;
> > +	/*
> > +	 * we may have allocated with a different nodes_allowed previously
> > +	 */
> > +	if (!node_isset(start_nid, *h->nodes_allowed))
> > +		start_nid = hstate_next_node(h);
> >  
> >  	do {
> >  		page = alloc_fresh_huge_page_node(h, h->hugetlb_next_nid);
> > @@ -1169,20 +1174,28 @@ static inline void try_to_free_low(struc
> >  
> >  /*
> >   * Increment or decrement surplus_huge_pages.  Keep node-specific counters
> > - * balanced by operating on them in a round-robin fashion.
> > + * balanced by operating on them in a round-robin fashion.  Use nodes_allowed
> > + * mask when decreasing suplus pages as we're "promoting" them to persistent.
> 
> s/suplus/surplus/

ACK

> 
> > + * Use node_online_map for increment surplus pages as we're demoting previously
> > + * persistent huge pages.
> > + * Called holding the hugetlb_lock.
> >   * Returns 1 if an adjustment was made.
> >   */
> >  static int adjust_pool_surplus(struct hstate *h, int delta)
> >  {
> > +	nodemask_t *nodemask = &node_online_map;
> >  	static int prev_nid;
> >  	int nid = prev_nid;
> >  	int ret = 0;
> >  
> >  	VM_BUG_ON(delta != -1 && delta != 1);
> > +	if (delta < 0)
> > +		nodemask = h->nodes_allowed;
> > +
> 
> Please spell out why nodes_allowed is only used when decreasing the surplus
> count.

I thought my addition to the comment block did that.  

My thinking:  surplus pages are, by definition, in use, so the only time
we decrease them via adjust_pool_surplus() is when we're increasing
nr_hugepages.  New "persistent" huge pages are masked by nodes_allowed,
so promoting surplus pages to persistent should also be so masked.

Conversely, since this series uses the node_online_mask for freeing
available persistent huge pages, I mention that we use it for increasing
the surplus page count.

Should I add more to the comment?

> 
> >  	do {
> > -		nid = next_node(nid, node_online_map);
> > +		nid = next_node(nid, *nodemask);
> >  		if (nid == MAX_NUMNODES)
> > -			nid = first_node(node_online_map);
> > +			nid = first_node(*nodemask);
> >  
> >  		/* To shrink on this node, there must be a surplus page */
> >  		if (delta < 0 && !h->surplus_huge_pages_node[nid])
> > 
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>