lkml.org 
[lkml]   [2010]   [Apr]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH 26/39] nobootmem: use lmb.default_alloc_limit in alloc_bootmem path
    From
    Date
    On Mon, 2010-04-12 at 21:50 -0700, Yinghai wrote:
    > On 04/12/2010 09:23 PM, Benjamin Herrenschmidt wrote:
    > > On Thu, 2010-04-08 at 23:03 -0700, Yinghai Lu wrote:
    > >> Generic version __lmb_find_area() is going from high to low, and for 32bit
    > >> active_region for 32bit does include high pages
    > >>
    > >> need to replace the limit with lmb.default_alloc_limit, aka get_max_mapped()
    > >>
    > >> with this patch, x86 32bit could use generic version of __lmb_find_area()
    > >
    > > So you unconditionally add access to some lmb specific data structure to
    > > generic code ? That isn't going to work very well on archs that don't
    > > use lmb.
    >
    > the whole function
    >
    > #ifdef CONFIG_HAVE_LMB
    > u64 __init find_memory_core_early(int nid, u64 size, u64 align,
    > u64 goal, u64 limit)
    > {
    > int i;
    >
    > /* Need to go over early_node_map to find out good range for node */
    > for_each_active_range_index_in_nid(i, nid) {
    > u64 addr;
    > u64 ei_start, ei_last;
    >
    > ei_last = early_node_map[i].end_pfn;
    > ei_last <<= PAGE_SHIFT;
    > ei_start = early_node_map[i].start_pfn;
    > ei_start <<= PAGE_SHIFT;
    > addr = __lmb_find_area(ei_start, ei_last,
    > goal, limit, size, align);
    >
    > if (addr == -1ULL)
    > continue;
    >
    > return addr;
    > }
    >
    > return -1ULL;
    > }
    > #endif
    >
    > need to access early_node_map[], so leave the function here.

    Hrm... find_memory_core_early() is a broken API anyways. Did you add
    that ? Again, you insist on pushing all over the place that crakpot
    find/reserve API instead of doing a proper allocation, and it's now
    leaking with ifdef's & all into the generic code.

    This is just all a pile of shit.

    I'm tempted to NACK the whole thing and wait for somebody who can code
    to come up with something half decent.

    Ben.


    > >
    > > Also, those things should be local to lmb_* anyways.
    >
    > if you insist, could move it to lmb.c and use work_with_active_regions() around it.
    >
    > YH
    > --
    > To unsubscribe from this list: send the line "unsubscribe linux-arch" in
    > the body of a message to majordomo@vger.kernel.org
    > More majordomo info at http://vger.kernel.org/majordomo-info.html




    \
     
     \ /
      Last update: 2010-04-13 07:17    [W:3.169 / U:0.268 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site