Re: [PATCHv2] mm, page_vma_mapped: Drop faulty pointer arithmetics in check_pte()

[Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>]

Linus Torvalds wrote:
> On Fri, Jan 19, 2018 at 4:49 AM, Kirill A. Shutemov
> <kirill.shutemov@linux.intel.com> wrote:
> >
> > +       if (pfn < page_to_pfn(pvmw->page))
> > +               return false;
> > +
> > +       /* THP can be referenced by any subpage */
> > +       if (pfn - page_to_pfn(pvmw->page) >= hpage_nr_pages(pvmw->page))
> > +               return false;
> > +
> 
> Is gcc actually clever enough to merge these? The "page_to_pfn()"
> logic can be pretty expensive (exactly for the sparsemem case, but
> per-node DISCOTIGMEM has some complexity too.

As far as I tested, using helper function made no difference. Unless I
explicitly insert barriers like cpu_relax() or smp_mb() between these,
the object side does not change.

> 
> So I'd prefer to make that explicit, perhaps by having a helper
> function that does this something like
> 
>    static inline bool pfn_in_hpage(unsigned long pfn, struct page *hpage)
>    {
>         unsigned long hpage_pfn = page_to_pfn(hpage);
> 
>         return pfn >= hpage_pfn &&  pfn - hpage_pfn < hpage_nr_pages(hpage);
>     }
> 
> and then just use
> 
>     return pfn_in_hpage(pfn, pvmw->page);
> 
> in that caller. Hmm? Wouldn't that be more legible, and avoid the
> repeated pvmw->page and page_to_pfn() cases?
> 
> Even if maybe gcc can do the CSE and turn it all into the same thing
> in the end..

You can apply with

  Acked-by: Michal Hocko <mhocko@suse.com> 
  Tested-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>

added.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>