Linux Virtualization - Mar 2016 - [PATCH v1 01/19] mm: use put_page to free page instead of putback_lru_page

On 03/11/2016 08:30 AM, Minchan Kim wrote:
> Procedure of page migration is as follows:
>
> First of all, it should isolate a page from LRU and try to
> migrate the page. If it is successful, it releases the page
> for freeing. Otherwise, it should put the page back to LRU
> list.
>
> For LRU pages, we have used putback_lru_page for both freeing
> and putback to LRU list. It's okay because put_page is aware of
> LRU list so if it releases last refcount of the page, it removes
> the page from LRU list. However, It makes unnecessary operations
> (e.g., lru_cache_add, pagevec and flags operations.
Yeah, and compaction (perhaps also other migration users) has to drain 
the lru pvec... Getting rid of this stuff is worth even by itself.
> It would be
> not significant but no worth to do) and harder to support new
> non-lru page migration because put_page isn't aware of non-lru
> page's data structure.
>
> To solve the problem, we can add new hook in put_page with
> PageMovable flags check but it can increase overhead in
> hot path and needs new locking scheme to stabilize the flag check
> with put_page.
>
> So, this patch cleans it up to divide two semantic(ie, put and putback).
> If migration is successful, use put_page instead of putback_lru_page and
> use putback_lru_page only on failure. That makes code more readable
> and doesn't add overhead in put_page.
I had an idea of checking for count==1 in putback_lru_page() which would 
take the put_page() shortcut from there. But maybe it can't be done 
nicely without races.
> Cc: Vlastimil Babka <vbabka at suse.cz>
> Cc: Mel Gorman <mgorman at suse.de>
> Cc: Hugh Dickins <hughd at google.com>
> Cc: Naoya Horiguchi <n-horiguchi at ah.jp.nec.com>
> Signed-off-by: Minchan Kim <minchan at kernel.org>
Acked-by: Vlastimil Babka <vbabka at suse.cz>

Note in -next/after 4.6-rc1 this will need some rebasing though.

On Mon, Mar 14, 2016 at 09:48:33AM +0100, Vlastimil Babka
wrote:
> On 03/11/2016 08:30 AM, Minchan Kim wrote:
> >Procedure of page migration is as follows:
> >
> >First of all, it should isolate a page from LRU and try to
> >migrate the page. If it is successful, it releases the page
> >for freeing. Otherwise, it should put the page back to LRU
> >list.
> >
> >For LRU pages, we have used putback_lru_page for both freeing
> >and putback to LRU list. It's okay because put_page is aware of
> >LRU list so if it releases last refcount of the page, it removes
> >the page from LRU list. However, It makes unnecessary operations
> >(e.g., lru_cache_add, pagevec and flags operations.
> 
> Yeah, and compaction (perhaps also other migration users) has to
> drain the lru pvec... Getting rid of this stuff is worth even by
> itself.
Good note. Although we cannot remove lru pvec draining completely,
at least, this patch removes a case which should drain pvec for
returning freed page to buddy.

Thanks for the notice.
> 
> >It would be
> >not significant but no worth to do) and harder to support new
> >non-lru page migration because put_page isn't aware of non-lru
> >page's data structure.
> >
> >To solve the problem, we can add new hook in put_page with
> >PageMovable flags check but it can increase overhead in
> >hot path and needs new locking scheme to stabilize the flag check
> >with put_page.
> >
> >So, this patch cleans it up to divide two semantic(ie, put and
putback).
> >If migration is successful, use put_page instead of putback_lru_page
and
> >use putback_lru_page only on failure. That makes code more readable
> >and doesn't add overhead in put_page.
> 
> I had an idea of checking for count==1 in putback_lru_page() which
> would take the put_page() shortcut from there. But maybe it can't be
> done nicely without races.
I thought about it and we might do it via page_freeze_refs but
what I want at this moment is to separte two semantic put and putback.
;-)
> 
> >Cc: Vlastimil Babka <vbabka at suse.cz>
> >Cc: Mel Gorman <mgorman at suse.de>
> >Cc: Hugh Dickins <hughd at google.com>
> >Cc: Naoya Horiguchi <n-horiguchi at ah.jp.nec.com>
> >Signed-off-by: Minchan Kim <minchan at kernel.org>
> 
> Acked-by: Vlastimil Babka <vbabka at suse.cz>
> 
> Note in -next/after 4.6-rc1 this will need some rebasing though.
Thanks for the review!

On 15.3.2016 2:16, Minchan Kim wrote:
> On Mon, Mar 14, 2016 at 09:48:33AM +0100, Vlastimil Babka wrote:
>> On 03/11/2016 08:30 AM, Minchan Kim wrote:
>>
>> Yeah, and compaction (perhaps also other migration users) has to
>> drain the lru pvec... Getting rid of this stuff is worth even by
>> itself.
> 
> Good note. Although we cannot remove lru pvec draining completely,
> at least, this patch removes a case which should drain pvec for
> returning freed page to buddy.
And this is in fact the only interesting case, right. The migrated page (at its
new target) doesn't concern compaction that much, that can go to lru pvec
just
fine. But we do want the freed buddy pages to merge ASAP. I guess that's the
same for CMA, page isolation...