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;
>> }
>>
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
