On Fri, Aug 27, 2021 at 09:42:21AM -0700, Dan Williams wrote:

On Fri, Aug 27, 2021 at 6:05 AM Li, Zhijian [off-list ref] wrote:

quoted

on 2021/8/27 20:10, Jason Gunthorpe wrote:

quoted

On Fri, Aug 27, 2021 at 08:15:40AM +0000, lizhijian@fujitsu.com wrote:

quoted

i looked over the change-log of hmm_vma_handle_pte(), and found that before
4055062 ("mm/hmm: add missing call to hmm_pte_need_fault in HMM_PFN_SPECIAL handling")

hmm_vma_handle_pte() will not check pte_special(pte) if pte_devmap(pte) is true.

when we reached
"if (pte_special(pte) && !is_zero_pfn(pte_pfn(pte))) {"
the pte have already presented and its pte's flag already fulfilled the request flags.


My question is that
Per https://01.org/blogs/dave/2020/linux-consumption-x86-page-table-bits,
pte_devmap(pte) and pte_special(pte) could be both true in fsdax user case, right ?

How? what code creates that?

I see:

insert_pfn():
      /* Ok, finally just insert the thing.. */
      if (pfn_t_devmap(pfn))
              entry = pte_mkdevmap(pfn_t_pte(pfn, prot));
      else
              entry = pte_mkspecial(pfn_t_pte(pfn, prot));

So what code path ends up setting both bits?

  pte_mkdevmap() will set both _PAGE_SPECIAL | PAGE_DEVMAP

  395 static inline pte_t pte_mkdevmap(pte_t pte)
  396 {
  397         return pte_set_flags(pte, _PAGE_SPECIAL|_PAGE_DEVMAP);
  398 }

I can't recall why _PAGE_SPECIAL is there. I'll take a look, but I
think setting _PAGE_SPECIAL in pte_mkdevmap() is overkill.

This is my feeling too, but every arch does it, so hmm should check
it, at least for now as a stable fix

devmap has a struct page so it should be refcounted inside the VMA and
that is the main thing that PAGE_SPECIAL disabled, AFAICR..

The only places where pte_special are used that I wonder if are OK for
devmap have to do with CPU cache maintenance

vm_normal_page(), hmm_vma_handle_pte(), gup_pte_range() all look OK to
drop the special bit

Jason