Messages in this thread | | | Date | Thu, 15 Aug 2013 15:45:11 +0800 | From | Xishi Qiu <> | Subject | Re: [PATCH] mm: skip the page buddy block instead of one page |
| |
On 2013/8/15 12:24, Minchan Kim wrote:
>> Please read full thread in detail. >> >> Mel suggested following as >> >> if (PageBuddy(page)) { >> int nr_pages = (1 << page_order(page)) - 1; >> if (PageBuddy(page)) { >> nr_pages = min(nr_pages, MAX_ORDER_NR_PAGES - 1); >> low_pfn += nr_pages; >> continue; >> } >> } >> >> min(nr_pages, xxx) removes your concern but I think Mel's version >> isn't right. It should be aligned with pageblock boundary so I >> suggested following. >> >> if (PageBuddy(page)) { >> #ifdef CONFIG_MEMORY_ISOLATION >> unsigned long order = page_order(page); >> if (PageBuddy(page)) { >> low_pfn += (1 << order) - 1; >> low_pfn = min(low_pfn, end_pfn); >> } >> #endif >> continue; >> } >>
Hi Minchan,
I understand now, but why use "end_pfn" here? Maybe like this:
if (PageBuddy(page)) { /* * page_order is racy without zone->lock but worst case * by the racing is just skipping pageblock_nr_pages. */ unsigned long nr_pages = 1 << page_order(page); if (likely(PageBuddy(page))) { nr_pages = min(nr_pages, MAX_ORDER_NR_PAGES);
/* Align with pageblock boundary */ if ((low_pfn & (pageblock_nr_pages - 1)) + nr_pages > pageblock_nr_pages) low_pfn = ALIGN(low_pfn + 1, pageblock_nr_pages) - 1; else low_pfn += nr_pages - 1; } continue; }
Thanks, Xishi Qiu
>> so worst case is (pageblock_nr_pages - 1). >> but we don't need to add CONFIG_MEMORY_ISOLATION so my suggestion >> is following as. >> >> if (PageBuddy(page)) { >> unsigned long order = page_order(page); >> if (PageBuddy(page)) { >> low_pfn += (1 << order) - 1; >> low_pfn = min(low_pfn, end_pfn); > > Maybe it should be low_pfn = min(low_pfn, end_pfn - 1). > > >> } >> continue; >> } >> >> >
| |