Messages in this thread Patch in this message |  | | Date | Mon, 3 Feb 2020 13:34:42 -0800 | From | Matthew Wilcox <> | Subject | Re: [PATCH RFC 01/10] mm: Add pmd support for _PAGE_SPECIAL |
| |
On Fri, Jan 10, 2020 at 07:03:04PM +0000, Joao Martins wrote: > +++ b/arch/x86/include/asm/pgtable.h > @@ -293,6 +293,15 @@ static inline int pgd_devmap(pgd_t pgd) > { > return 0; > } > +#endif > + > +#ifdef CONFIG_ARCH_HAS_PTE_SPECIAL > +static inline int pmd_special(pmd_t pmd) > +{ > + return !!(pmd_flags(pmd) & _PAGE_SPECIAL); > +} > +#endif
The ifdef/endif don't make much sense here; x86 does have PTE_SPECIAL, and this is an x86 header file, so that can be assumed.
> +++ b/mm/gup.c > @@ -2079,6 +2079,9 @@ static int gup_huge_pmd(pmd_t orig, pmd_t *pmdp, unsigned long addr, > return __gup_device_huge_pmd(orig, pmdp, addr, end, pages, nr); > } > > + if (pmd_special(orig)) > + return 0;
Here, you're calling it unconditionally. I think you need a pmd_special() conditionally defined in include/asm-generic/pgtable.h
+#ifndef CONFIG_ARCH_HAS_PTE_SPECIAL +static inline bool pmd_special(pmd_t pmd) +{ + return false; +} +#endif
(oh, and plese use bool instead of int; I know that's different from pte_special(), but pte_special() predates bool and nobody's done the work to convert it yet)
> +++ b/mm/huge_memory.c > @@ -791,6 +791,8 @@ static void insert_pfn_pmd(struct vm_area_struct *vma, unsigned long addr, > entry = pmd_mkhuge(pfn_t_pmd(pfn, prot)); > if (pfn_t_devmap(pfn)) > entry = pmd_mkdevmap(entry); > + else if (pfn_t_special(pfn)) > + entry = pmd_mkspecial(entry);
Again, we'll need a generic one.
> @@ -823,8 +825,7 @@ vm_fault_t vmf_insert_pfn_pmd(struct vm_fault *vmf, pfn_t pfn, bool write) > * but we need to be consistent with PTEs and architectures that > * can't support a 'special' bit. > */ > - BUG_ON(!(vma->vm_flags & (VM_PFNMAP|VM_MIXEDMAP)) && > - !pfn_t_devmap(pfn)); > + BUG_ON(!(vma->vm_flags & (VM_PFNMAP|VM_MIXEDMAP)));
Should that rather be ...
+ BUG_ON(!(vma->vm_flags & (VM_PFNMAP|VM_MIXEDMAP)) && + !pfn_t_devmap(pfn) && !pfn_t_special(pfn));
I also think this comment needs adjusting:
/* * There is no pmd_special() but there may be special pmds, e.g. * in a direct-access (dax) mapping, so let's just replicate the * !CONFIG_ARCH_HAS_PTE_SPECIAL case from vm_normal_page() here. */
|  |