Re: [patch 046/181] mm: remove sparsemem allocation details from the bootmem allocator

[Johannes Weiner <hannes@cmpxchg.org>]

On Sat, Jun 23, 2012 at 03:35:37PM -0700, Yinghai Lu wrote:
> On Sat, Jun 23, 2012 at 3:06 PM, Johannes Weiner <hannes@cmpxchg.org> wrote:
> > On Sat, Jun 23, 2012 at 11:58:02AM -0700, Yinghai Lu wrote:
> >> +again:
> >> +     p = ___alloc_bootmem_node_nopanic(NODE_DATA(nid), size,
> >> +                                       SMP_CACHE_BYTES, goal, limit);
> >> +     if (!p && limit) {
> >> +             limit = 0;
> >> +             goto again;
> >> +     }
> >
> > I was aware that __alloc_bootmem_node_nopanic does a slightly
> > different fallback sequence, but as soon as you go outside the
> > section, you have a node-section-dependency either way.
> 
> you need to make sure, first try to get the in the same section, then same node.
> 
> >
> > Without this patch, it would retry without the node first, then
> > without the goal.  With your patch, you would try to limit the upper
> > end, but you may already go below the requested section.  What would
> > the limit gain?
> 
> goal: start of section
> limit: start of next section.
> 
> the layout should be
> start_of_range_same_node...start_of_range_same_section....end_of_range_same_section...end_of_range_of_same_node.
> 
> before your commit:
> 1. first try: start_of_range_same_section....end_of_range_same_section
> 2. second try: on same node.
> 
> with your commit:
> 1. first try: start_of_range_same_section....end_of_range_same_node.
> ===> it would get buffer out of same section.
> 2. second try: 0...0, aka 0... -1UL
> 
> with this patch:
> 1. first try: start_of_range_same_section....end_of_range_same_section
> 2. second try: 0...end_of_range_of_same_section.
> 3. third try: start_of_range_same_section...0, aka
> tart_of_range_same_section...-1UL;
> 4. third try: 0...0, aka 0...-1UL;

What I failed to take into account at first is that neither bootmem
implementation (necessarily) does a linear bottom-up search starting
from goal, so yes, we definitely need an upper limit for the first
attempt.

No use in reverting it, as the bootmem version was already broken
before, when the limit was removed in f5bf18f "bootmem/sparsemem:
remove limit constraint in alloc_bootmem_section".

I'm okay with exporting and using ___alloc_bootmem_node_nopanic() in
3.5.