Re: [PATCH v9 2/8] mm: Adjust shuffle code to allow for future coalescing

[Alexander Duyck <alexander.duyck@xxxxxxxxx>]

On Tue, Sep 10, 2019 at 5:20 AM Michal Hocko <mhocko@xxxxxxxxxx> wrote:
>
> On Sat 07-09-19 10:25:20, Alexander Duyck wrote:
> > From: Alexander Duyck <alexander.h.duyck@xxxxxxxxxxxxxxx>
> >
> > Move the head/tail adding logic out of the shuffle code and into the
> > __free_one_page function since ultimately that is where it is really
> > needed anyway. By doing this we should be able to reduce the overhead
> > and can consolidate all of the list addition bits in one spot.
>
> This changelog doesn't really explain why we want this. You are
> reshuffling the code, allright, but why do we want to reshuffle? Is the
> result readability a better code reuse or something else? Where
> does the claimed reduced overhead coming from?
>
> From a quick look buddy_merge_likely looks nicer than the code splat
> we have. Good.
>
> But then
>
> > Reviewed-by: Dan Williams <dan.j.williams@xxxxxxxxx>
> > Signed-off-by: Alexander Duyck <alexander.h.duyck@xxxxxxxxxxxxxxx>
>
> [...]
>
> > -     if (is_shuffle_order(order))
> > -             add_to_free_area_random(page, &zone->free_area[order],
> > -                             migratetype);
> > +     area = &zone->free_area[order];
> > +     if (is_shuffle_order(order) ? shuffle_pick_tail() :
> > +         buddy_merge_likely(pfn, buddy_pfn, page, order))
>
> Ouch this is just awful don't you think?

Yeah. I am going to go with Kirill's suggestion and probably do
something more along the lines of:
       bool to_tail;
        ...
        if (is_shuffle_order(order))
                to_tail = shuffle_pick_tail();
       else
                to_tail = buddy_merge_likely(pfn, buddy_pfn, page, order);

        if (to_tail)
                add_to_free_area_tail(page, area, migratetype);
        else
                add_to_free_area(page, area, migratetype);