Messages in this thread | | | Date | Tue, 5 Oct 2021 10:40:47 -0400 | From | Peter Xu <> | 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
| |