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

Alexander Duyck
2019-09-10T14:48:57Z

On Tue, Sep 10, 2019 at 5:20 AM Michal Hocko <mhocko@kernel.org> wrote:
>
> On Sat 07-09-19 10:25:20, Alexander Duyck wrote:
> > From: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> >
> > 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@intel.com>
> > Signed-off-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
>
> [...]
>
> > - 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);