lkml.org 
[lkml]   [2008]   [Mar]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH] [1/18] Convert hugeltlb.c over to pass global state around in a structure
    From
    Date
    I didn't see anything fundamentally wrong with this... In fact it is
    looking really nice notwithstanding the minor nits below.

    On Mon, 2008-03-17 at 02:58 +0100, Andi Kleen wrote:
    > Large, but rather mechanical patch that converts most of the hugetlb.c
    > globals into structure members and passes them around.
    >
    > Right now there is only a single global hstate structure, but
    > most of the infrastructure to extend it is there.
    >
    > Signed-off-by: Andi Kleen <ak@suse.de>
    >
    <snip>
    > @@ -117,23 +113,24 @@ static struct page *dequeue_huge_page_vm
    > return page;
    > }
    >
    > -static void update_and_free_page(struct page *page)
    > +static void update_and_free_page(struct hstate *h, struct page *page)
    > {
    > int i;
    > - nr_huge_pages--;
    > - nr_huge_pages_node[page_to_nid(page)]--;
    > - for (i = 0; i < (HPAGE_SIZE / PAGE_SIZE); i++) {
    > + h->nr_huge_pages--;
    > + h->nr_huge_pages_node[page_to_nid(page)]--;
    > + for (i = 0; i < (1 << huge_page_order(h)); i++) {
    > page[i].flags &= ~(1 << PG_locked | 1 << PG_error | 1 << PG_referenced |
    > 1 << PG_dirty | 1 << PG_active | 1 << PG_reserved |
    > 1 << PG_private | 1<< PG_writeback);
    > }

    Could you define a macro for (1 << huge_page_order(h))? It is used at
    least 4 times. How about something like pages_per_huge_page(h) or
    something? I think that would convey the meaning more clearly.

    <snip>

    > @@ -190,18 +187,18 @@ static int adjust_pool_surplus(int delta
    > return ret;
    > }
    >
    > -static struct page *alloc_fresh_huge_page_node(int nid)
    > +static struct page *alloc_fresh_huge_page_node(struct hstate *h, int nid)
    > {
    > struct page *page;
    >
    > page = alloc_pages_node(nid,
    > htlb_alloc_mask|__GFP_COMP|__GFP_THISNODE|__GFP_NOWARN,
    > - HUGETLB_PAGE_ORDER);
    > + huge_page_order(h));

    Whitespace?

    <snip>

    > @@ -272,17 +270,17 @@ static struct page *alloc_buddy_huge_pag
    > * per-node value is checked there.
    > */
    > spin_lock(&hugetlb_lock);
    > - if (surplus_huge_pages >= nr_overcommit_huge_pages) {
    > + if (h->surplus_huge_pages >= h->nr_overcommit_huge_pages) {
    > spin_unlock(&hugetlb_lock);
    > return NULL;
    > } else {
    > - nr_huge_pages++;
    > - surplus_huge_pages++;
    > + h->nr_huge_pages++;
    > + h->surplus_huge_pages++;
    > }
    > spin_unlock(&hugetlb_lock);
    >
    > page = alloc_pages(htlb_alloc_mask|__GFP_COMP|__GFP_NOWARN,
    > - HUGETLB_PAGE_ORDER);
    > + huge_page_order(h));

    Whitespace?

    --
    Adam Litke - (agl at us.ibm.com)
    IBM Linux Technology Center



    \
     
     \ /
      Last update: 2008-03-17 21:17    [W:0.025 / U:5.772 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site