On Fri, Aug 27, 2021 at 09:05:21PM +0800, Li, Zhijian wrote: > > on 2021/8/27 20:10, Jason Gunthorpe wrote: > > On Fri, Aug 27, 2021 at 08:15:40AM +0000, lizhijian@xxxxxxxxxxx wrote: > > > 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 } > > below is a calltrace example > > [ 400.728559] Call Trace: > [ 400.731595] dump_stack+0x6d/0x8b > [ 400.735536] insert_pfn+0x16c/0x180 > [ 400.739596] __vm_insert_mixed+0x84/0xc0 > [ 400.744144] dax_iomap_pte_fault+0x845/0x870 > [ 400.749089] ext4_dax_huge_fault+0x171/0x1e0 > [ 400.754096] __do_fault+0x31/0xe0 > [ 400.758090] ? pmd_devmap_trans_unstable+0x37/0x90 > [ 400.763541] handle_mm_fault+0x11b1/0x1680 > [ 400.768260] exc_page_fault+0x2f4/0x570 > [ 400.772788] ? asm_exc_page_fault+0x8/0x30 > [ 400.777539] asm_exc_page_fault+0x1e/0x30 > > > So is my previous change reasonable ? Yes, can you send a proper patch and include the mm mailing list? Jason