lkml.org 
[lkml]   [2008]   [Sep]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH] properly reserve in bootmem the lmb reserved regions that cross numa nodes
    From
    Date
    This seems like the right approach to me.  I have pointed out a few
    stylistic issues below.

    On Tue, 2008-09-30 at 09:53 -0500, Jon Tollefson wrote:
    <snip>
    > + /* Mark reserved regions */
    > + for (i = 0; i < lmb.reserved.cnt; i++) {
    > + unsigned long physbase = lmb.reserved.region[i].base;
    > + unsigned long size = lmb.reserved.region[i].size;
    > + unsigned long start_pfn = physbase >> PAGE_SHIFT;
    > + unsigned long end_pfn = ((physbase+size-1) >> PAGE_SHIFT);

    CodingStyle dictates that this should be:
    unsigned long end_pfn = ((physbase + size - 1) >> PAGE_SHIFT);

    <snip>

    > +/**
    > + * get_node_active_region - Return active region containing start_pfn
    > + * @start_pfn The page to return the region for.
    > + *
    > + * It will return NULL if active region is not found.
    > + */
    > +struct node_active_region *get_node_active_region(
    > + unsigned long start_pfn)

    Bad style. I think the convention would be to write it like this:

    struct node_active_region *
    get_node_active_region(unsigned long start_pfn)

    > +{
    > + int i;
    > + for (i = 0; i < nr_nodemap_entries; i++) {
    > + unsigned long node_start_pfn = early_node_map[i].start_pfn;
    > + unsigned long node_end_pfn = early_node_map[i].end_pfn;
    > +
    > + if (node_start_pfn <= start_pfn && node_end_pfn > start_pfn)
    > + return &early_node_map[i];
    > + }
    > + return NULL;
    > +}

    Since this is using the early_node_map[], should we mark the function
    __mminit?

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



    \
     
     \ /
      Last update: 2008-09-30 17:51    [W:0.036 / U:2.512 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site