| Message ID | 1584507679-11976-1-git-send-email-anshuman.khandual@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/mm: Make pud_present() check _PAGE_PROTNONE and _PAGE_PSE as well | expand |
On 03/18/2020 10:31 AM, Anshuman Khandual wrote: > pud_present() should also check _PAGE_PROTNONE and _PAGE_PSE bits like in > case pmd_present(). This makes a PUD entry test positive for pud_present() > after getting invalidated with pud_mknotpresent(), hence standardizing the > semantics with PMD helpers. > > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: Borislav Petkov <bp@alien8.de> > Cc: "H. Peter Anvin" <hpa@zytor.com> > Cc: Dave Hansen <dave.hansen@intel.com> > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: x86@kernel.org > Cc: linux-mm@kvack.org > Cc: linux-kernel@vger.kernel.org > Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com> > --- > Even though pud_mknotpresent() is not used any where currently, there is > a discrepancy between PMD and PUD. > > WARN_ON(!pud_present(pud_mknotpresent(pud_mkhuge(pud)))) -> Fail > WARN_ON(!pmd_present(pmd_mknotpresent(pmd_mkhuge(pmd)))) -> Pass > > Though pud_mknotpresent() currently clears _PAGE_PROTNONE, pud_present() > does not check it. This change fixes both inconsistencies. > > This has been build and boot tested on x86. Adding Kirill and Dan. +Cc: Kirill A. Shutemov <kirill@shutemov.name> +Cc: Dan Williams <dan.j.williams@intel.com>
On Fri, Mar 20, 2020 at 08:53:16AM +0530, Anshuman Khandual wrote: > > > On 03/18/2020 10:31 AM, Anshuman Khandual wrote: > > pud_present() should also check _PAGE_PROTNONE and _PAGE_PSE bits like in > > case pmd_present(). This makes a PUD entry test positive for pud_present() > > after getting invalidated with pud_mknotpresent(), hence standardizing the > > semantics with PMD helpers. > > > > Cc: Thomas Gleixner <tglx@linutronix.de> > > Cc: Ingo Molnar <mingo@redhat.com> > > Cc: Borislav Petkov <bp@alien8.de> > > Cc: "H. Peter Anvin" <hpa@zytor.com> > > Cc: Dave Hansen <dave.hansen@intel.com> > > Cc: Andrew Morton <akpm@linux-foundation.org> > > Cc: x86@kernel.org > > Cc: linux-mm@kvack.org > > Cc: linux-kernel@vger.kernel.org > > Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com> > > --- > > Even though pud_mknotpresent() is not used any where currently, there is > > a discrepancy between PMD and PUD. > > > > WARN_ON(!pud_present(pud_mknotpresent(pud_mkhuge(pud)))) -> Fail > > WARN_ON(!pmd_present(pmd_mknotpresent(pmd_mkhuge(pmd)))) -> Pass > > > > Though pud_mknotpresent() currently clears _PAGE_PROTNONE, pud_present() > > does not check it. This change fixes both inconsistencies. > > > > This has been build and boot tested on x86. > > Adding Kirill and Dan. > > +Cc: Kirill A. Shutemov <kirill@shutemov.name> > +Cc: Dan Williams <dan.j.williams@intel.com> Or we can just drop the pud_mknotpresent(). There's no users AFAICS and only x86 provides it.
On 03/20/2020 05:17 PM, Kirill A. Shutemov wrote: > On Fri, Mar 20, 2020 at 08:53:16AM +0530, Anshuman Khandual wrote: >> >> >> On 03/18/2020 10:31 AM, Anshuman Khandual wrote: >>> pud_present() should also check _PAGE_PROTNONE and _PAGE_PSE bits like in >>> case pmd_present(). This makes a PUD entry test positive for pud_present() >>> after getting invalidated with pud_mknotpresent(), hence standardizing the >>> semantics with PMD helpers. >>> >>> Cc: Thomas Gleixner <tglx@linutronix.de> >>> Cc: Ingo Molnar <mingo@redhat.com> >>> Cc: Borislav Petkov <bp@alien8.de> >>> Cc: "H. Peter Anvin" <hpa@zytor.com> >>> Cc: Dave Hansen <dave.hansen@intel.com> >>> Cc: Andrew Morton <akpm@linux-foundation.org> >>> Cc: x86@kernel.org >>> Cc: linux-mm@kvack.org >>> Cc: linux-kernel@vger.kernel.org >>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com> >>> --- >>> Even though pud_mknotpresent() is not used any where currently, there is >>> a discrepancy between PMD and PUD. >>> >>> WARN_ON(!pud_present(pud_mknotpresent(pud_mkhuge(pud)))) -> Fail >>> WARN_ON(!pmd_present(pmd_mknotpresent(pmd_mkhuge(pmd)))) -> Pass >>> >>> Though pud_mknotpresent() currently clears _PAGE_PROTNONE, pud_present() >>> does not check it. This change fixes both inconsistencies. >>> >>> This has been build and boot tested on x86. >> >> Adding Kirill and Dan. >> >> +Cc: Kirill A. Shutemov <kirill@shutemov.name> >> +Cc: Dan Williams <dan.j.williams@intel.com> > > Or we can just drop the pud_mknotpresent(). There's no users AFAICS and > only x86 provides it. Yes that will be an option but IMHO fixing pud_present() here might be a better choice because, (1) pud_mknotpresent() with fixed pud_present() might be required later (2) PMD & PUD will be exact same (THP is supported on either level) Nonetheless, I am happy to go either way.
On 3/20/20 6:22 AM, Anshuman Khandual wrote: ... >>> +Cc: Kirill A. Shutemov <kirill@shutemov.name> >>> +Cc: Dan Williams <dan.j.williams@intel.com> >> >> Or we can just drop the pud_mknotpresent(). There's no users AFAICS and >> only x86 provides it. +1 > > Yes that will be an option but IMHO fixing pud_present() here might be > a better choice because, > > (1) pud_mknotpresent() with fixed pud_present() might be required later It might. Or it might not. Let's wait until it's actually used, and see. Dead code is an avoidable expense (adds size, space on the screen, email traffic and other wasted time), so let's avoid it here. > (2) PMD & PUD will be exact same (THP is supported on either level) > > Nonetheless, I am happy to go either way. > thanks,
On 03/21/2020 01:20 AM, John Hubbard wrote: > On 3/20/20 6:22 AM, Anshuman Khandual wrote: > ... >>>> +Cc: Kirill A. Shutemov <kirill@shutemov.name> >>>> +Cc: Dan Williams <dan.j.williams@intel.com> >>> >>> Or we can just drop the pud_mknotpresent(). There's no users AFAICS and >>> only x86 provides it. > > +1 > >> >> Yes that will be an option but IMHO fixing pud_present() here might be >> a better choice because, >> >> (1) pud_mknotpresent() with fixed pud_present() might be required later > > > It might. Or it might not. Let's wait until it's actually used, and see. > Dead code is an avoidable expense (adds size, space on the screen, email > traffic and other wasted time), so let's avoid it here. Sure, will resend with pud_mknotpresent() dropped. > > >> (2) PMD & PUD will be exact same (THP is supported on either level) >> >> Nonetheless, I am happy to go either way. >> > > > thanks,
diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h index 7e118660bbd9..8adf1d00b506 100644 --- a/arch/x86/include/asm/pgtable.h +++ b/arch/x86/include/asm/pgtable.h @@ -857,7 +857,13 @@ static inline int pud_none(pud_t pud) static inline int pud_present(pud_t pud) { - return pud_flags(pud) & _PAGE_PRESENT; + /* + * Checking for _PAGE_PSE is needed too because + * split_huge_page will temporarily clear the present bit (but + * the _PAGE_PSE flag will remain set at all times while the + * _PAGE_PRESENT bit is clear). + */ + return pud_flags(pud) & (_PAGE_PRESENT | _PAGE_PROTNONE | _PAGE_PSE); } static inline unsigned long pud_page_vaddr(pud_t pud)
pud_present() should also check _PAGE_PROTNONE and _PAGE_PSE bits like in case pmd_present(). This makes a PUD entry test positive for pud_present() after getting invalidated with pud_mknotpresent(), hence standardizing the semantics with PMD helpers. Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Ingo Molnar <mingo@redhat.com> Cc: Borislav Petkov <bp@alien8.de> Cc: "H. Peter Anvin" <hpa@zytor.com> Cc: Dave Hansen <dave.hansen@intel.com> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: x86@kernel.org Cc: linux-mm@kvack.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com> --- Even though pud_mknotpresent() is not used any where currently, there is a discrepancy between PMD and PUD. WARN_ON(!pud_present(pud_mknotpresent(pud_mkhuge(pud)))) -> Fail WARN_ON(!pmd_present(pmd_mknotpresent(pmd_mkhuge(pmd)))) -> Pass Though pud_mknotpresent() currently clears _PAGE_PROTNONE, pud_present() does not check it. This change fixes both inconsistencies. This has been build and boot tested on x86. arch/x86/include/asm/pgtable.h | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)