| Message ID | 1416478790-27522-3-git-send-email-mgorman@suse.de (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
On Thu, Nov 20, 2014 at 2:19 AM, Mel Gorman <mgorman@suse.de> wrote: > This is a preparatory patch that introduces protnone helpers for automatic > NUMA balancing. Oh, I hadn't noticed that you had renamed these things. It was probably already true in your V1 version. I do *not* think that "pte_protnone_numa()" makes sense as a name. It only confuses people to think that there is still/again something NUMA-special about the PTE. The whole point of the protnone changes was to make it really very very clear that from a hardware standpoint, this is *exactly* about protnone, and nothing else. The fact that we then use protnone PTE's for numa faults is a VM internal issue, it should *not* show up in the architecture page table helpers. I'm not NAK'ing this name, but I really think it's a very important part of the whole patch series - to stop the stupid confusion about NUMA entries. As far as the page tables are concerned, this has absolutely _zero_ to do with NUMA. We made that mistake once. We're fixing it. Let the naming *show* that it's fixed, and this is "pte_protnone()". The places that use this for NUMA handling might have a comment or something. But they'll be in the VM where this matters, not in the architecture page table description files. The comment would be something like "if the vma is accessible, but the PTE is marked protnone, this is a autonuma entry". Linus
On Thu, Nov 20, 2014 at 11:54:06AM -0800, Linus Torvalds wrote: > On Thu, Nov 20, 2014 at 2:19 AM, Mel Gorman <mgorman@suse.de> wrote: > > This is a preparatory patch that introduces protnone helpers for automatic > > NUMA balancing. > > Oh, I hadn't noticed that you had renamed these things. It was > probably already true in your V1 version. > > I do *not* think that "pte_protnone_numa()" makes sense as a name. It > only confuses people to think that there is still/again something > NUMA-special about the PTE. The whole point of the protnone changes > was to make it really very very clear that from a hardware standpoint, > this is *exactly* about protnone, and nothing else. > > The fact that we then use protnone PTE's for numa faults is a VM > internal issue, it should *not* show up in the architecture page table > helpers. > > I'm not NAK'ing this name, but I really think it's a very important > part of the whole patch series - to stop the stupid confusion about > NUMA entries. As far as the page tables are concerned, this has > absolutely _zero_ to do with NUMA. > > We made that mistake once. We're fixing it. Let the naming *show* that > it's fixed, and this is "pte_protnone()". > > The places that use this for NUMA handling might have a comment or > something. But they'll be in the VM where this matters, not in the > architecture page table description files. The comment would be > something like "if the vma is accessible, but the PTE is marked > protnone, this is a autonuma entry". > I feared that people would eventually make the mistake of thinking that pte_protnone() would return true for PROT_NONE VMAs that do *not* have the page table bit set. I'll use the old name as you suggest and expand the comment. It'll be in v3.
diff --git a/arch/powerpc/include/asm/pgtable.h b/arch/powerpc/include/asm/pgtable.h index 316f9a5..452c3b4 100644 --- a/arch/powerpc/include/asm/pgtable.h +++ b/arch/powerpc/include/asm/pgtable.h @@ -39,6 +39,17 @@ static inline int pte_none(pte_t pte) { return (pte_val(pte) & ~_PTE_NONE_MASK) static inline pgprot_t pte_pgprot(pte_t pte) { return __pgprot(pte_val(pte) & PAGE_PROT_BITS); } #ifdef CONFIG_NUMA_BALANCING +static inline int pte_protnone_numa(pte_t pte) +{ + return (pte_val(pte) & + (_PAGE_PRESENT | _PAGE_USER)) == _PAGE_PRESENT; +} + +static inline int pmd_protnone_numa(pmd_t pmd) +{ + return pte_protnone_numa(pmd_pte(pmd)); +} + static inline int pte_present(pte_t pte) { return pte_val(pte) & _PAGE_NUMA_MASK; diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h index aa97a07..613cd00 100644 --- a/arch/x86/include/asm/pgtable.h +++ b/arch/x86/include/asm/pgtable.h @@ -497,6 +497,22 @@ static inline int pmd_present(pmd_t pmd) _PAGE_NUMA); } +#ifdef CONFIG_NUMA_BALANCING +/* + * These work without NUMA balancing but the kernel does not care. See the + * comment in include/asm-generic/pgtable.h + */ +static inline int pte_protnone_numa(pte_t pte) +{ + return pte_flags(pte) & _PAGE_PROTNONE; +} + +static inline int pmd_protnone_numa(pmd_t pmd) +{ + return pmd_flags(pmd) & _PAGE_PROTNONE; +} +#endif /* CONFIG_NUMA_BALANCING */ + static inline int pmd_none(pmd_t pmd) { /* Only check low word on 32-bit platforms, since it might be diff --git a/include/asm-generic/pgtable.h b/include/asm-generic/pgtable.h index 752e30d..7e74122 100644 --- a/include/asm-generic/pgtable.h +++ b/include/asm-generic/pgtable.h @@ -677,6 +677,25 @@ static inline int pmd_trans_unstable(pmd_t *pmd) #endif } +#ifndef CONFIG_NUMA_BALANCING +/* + * Technically a PTE can be PROTNONE even when not doing NUMA balancing but + * the only case the kernel cares is for NUMA balancing. By default, + * implement the helper as "always no". Note that this does not check VMA + * protection bits meaning that it is up to the caller to distinguish between + * PROT_NONE protections and NUMA hinting fault protections. + */ +static inline int pte_protnone_numa(pte_t pte) +{ + return 0; +} + +static inline int pmd_protnone_numa(pmd_t pmd) +{ + return 0; +} +#endif /* CONFIG_NUMA_BALANCING */ + #ifdef CONFIG_NUMA_BALANCING /* * _PAGE_NUMA distinguishes between an unmapped page table entry, an entry that