On Tue, 15 Sep 2009, Lee Schermerhorn wrote:
> Index: linux-2.6.31-mmotm-090914-0157/mm/hugetlb.c
> ===================================================================
> --- linux-2.6.31-mmotm-090914-0157.orig/mm/hugetlb.c 2009-09-15 13:23:01.000000000 -0400
> +++ linux-2.6.31-mmotm-090914-0157/mm/hugetlb.c 2009-09-15 13:42:14.000000000 -0400
> @@ -622,6 +622,20 @@ static struct page *alloc_fresh_huge_pag
> }
>
> /*
> + * common helper function for hstate_next_node_to_{alloc|free}.
> + * return next node in node_online_map, wrapping at end.
> + */
> +static int next_node_allowed(int nid)
> +{
> + nid = next_node(nid, node_online_map);
> + if (nid == MAX_NUMNODES)
> + nid = first_node(node_online_map);
> + VM_BUG_ON(nid >= MAX_NUMNODES);
> +
> + return nid;
> +}
> +
> +/*
> * Use a helper variable to find the next node and then
> * copy it back to next_nid_to_alloc afterwards:
> * otherwise there's a window in which a racer might
> @@ -634,12 +648,12 @@ static struct page *alloc_fresh_huge_pag
> */
> static int hstate_next_node_to_alloc(struct hstate *h)
> {
> - int next_nid;
> - next_nid = next_node(h->next_nid_to_alloc, node_online_map);
> - if (next_nid == MAX_NUMNODES)
> - next_nid = first_node(node_online_map);
> + int nid, next_nid;
> +
> + nid = h->next_nid_to_alloc;
> + next_nid = next_node_allowed(nid);
> h->next_nid_to_alloc = next_nid;
> - return next_nid;
> + return nid;
> }
>
> static int alloc_fresh_huge_page(struct hstate *h)
I thought you had refactored this to drop next_nid entirely since gcc
optimizes it away?
> @@ -649,15 +663,17 @@ static int alloc_fresh_huge_page(struct
> int next_nid;
> int ret = 0;
>
> - start_nid = h->next_nid_to_alloc;
> + start_nid = hstate_next_node_to_alloc(h);
> next_nid = start_nid;
>
> do {
> page = alloc_fresh_huge_page_node(h, next_nid);
> - if (page)
> + if (page) {
> ret = 1;
> + break;
> + }
> next_nid = hstate_next_node_to_alloc(h);
> - } while (!page && next_nid != start_nid);
> + } while (next_nid != start_nid);
>
> if (ret)
> count_vm_event(HTLB_BUDDY_PGALLOC);
> @@ -668,17 +684,19 @@ static int alloc_fresh_huge_page(struct
> }
>
> /*
> - * helper for free_pool_huge_page() - find next node
> - * from which to free a huge page
> + * helper for free_pool_huge_page() - return the next node
> + * from which to free a huge page. Advance the next node id
> + * whether or not we find a free huge page to free so that the
> + * next attempt to free addresses the next node.
> */
> static int hstate_next_node_to_free(struct hstate *h)
> {
> - int next_nid;
> - next_nid = next_node(h->next_nid_to_free, node_online_map);
> - if (next_nid == MAX_NUMNODES)
> - next_nid = first_node(node_online_map);
> + int nid, next_nid;
> +
> + nid = h->next_nid_to_free;
> + next_nid = next_node_allowed(nid);
> h->next_nid_to_free = next_nid;
> - return next_nid;
> + return nid;
> }
>
> /*
Ditto for next_nid.
> @@ -693,7 +711,7 @@ static int free_pool_huge_page(struct hs
> int next_nid;
> int ret = 0;
>
> - start_nid = h->next_nid_to_free;
> + start_nid = hstate_next_node_to_free(h);
> next_nid = start_nid;
>
> do {
> @@ -715,9 +733,10 @@ static int free_pool_huge_page(struct hs
> }
> update_and_free_page(h, page);
> ret = 1;
> + break;
> }
> next_nid = hstate_next_node_to_free(h);
> - } while (!ret && next_nid != start_nid);
> + } while (next_nid != start_nid);
>
> return ret;
> }
> @@ -1028,10 +1047,9 @@ int __weak alloc_bootmem_huge_page(struc
> void *addr;
>
> addr = __alloc_bootmem_node_nopanic(
> - NODE_DATA(h->next_nid_to_alloc),
> + NODE_DATA(hstate_next_node_to_alloc(h)),
> huge_page_size(h), huge_page_size(h), 0);
>
> - hstate_next_node_to_alloc(h);
> if (addr) {
> /*
> * Use the beginning of the huge page to store the
Shouldn't that panic if hstate_next_node_to_alloc() returns a memoryless
node since it uses node_online_map?
--
To unsubscribe from this list: send the line "unsubscribe linux-numa" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
