On Thu, Aug 15, 2013 at 03:45:11PM +0800, Xishi Qiu wrote:
>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);
How much sense it make? nr_pages is still equal to itself since nr_pages can't
larger than 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, send a message with 'unsubscribe linux-mm' in
>the body to majordomo@xxxxxxxxx. For more info on Linux MM,
>see: http://www.linux-mm.org/ .
>Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>