On Mon 14-01-19 19:29:39, Arun KS wrote:
> On 2019-01-10 21:53, Alexander Duyck wrote:
[...]
> > Couldn't you just do something like the following:
> > if ((end - start) >= (1UL << (MAX_ORDER - 1))
> > order = MAX_ORDER - 1;
> > else
> > order = __fls(end - start);
> >
> > I would think this would save you a few steps in terms of conversions
> > and such since you are already working in page frame numbers anyway so
> > a block of 8 pfns would represent an order 3 page wouldn't it?
> >
> > Also it seems like an alternative to using "end" would be to just track
> > nr_pages. Then you wouldn't have to do the "end - start" math in a few
> > spots as long as you remembered to decrement nr_pages by the amount you
> > increment start by.
>
> Thanks for that. How about this?
>
> static int online_pages_blocks(unsigned long start, unsigned long nr_pages)
> {
> unsigned long end = start + nr_pages;
> int order;
>
> while (nr_pages) {
> if (nr_pages >= (1UL << (MAX_ORDER - 1)))
> order = MAX_ORDER - 1;
> else
> order = __fls(nr_pages);
>
> (*online_page_callback)(pfn_to_page(start), order);
> nr_pages -= (1UL << order);
> start += (1UL << order);
> }
> return end - start;
> }
I find this much less readable so if this is really a big win
performance wise then make it a separate patch with some nubbers please.
--
Michal Hocko
SUSE Labs
