linux-kernel - Re: [PATCH 2/2] mm,thp,compaction,cma: allow THP migration for CMA allocations

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Fri, 28 Feb 2020 16:17:17 +0100
From:   Vlastimil Babka <vbabka@...e.cz>
To:     Rik van Riel <riel@...riel.com>, linux-kernel@...r.kernel.org
Cc:     kernel-team@...com, akpm@...ux-foundation.org, linux-mm@...ck.org,
        mhocko@...nel.org, mgorman@...hsingularity.net,
        rientjes@...gle.com, aarcange@...hat.com
Subject: Re: [PATCH 2/2] mm,thp,compaction,cma: allow THP migration for CMA
 allocations

On 2/26/20 6:53 PM, Rik van Riel wrote:
> On Wed, 2020-02-26 at 10:48 +0100, Vlastimil Babka wrote:
>> On 2/25/20 7:44 PM, Rik van Riel wrote:
>>
>> Uh, is it any different from base pages which have to pass the same
>> check? I
>> guess the caller could do e.g. lru_add_drain_all() first.
> 
> You are right, it is not different.
> 
> As for lru_add_drain_all(), I wonder at what point that
> should happen?

Right now it seems to be done in alloc_contig_range(), but rather late.

> It appears that the order in which things are done does
> not really provide a good moment:
> 1) decide to attempt allocating a range of memory
> 2) scan each page block for unmovable pages
> 3) if no unmovable pages are found, mark the page block
>    MIGRATE_ISOLATE
> 
> I wonder if we should do things the opposite way, first
> marking the page block MIGRATE_ISOLATE (to prevent new
> allocations), then scanning it, and calling lru_add_drain_all
> if we encounter a page that looks like it could benefit from
> that.
> 
> If we still see unmovable pages after that, it is cheap
> enough to set the page block back to its previous state.

Yeah seems like the whole has_unmovable_pages() thing isn't much useful
here. It might prevent some unnecessary action like isolating something,
then finding non-movable page and rolling back the isolation. But maybe
it's not worth the savings, and also has_unmovable_pages() being false
doesn't guarantee succeed in the actual isolate+migrate attempt.  And if
it can cause a false negative due to lru pages not drained, then it's
actually worse than if it wasn't called at all.