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
