lkml.org 
[lkml]   [2018]   [Sep]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC] memory_hotplug: Free pages as pageblock_order
Hello Michal and Balbir,

Thanks for reviewing.

On 2018-09-12 18:27, Balbir Singh wrote:
> On Wed, Sep 12, 2018 at 12:38:53PM +0200, Michal Hocko wrote:
>> On Wed 12-09-18 14:56:45, Arun KS wrote:
>> > When free pages are done with pageblock_order, time spend on
>> > coalescing pages by buddy allocator can be reduced. With
>> > section size of 256MB, hot add latency of a single section
>> > shows improvement from 50-60 ms to less than 1 ms, hence
>> > improving the hot add latency by 60%.
>>
>> Where does the improvement come from? You are still doing the same
>> amount of work except that the number of callbacks is lower. Is this
>> the
>> real source of 60% improvement?
>>
>
> It looks like only the first page of the pageblock is initialized, is
> some of the cost amortized in terms of doing one initialization for
> the page with order (order) and then relying on split_page and helpers
> to do the rest? Of course the number of callbacks reduce by a
> significant
> number as well.
Currently, order zero pages are freed one by one, they goes to pcp list
and later when pcp->count >= pcp->high, kernel calls __free_one_page()
in a loop. __free_one_page() tries to merge these pages to create bigger
order page.

But when we free with higher order page(pageblock_order), this merging
is not done. AFAIU, this is the reason for improvement in hot add
latency.

>
>
>> >
>> > If this looks okey, I'll modify users of set_online_page_callback
>> > and resend clean patch.
>>
>> [...]
>>
>> > +static int generic_online_pages(struct page *page, unsigned int order);
>> > +static online_pages_callback_t online_pages_callback = generic_online_pages;
>> > +
>> > +static int generic_online_pages(struct page *page, unsigned int order)
>> > +{
>> > + unsigned long nr_pages = 1 << order;
>> > + struct page *p = page;
>> > + unsigned int loop;
>> > +
>> > + for (loop = 0 ; loop < nr_pages ; loop++, p++) {
>> > + __ClearPageReserved(p);
>> > + set_page_count(p, 0);
>> > + }
>> > + adjust_managed_page_count(page, nr_pages);
>> > + init_page_count(page);
>> > + __free_pages(page, order);
>> > +
>> > + return 0;
>> > +}
>> > +
>> > +static int online_pages_blocks(unsigned long start_pfn, unsigned long nr_pages)
>> > +{
>> > + unsigned long pages_per_block = (1 << pageblock_order);
>> > + unsigned long nr_pageblocks = nr_pages / pages_per_block;
>> > +// unsigned long rem_pages = nr_pages % pages_per_block;
>> > + int i, ret, onlined_pages = 0;
>> > + struct page *page;
>> > +
>> > + for (i = 0 ; i < nr_pageblocks ; i++) {
>> > + page = pfn_to_page(start_pfn + (i * pages_per_block));
>> > + ret = (*online_pages_callback)(page, pageblock_order);
>> > + if (!ret)
>> > + onlined_pages += pages_per_block;
>> > + else if (ret > 0)
>> > + onlined_pages += ret;
>> > + }
>>
>> Could you explain why does the pages_per_block step makes any sense?
>> Why
>> don't you simply apply handle the full nr_pages worth of memory range
>> instead?
Yes. We can move the this loop to generic_online_pages and do
__free_pages() of pageblock_order.

>>
>> > +/*
>> > + if (rem_pages)
>> > + onlined_pages += online_page_single(start_pfn + i, rem_pages);
>> > +*/
>
> Do we expect no rem_pages with this patch?
I ll remove this code, in assumption that section size will be always
multiple of pageblock_order.

Regards,
Arun
>
>> > +
>> > + return onlined_pages;
>> > +}
>> > +
>> > static int online_pages_range(unsigned long start_pfn, unsigned long nr_pages,
>> > void *arg)
>> > {
>> > - unsigned long i;
>> > unsigned long onlined_pages = *(unsigned long *)arg;
>> > - struct page *page;
>> >
>> > if (PageReserved(pfn_to_page(start_pfn)))
>> > - for (i = 0; i < nr_pages; i++) {
>> > - page = pfn_to_page(start_pfn + i);
>> > - (*online_page_callback)(page);
>> > - onlined_pages++;
>> > - }
>> > + onlined_pages = online_pages_blocks(start_pfn, nr_pages);
>> >
>> > online_mem_sections(start_pfn, start_pfn + nr_pages);
>
>
> Balbir Singh.

\
 
 \ /
  Last update: 2018-09-12 16:10    [W:0.216 / U:0.056 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site