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

[Peter Xu <peterx@redhat.com>]

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