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 <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: Dave Hansen <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Anshuman Khandual <[email protected]>
---
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(-)
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)
--
2.17.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 <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Borislav Petkov <[email protected]>
> Cc: "H. Peter Anvin" <[email protected]>
> Cc: Dave Hansen <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Anshuman Khandual <[email protected]>
> ---
> 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 <[email protected]>
+Cc: Dan Williams <[email protected]>
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 <[email protected]>
> > Cc: Ingo Molnar <[email protected]>
> > Cc: Borislav Petkov <[email protected]>
> > Cc: "H. Peter Anvin" <[email protected]>
> > Cc: Dave Hansen <[email protected]>
> > Cc: Andrew Morton <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > Signed-off-by: Anshuman Khandual <[email protected]>
> > ---
> > 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 <[email protected]>
> +Cc: Dan Williams <[email protected]>
Or we can just drop the pud_mknotpresent(). There's no users AFAICS and
only x86 provides it.
--
Kirill A. Shutemov
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 <[email protected]>
>>> Cc: Ingo Molnar <[email protected]>
>>> Cc: Borislav Petkov <[email protected]>
>>> Cc: "H. Peter Anvin" <[email protected]>
>>> Cc: Dave Hansen <[email protected]>
>>> Cc: Andrew Morton <[email protected]>
>>> Cc: [email protected]
>>> Cc: [email protected]
>>> Cc: [email protected]
>>> Signed-off-by: Anshuman Khandual <[email protected]>
>>> ---
>>> 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 <[email protected]>
>> +Cc: Dan Williams <[email protected]>
>
> 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 <[email protected]>
>>> +Cc: Dan Williams <[email protected]>
>>
>> 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,
--
John Hubbard
NVIDIA
On 03/21/2020 01:20 AM, John Hubbard wrote:
> On 3/20/20 6:22 AM, Anshuman Khandual wrote:
> ...
>>>> +Cc: Kirill A. Shutemov <[email protected]>
>>>> +Cc: Dan Williams <[email protected]>
>>>
>>> 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,