Re: [PATCH] mm: compaction: Abort compaction if too many pages are isolated and caller is asynchronous

[Minchan Kim <minchan.kim@gmail.com>]

Hi Andrea,

On Fri, Jun 3, 2011 at 6:40 AM, Andrea Arcangeli <aarcange@redhat.com> wrote:
> Hello Minchan,
>
> On Fri, Jun 03, 2011 at 05:21:56AM +0900, Minchan Kim wrote:
>> Isn't it rather aggressive?
>> I think cursor page is likely to be PageTail rather than PageHead.
>> Could we handle it simply with below code?
>
> It's not so likely, there is small percentage of compound pages that
> aren't THP compared to the rest that is either regular pagecache or
> anon regular or anon THP or regular shm. If it's THP chances are we

I mean we have more tail pages than head pages. So I think we are likely to
meet tail pages. Of course, compared to all pages(page cache, anon and
so on), compound pages would be very small percentage.

> isolated the head and it's useless to insist on more tail pages (at
> least for large page size like on x86). Plus we've compaction so

I can't understand your point. Could you elaborate it?

> insisting and screwing lru ordering isn't worth it, better to be
> permissive and abort... in fact I wouldn't dislike to remove the
> entire lumpy logic when COMPACTION_BUILD is true, but that alters the
> trace too...

AFAIK, it's final destination to go as compaction will not break lru
ordering if my patch(inorder-putback) is merged.

>
>> get_page(cursor_page)
>> /* The page is freed already */
>> if (1 == page_count(cursor_page)) {
>>       put_page(cursor_page)
>>       continue;
>> }
>> put_page(cursor_page);
>
> We can't call get_page on an tail page or we break split_huge_page,

Why don't we call get_page on tail page if tail page isn't free?
Maybe I need investigating split_huge_page.

> only an isolated lru can be boosted, if we take the lru_lock and we
> check the page is in lru, then we can isolate and pin it safely.
>

Thanks.


-- 
Kind regards,
Minchan Kim