diff mbox series

x86/mm: Make pud_present() check _PAGE_PROTNONE and _PAGE_PSE as well

Message ID 1584507679-11976-1-git-send-email-anshuman.khandual@arm.com
State New, archived
Headers show
Series x86/mm: Make pud_present() check _PAGE_PROTNONE and _PAGE_PSE as well | expand

Commit Message

Anshuman Khandual March 18, 2020, 5:01 a.m. UTC
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(-)

Comments

Anshuman Khandual March 20, 2020, 3:23 a.m. UTC | #1
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>
Kirill A. Shutemov March 20, 2020, 11:47 a.m. UTC | #2
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.
Anshuman Khandual March 20, 2020, 1:22 p.m. UTC | #3
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.
John Hubbard March 20, 2020, 7:50 p.m. UTC | #4
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,
Anshuman Khandual March 22, 2020, 1 a.m. UTC | #5
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 mbox series

Patch

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)