linux-kernel - Re: [PATCH 3/3] mm/smaps: Simplify shmem handling of pte holes

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: <YVxj753GsZB/m7/J@t490s>
Date:   Tue, 5 Oct 2021 10:40:47 -0400
From:   Peter Xu <peterx@...hat.com>
To:     Vlastimil Babka <vbabka@...e.cz>
Cc:     linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        Andrew Morton <akpm@...ux-foundation.org>,
        Hugh Dickins <hughd@...gle.com>,
        Andrea Arcangeli <aarcange@...hat.com>,
        Matthew Wilcox <willy@...radead.org>
Subject: Re: [PATCH 3/3] mm/smaps: Simplify shmem handling of pte holes

Hi, Vlastimil,

On Tue, Oct 05, 2021 at 01:15:05PM +0200, Vlastimil Babka wrote:
> > Since at it, use the pte_hole() helper rather than dup the page cache lookup.
> 
> pte_hole() is for checking a range and we are calling it for single page,
> isnt't that causing larger overhead in the end? There's xarray involved, so
> maybe Matthew will know best.

Per my understanding, pte_hole() calls xas_load() too at last, just like the
old code; it's just that the xas_for_each() of shmem_partial_swap_usage() will
only run one iteration, iiuc.

> 
> > Still keep the CONFIG_SHMEM part so the code can be optimized to nop for !SHMEM.
> > 
> > There will be a very slight functional change in smaps_pte_entry(), that for
> > !SHMEM we'll return early for pte_none (before checking page==NULL), but that's
> > even nicer.
> 
> I don't think this is true, 'unlikely(IS_ENABLED(CONFIG_SHMEM))' will be a
> compile-time constant false and shortcut the rest of the 'if' evaluation
> thus there will be no page check? Or I misunderstood.

The page check I was referring is this one in smaps_pte_entry():

	if (!page)
		return;

After the change, with !SHMEM the "else" block will be kept there (unlike the
old code as you mentioned it'll be optimized), the smaps_pte_hole_lookup() will
be noop so it'll be a direct "return" in that "else", then it should return a
bit earlier by not checking "!page" (because in that case pte_none must have
page==NULL).

Thanks,

-- 
Peter Xu