lkml.org 
[lkml]   [2013]   [Jun]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [Part3 PATCH v2 1/4] bootmem, mem-hotplug: Register local pagetable pages with LOCAL_NODE_DATA when freeing bootmem.
    Hi Michal,

    Please see below.

    On 06/13/2013 10:16 PM, Michal Nazarewicz wrote:
    ......
    >> diff --git a/include/linux/memblock.h b/include/linux/memblock.h
    >> index a85ced9..8a38eef 100644
    >> --- a/include/linux/memblock.h
    >> +++ b/include/linux/memblock.h
    >> @@ -131,6 +131,28 @@ void __next_free_mem_range_rev(u64 *idx, int nid, phys_addr_t *out_start,
    >> i != (u64)ULLONG_MAX; \
    >> __next_free_mem_range_rev(&i, nid, p_start, p_end, p_nid))
    >>
    >> +void __next_local_node_mem_range(int *idx, int nid, phys_addr_t *out_start,
    >> + phys_addr_t *out_end, int *out_nid);
    >
    > Why not make it return int?

    The same reason below.

    >
    >> +
    >> +/**
    >> + * for_each_local_node_mem_range - iterate memblock areas storing local node
    >> + * data
    >> + * @i: int used as loop variable
    >> + * @nid: node selector, %MAX_NUMNODES for all nodes
    >> + * @p_start: ptr to phys_addr_t for start address of the range, can be %NULL
    >> + * @p_end: ptr to phys_addr_t for end address of the range, can be %NULL
    >> + * @p_nid: ptr to int for nid of the range, can be %NULL
    >> + *
    >> + * Walks over memblock areas storing local node data. Since all the local node
    >> + * areas will be reserved by memblock, this iterator will only iterate
    >> + * memblock.reserve. Available as soon as memblock is initialized.
    >> + */
    >> +#define for_each_local_node_mem_range(i, nid, p_start, p_end, p_nid) \
    >> + for (i = -1, \
    >> + __next_local_node_mem_range(&i, nid, p_start, p_end, p_nid); \
    >> + i != -1; \
    >> + __next_local_node_mem_range(&i, nid, p_start, p_end, p_nid))
    >> +
    >
    > If __next_local_node_mem_range() returned int, this would be easier:
    >
    > +#define for_each_local_node_mem_range(i, nid, p_start, p_end, p_nid) \
    > + for (i = -1;
    > + (i = __next_local_node_mem_range(i, nid, p_start, p_end, p_nid)) != -1; )

    Yes, we can do it like this.

    But I tried to do something similar to for_each_free_mem_range and
    for_each_free_mem_range_reverse to keep the code coincident.

    How do you think to change all this similar functions into your way ?

    ......
    >> +void __init_memblock __next_local_node_mem_range(int *idx, int nid,
    >> + phys_addr_t *out_start,
    >> + phys_addr_t *out_end, int *out_nid)
    >> +{
    >> + __next_flag_mem_range(idx, nid, MEMBLK_LOCAL_NODE,
    >> + out_start, out_end, out_nid);
    >> +}
    > static inline in a header file perhaps?

    OK, will put it in a header file in the next version.

    Thanks. :)



    \
     
     \ /
      Last update: 2013-06-14 08:01    [W:2.097 / U:0.180 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site