linux-kernel - Re: [PATCH v15 3/7] mm: Add function __putback_isolated

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:   Tue, 17 Dec 2019 10:24:46 -0800
From:   Alexander Duyck <alexander.h.duyck@...ux.intel.com>
To:     David Hildenbrand <david@...hat.com>,
        Alexander Duyck <alexander.duyck@...il.com>,
        kvm@...r.kernel.org, mst@...hat.com, linux-kernel@...r.kernel.org,
        willy@...radead.org, mhocko@...nel.org, linux-mm@...ck.org,
        akpm@...ux-foundation.org, mgorman@...hsingularity.net,
        vbabka@...e.cz
Cc:     yang.zhang.wz@...il.com, nitesh@...hat.com, konrad.wilk@...cle.com,
        pagupta@...hat.com, riel@...riel.com, lcapitulino@...hat.com,
        dave.hansen@...el.com, wei.w.wang@...el.com, aarcange@...hat.com,
        pbonzini@...hat.com, dan.j.williams@...el.com, osalvador@...e.de
Subject: Re: [PATCH v15 3/7] mm: Add function __putback_isolated_page

On Tue, 2019-12-17 at 18:24 +0100, David Hildenbrand wrote:
>  >>> Also there are some scenarios where __page_to_pfn is not that simple a
> > > > call with us having to get the node ID so we can find the pgdat structure
> > > > to perform the calculation. I'm not sure the compiler would be ble to
> > > > figure out that the result is the same for both calls, so it is better to
> > > > make it explicit.
> > > 
> > > Only in case of CONFIG_SPARSEMEM we have to go via the section - but I
> > > doubt this is really worth optimizing here.
> > > 
> > > But yeah, I'm fine with this change, only "IMHO
> > > get_pageblock_migratetype() would be nicer" :)
> > 
> > Aren't most distros running with CONFIG_SPARSEMEM enabled? If that is the
> > case why not optimize for it?
> 
> Because I tend to dislike micro-optimizations without performance
> numbers for code that is not on a hot path. But I mean in this case, as
> you said, you need the pfn either way, so it's completely fine with.
> 
> I do wonder, however, if you should just pass in the migratetype from
> the caller. That would be even faster ;)

The problem is page isolation. We can end up with a page being moved to an
isolate pageblock while we aren't holding the zone lock, and as such we
likely need to test it again anyway. So there isn't value in storing and
reusing the value for cases like page reporting.

In addition, the act of isolating the page can cause the migratetype to
change as __isolate_free_page will attempt to change the migratetype to
movable if it is one of the standard percpu types and we are pulling at
least half a pageblock out. So storing the value before we isolate it
would be problematic as well.

Undoing page isolation is the exception to the issues pointed out above,
but in that case we are overwriting the pageblock migratetype anyway so
the cache lines involved should all be warm from having just set the
value.