linux-kernel - Re: [PATCH 3/3] mm: page_alloc: remove redundant set_freepage

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAL1ERfOZ5eiJTDneh=5HAoOs7z2AhxxrKaLL9sHKtRk0Cfcf6Q@mail.gmail.com>
Date:	Wed, 10 Dec 2014 22:44:13 +0800
From:	Weijie Yang <weijie.yang.kh@...il.com>
To:	Vlastimil Babka <vbabka@...e.cz>
Cc:	Weijie Yang <weijie.yang@...sung.com>,
	Joonsoo Kim <iamjoonsoo.kim@....com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Mel Gorman <mgorman@...e.de>, Rik van Riel <riel@...hat.com>,
	Johannes Weiner <hannes@...xchg.org>,
	Minchan Kim <minchan@...nel.org>,
	Linux-Kernel <linux-kernel@...r.kernel.org>,
	Linux-MM <linux-mm@...ck.org>
Subject: Re: [PATCH 3/3] mm: page_alloc: remove redundant set_freepage_migratetype()
 calls

On Tue, Dec 9, 2014 at 5:49 PM, Vlastimil Babka <vbabka@...e.cz> wrote:
> On 12/09/2014 08:51 AM, Weijie Yang wrote:
>>
>> The freepage_migratetype is a temporary cached value which represents
>> the free page's pageblock migratetype. Now we use it in two scenarios:
>>
>> 1. Use it as a cached value in page freeing path. This cached value
>> is temporary and non-100% update, which help us decide which pcp
>> freelist and buddy freelist the page should go rather than using
>> get_pfnblock_migratetype() to save some instructions.
>> When there is race between page isolation and free path, we need use
>> additional method to get a accurate value to put the free pages to
>> the correct freelist and get a precise free pages statistics.
>>
>> 2. Use it in page alloc path to update NR_FREE_CMA_PAGES statistics.
>
>
> Maybe add that in this case, the value is only valid between being set by
> __rmqueue_smallest/__rmqueue_fallback and being consumed by rmqueue_bulk or
> buffered_rmqueue for the purposes of statistics.
> Oh, except that in rmqueue_bulk, we are placing it on pcplists, so it's case
> 1. Tricky.

I will add more description and comments in the next version.
Thanks.

> Anyway, the comments for get/set_freepage_migratetype() say:
>
> /* It's valid only if the page is free path or free_list */
>
> And that's not really true. So should it instead say something like "The
> value is only valid when the page is on pcp list, for determining on which
> free list the page should go if the pcp list is flushed. It is also
> temporarily valid during allocation from free list."

I will update the comments. Thanks

>> This patch aims at the scenario 1 and removes two redundant
>> set_freepage_migratetype() calls, which will make sense in the hot path.
>>
>> Signed-off-by: Weijie Yang <weijie.yang@...sung.com>
>> ---
>>   mm/page_alloc.c |    2 --
>>   1 file changed, 2 deletions(-)
>>
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index 616a2c9..99af01a 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -775,7 +775,6 @@ static void __free_pages_ok(struct page *page,
>> unsigned int order)
>>         migratetype = get_pfnblock_migratetype(page, pfn);
>>         local_irq_save(flags);
>>         __count_vm_events(PGFREE, 1 << order);
>> -       set_freepage_migratetype(page, migratetype);
>>         free_one_page(page_zone(page), page, pfn, order, migratetype);
>>         local_irq_restore(flags);
>>   }
>> @@ -1024,7 +1023,6 @@ int move_freepages(struct zone *zone,
>>                 order = page_order(page);
>>                 list_move(&page->lru,
>>                           &zone->free_area[order].free_list[migratetype]);
>> -               set_freepage_migratetype(page, migratetype);
>>                 page += 1 << order;
>>                 pages_moved += 1 << order;
>>         }
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/