* [PATCH 1/2] arm64/mm: Change THP helpers to comply with generic MM semantics
@ 2020-08-17 9:19 ` Anshuman Khandual
0 siblings, 0 replies; 26+ messages in thread
From: Anshuman Khandual @ 2020-08-17 9:19 UTC (permalink / raw)
To: linux-mm, linux-arm-kernel
Cc: Mark Rutland, Anshuman Khandual, catalin.marinas, Suzuki Poulose,
linux-kernel, Marc Zyngier, akpm, will
pmd_present() and pmd_trans_huge() are expected to behave in the following
manner during various phases of a given PMD. It is derived from a previous
detailed discussion on this topic [1] and present THP documentation [2].
pmd_present(pmd):
- Returns true if pmd refers to system RAM with a valid pmd_page(pmd)
- Returns false if pmd does not refer to system RAM - Invalid pmd_page(pmd)
pmd_trans_huge(pmd):
- Returns true if pmd refers to system RAM and is a trans huge mapping
-------------------------------------------------------------------------
| PMD states | pmd_present | pmd_trans_huge |
-------------------------------------------------------------------------
| Mapped | Yes | Yes |
-------------------------------------------------------------------------
| Splitting | Yes | Yes |
-------------------------------------------------------------------------
| Migration/Swap | No | No |
-------------------------------------------------------------------------
The problem:
PMD is first invalidated with pmdp_invalidate() before it's splitting. This
invalidation clears PMD_SECT_VALID as below.
PMD Split -> pmdp_invalidate() -> pmd_mkinvalid -> Clears PMD_SECT_VALID
Once PMD_SECT_VALID gets cleared, it results in pmd_present() return false
on the PMD entry. It will need another bit apart from PMD_SECT_VALID to re-
affirm pmd_present() as true during the THP split process. To comply with
above mentioned semantics, pmd_trans_huge() should also check pmd_present()
first before testing presence of an actual transparent huge mapping.
The solution:
Ideally PMD_TYPE_SECT should have been used here instead. But it shares the
bit position with PMD_SECT_VALID which is used for THP invalidation. Hence
it will not be there for pmd_present() check after pmdp_invalidate().
A new software defined PMD_PRESENT_INVALID (bit 59) can be set on the PMD
entry during invalidation which can help pmd_present() return true and in
recognizing the fact that it still points to memory.
This bit is transient. During the split process it will be overridden by a
page table page representing normal pages in place of erstwhile huge page.
Other pmdp_invalidate() callers always write a fresh PMD value on the entry
overriding this transient PMD_PRESENT_INVALID bit, which makes it safe.
[1]: https://lkml.org/lkml/2018/10/17/231
[2]: https://www.kernel.org/doc/Documentation/vm/transhuge.txt
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Suzuki Poulose <suzuki.poulose@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
arch/arm64/include/asm/pgtable-prot.h | 7 ++++++
arch/arm64/include/asm/pgtable.h | 34 ++++++++++++++++++++++++---
2 files changed, 38 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/include/asm/pgtable-prot.h b/arch/arm64/include/asm/pgtable-prot.h
index 4d867c6446c4..28792fdd9627 100644
--- a/arch/arm64/include/asm/pgtable-prot.h
+++ b/arch/arm64/include/asm/pgtable-prot.h
@@ -19,6 +19,13 @@
#define PTE_DEVMAP (_AT(pteval_t, 1) << 57)
#define PTE_PROT_NONE (_AT(pteval_t, 1) << 58) /* only when !PTE_VALID */
+/*
+ * This help indicate that the entry is present i.e pmd_page()
+ * still points to a valid huge page in memory even if the pmd
+ * has been invalidated.
+ */
+#define PMD_PRESENT_INVALID (_AT(pteval_t, 1) << 59) /* only when !PMD_SECT_VALID */
+
#ifndef __ASSEMBLY__
#include <asm/cpufeature.h>
diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index d5d3fbe73953..7aa69cace784 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -145,6 +145,18 @@ static inline pte_t set_pte_bit(pte_t pte, pgprot_t prot)
return pte;
}
+static inline pmd_t clr_pmd_bit(pmd_t pmd, pgprot_t prot)
+{
+ pmd_val(pmd) &= ~pgprot_val(prot);
+ return pmd;
+}
+
+static inline pmd_t set_pmd_bit(pmd_t pmd, pgprot_t prot)
+{
+ pmd_val(pmd) |= pgprot_val(prot);
+ return pmd;
+}
+
static inline pte_t pte_wrprotect(pte_t pte)
{
pte = clear_pte_bit(pte, __pgprot(PTE_WRITE));
@@ -363,15 +375,24 @@ static inline int pmd_protnone(pmd_t pmd)
}
#endif
+#define pmd_present_invalid(pmd) (!!(pmd_val(pmd) & PMD_PRESENT_INVALID))
+
+static inline int pmd_present(pmd_t pmd)
+{
+ return pte_present(pmd_pte(pmd)) || pmd_present_invalid(pmd);
+}
+
/*
* THP definitions.
*/
#ifdef CONFIG_TRANSPARENT_HUGEPAGE
-#define pmd_trans_huge(pmd) (pmd_val(pmd) && !(pmd_val(pmd) & PMD_TABLE_BIT))
+static inline int pmd_trans_huge(pmd_t pmd)
+{
+ return pmd_val(pmd) && pmd_present(pmd) && !(pmd_val(pmd) & PMD_TABLE_BIT);
+}
#endif /* CONFIG_TRANSPARENT_HUGEPAGE */
-#define pmd_present(pmd) pte_present(pmd_pte(pmd))
#define pmd_dirty(pmd) pte_dirty(pmd_pte(pmd))
#define pmd_young(pmd) pte_young(pmd_pte(pmd))
#define pmd_valid(pmd) pte_valid(pmd_pte(pmd))
@@ -381,7 +402,14 @@ static inline int pmd_protnone(pmd_t pmd)
#define pmd_mkclean(pmd) pte_pmd(pte_mkclean(pmd_pte(pmd)))
#define pmd_mkdirty(pmd) pte_pmd(pte_mkdirty(pmd_pte(pmd)))
#define pmd_mkyoung(pmd) pte_pmd(pte_mkyoung(pmd_pte(pmd)))
-#define pmd_mkinvalid(pmd) (__pmd(pmd_val(pmd) & ~PMD_SECT_VALID))
+
+static inline pmd_t pmd_mkinvalid(pmd_t pmd)
+{
+ pmd = set_pmd_bit(pmd, __pgprot(PMD_PRESENT_INVALID));
+ pmd = clr_pmd_bit(pmd, __pgprot(PMD_SECT_VALID));
+
+ return pmd;
+}
#define pmd_thp_or_huge(pmd) (pmd_huge(pmd) || pmd_trans_huge(pmd))
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 1/2] arm64/mm: Change THP helpers to comply with generic MM semantics
2020-08-17 9:19 ` Anshuman Khandual
@ 2020-08-18 9:13 ` Jonathan Cameron
-1 siblings, 0 replies; 26+ messages in thread
From: Jonathan Cameron @ 2020-08-18 9:13 UTC (permalink / raw)
To: Anshuman Khandual
Cc: linux-mm, linux-arm-kernel, catalin.marinas, will, akpm,
Mark Rutland, Marc Zyngier, Suzuki Poulose, linux-kernel
On Mon, 17 Aug 2020 14:49:43 +0530
Anshuman Khandual <anshuman.khandual@arm.com> wrote:
> pmd_present() and pmd_trans_huge() are expected to behave in the following
> manner during various phases of a given PMD. It is derived from a previous
> detailed discussion on this topic [1] and present THP documentation [2].
>
> pmd_present(pmd):
>
> - Returns true if pmd refers to system RAM with a valid pmd_page(pmd)
> - Returns false if pmd does not refer to system RAM - Invalid pmd_page(pmd)
>
> pmd_trans_huge(pmd):
>
> - Returns true if pmd refers to system RAM and is a trans huge mapping
>
> -------------------------------------------------------------------------
> | PMD states | pmd_present | pmd_trans_huge |
> -------------------------------------------------------------------------
> | Mapped | Yes | Yes |
> -------------------------------------------------------------------------
> | Splitting | Yes | Yes |
> -------------------------------------------------------------------------
> | Migration/Swap | No | No |
> -------------------------------------------------------------------------
>
> The problem:
>
> PMD is first invalidated with pmdp_invalidate() before it's splitting. This
> invalidation clears PMD_SECT_VALID as below.
>
> PMD Split -> pmdp_invalidate() -> pmd_mkinvalid -> Clears PMD_SECT_VALID
>
> Once PMD_SECT_VALID gets cleared, it results in pmd_present() return false
> on the PMD entry. It will need another bit apart from PMD_SECT_VALID to re-
> affirm pmd_present() as true during the THP split process. To comply with
> above mentioned semantics, pmd_trans_huge() should also check pmd_present()
> first before testing presence of an actual transparent huge mapping.
>
> The solution:
>
> Ideally PMD_TYPE_SECT should have been used here instead. But it shares the
> bit position with PMD_SECT_VALID which is used for THP invalidation. Hence
> it will not be there for pmd_present() check after pmdp_invalidate().
>
> A new software defined PMD_PRESENT_INVALID (bit 59) can be set on the PMD
> entry during invalidation which can help pmd_present() return true and in
> recognizing the fact that it still points to memory.
>
> This bit is transient. During the split process it will be overridden by a
> page table page representing normal pages in place of erstwhile huge page.
> Other pmdp_invalidate() callers always write a fresh PMD value on the entry
> overriding this transient PMD_PRESENT_INVALID bit, which makes it safe.
>
> [1]: https://lkml.org/lkml/2018/10/17/231
> [2]: https://www.kernel.org/doc/Documentation/vm/transhuge.txt
Hi Anshuman,
One query on this. From my reading of the ARM ARM, bit 59 is not
an ignored bit. The exact requirements for hardware to be using
it are a bit complex though.
It 'might' be safe to use it for this, but if so can we have a comment
explaining why. Also more than possible I'm misunderstanding things!
Thanks,
Jonathan
>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Suzuki Poulose <suzuki.poulose@arm.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
> arch/arm64/include/asm/pgtable-prot.h | 7 ++++++
> arch/arm64/include/asm/pgtable.h | 34 ++++++++++++++++++++++++---
> 2 files changed, 38 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/include/asm/pgtable-prot.h b/arch/arm64/include/asm/pgtable-prot.h
> index 4d867c6446c4..28792fdd9627 100644
> --- a/arch/arm64/include/asm/pgtable-prot.h
> +++ b/arch/arm64/include/asm/pgtable-prot.h
> @@ -19,6 +19,13 @@
> #define PTE_DEVMAP (_AT(pteval_t, 1) << 57)
> #define PTE_PROT_NONE (_AT(pteval_t, 1) << 58) /* only when !PTE_VALID */
>
> +/*
> + * This help indicate that the entry is present i.e pmd_page()
> + * still points to a valid huge page in memory even if the pmd
> + * has been invalidated.
> + */
> +#define PMD_PRESENT_INVALID (_AT(pteval_t, 1) << 59) /* only when !PMD_SECT_VALID */
> +
> #ifndef __ASSEMBLY__
>
> #include <asm/cpufeature.h>
> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> index d5d3fbe73953..7aa69cace784 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -145,6 +145,18 @@ static inline pte_t set_pte_bit(pte_t pte, pgprot_t prot)
> return pte;
> }
>
> +static inline pmd_t clr_pmd_bit(pmd_t pmd, pgprot_t prot)
> +{
> + pmd_val(pmd) &= ~pgprot_val(prot);
> + return pmd;
> +}
> +
> +static inline pmd_t set_pmd_bit(pmd_t pmd, pgprot_t prot)
> +{
> + pmd_val(pmd) |= pgprot_val(prot);
> + return pmd;
> +}
> +
> static inline pte_t pte_wrprotect(pte_t pte)
> {
> pte = clear_pte_bit(pte, __pgprot(PTE_WRITE));
> @@ -363,15 +375,24 @@ static inline int pmd_protnone(pmd_t pmd)
> }
> #endif
>
> +#define pmd_present_invalid(pmd) (!!(pmd_val(pmd) & PMD_PRESENT_INVALID))
> +
> +static inline int pmd_present(pmd_t pmd)
> +{
> + return pte_present(pmd_pte(pmd)) || pmd_present_invalid(pmd);
> +}
> +
> /*
> * THP definitions.
> */
>
> #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> -#define pmd_trans_huge(pmd) (pmd_val(pmd) && !(pmd_val(pmd) & PMD_TABLE_BIT))
> +static inline int pmd_trans_huge(pmd_t pmd)
> +{
> + return pmd_val(pmd) && pmd_present(pmd) && !(pmd_val(pmd) & PMD_TABLE_BIT);
> +}
> #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
>
> -#define pmd_present(pmd) pte_present(pmd_pte(pmd))
> #define pmd_dirty(pmd) pte_dirty(pmd_pte(pmd))
> #define pmd_young(pmd) pte_young(pmd_pte(pmd))
> #define pmd_valid(pmd) pte_valid(pmd_pte(pmd))
> @@ -381,7 +402,14 @@ static inline int pmd_protnone(pmd_t pmd)
> #define pmd_mkclean(pmd) pte_pmd(pte_mkclean(pmd_pte(pmd)))
> #define pmd_mkdirty(pmd) pte_pmd(pte_mkdirty(pmd_pte(pmd)))
> #define pmd_mkyoung(pmd) pte_pmd(pte_mkyoung(pmd_pte(pmd)))
> -#define pmd_mkinvalid(pmd) (__pmd(pmd_val(pmd) & ~PMD_SECT_VALID))
> +
> +static inline pmd_t pmd_mkinvalid(pmd_t pmd)
> +{
> + pmd = set_pmd_bit(pmd, __pgprot(PMD_PRESENT_INVALID));
> + pmd = clr_pmd_bit(pmd, __pgprot(PMD_SECT_VALID));
> +
> + return pmd;
> +}
>
> #define pmd_thp_or_huge(pmd) (pmd_huge(pmd) || pmd_trans_huge(pmd))
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 1/2] arm64/mm: Change THP helpers to comply with generic MM semantics
@ 2020-08-18 9:13 ` Jonathan Cameron
0 siblings, 0 replies; 26+ messages in thread
From: Jonathan Cameron @ 2020-08-18 9:13 UTC (permalink / raw)
To: Anshuman Khandual
Cc: Mark Rutland, Suzuki Poulose, catalin.marinas, linux-kernel,
linux-mm, Marc Zyngier, akpm, will, linux-arm-kernel
On Mon, 17 Aug 2020 14:49:43 +0530
Anshuman Khandual <anshuman.khandual@arm.com> wrote:
> pmd_present() and pmd_trans_huge() are expected to behave in the following
> manner during various phases of a given PMD. It is derived from a previous
> detailed discussion on this topic [1] and present THP documentation [2].
>
> pmd_present(pmd):
>
> - Returns true if pmd refers to system RAM with a valid pmd_page(pmd)
> - Returns false if pmd does not refer to system RAM - Invalid pmd_page(pmd)
>
> pmd_trans_huge(pmd):
>
> - Returns true if pmd refers to system RAM and is a trans huge mapping
>
> -------------------------------------------------------------------------
> | PMD states | pmd_present | pmd_trans_huge |
> -------------------------------------------------------------------------
> | Mapped | Yes | Yes |
> -------------------------------------------------------------------------
> | Splitting | Yes | Yes |
> -------------------------------------------------------------------------
> | Migration/Swap | No | No |
> -------------------------------------------------------------------------
>
> The problem:
>
> PMD is first invalidated with pmdp_invalidate() before it's splitting. This
> invalidation clears PMD_SECT_VALID as below.
>
> PMD Split -> pmdp_invalidate() -> pmd_mkinvalid -> Clears PMD_SECT_VALID
>
> Once PMD_SECT_VALID gets cleared, it results in pmd_present() return false
> on the PMD entry. It will need another bit apart from PMD_SECT_VALID to re-
> affirm pmd_present() as true during the THP split process. To comply with
> above mentioned semantics, pmd_trans_huge() should also check pmd_present()
> first before testing presence of an actual transparent huge mapping.
>
> The solution:
>
> Ideally PMD_TYPE_SECT should have been used here instead. But it shares the
> bit position with PMD_SECT_VALID which is used for THP invalidation. Hence
> it will not be there for pmd_present() check after pmdp_invalidate().
>
> A new software defined PMD_PRESENT_INVALID (bit 59) can be set on the PMD
> entry during invalidation which can help pmd_present() return true and in
> recognizing the fact that it still points to memory.
>
> This bit is transient. During the split process it will be overridden by a
> page table page representing normal pages in place of erstwhile huge page.
> Other pmdp_invalidate() callers always write a fresh PMD value on the entry
> overriding this transient PMD_PRESENT_INVALID bit, which makes it safe.
>
> [1]: https://lkml.org/lkml/2018/10/17/231
> [2]: https://www.kernel.org/doc/Documentation/vm/transhuge.txt
Hi Anshuman,
One query on this. From my reading of the ARM ARM, bit 59 is not
an ignored bit. The exact requirements for hardware to be using
it are a bit complex though.
It 'might' be safe to use it for this, but if so can we have a comment
explaining why. Also more than possible I'm misunderstanding things!
Thanks,
Jonathan
>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Suzuki Poulose <suzuki.poulose@arm.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
> arch/arm64/include/asm/pgtable-prot.h | 7 ++++++
> arch/arm64/include/asm/pgtable.h | 34 ++++++++++++++++++++++++---
> 2 files changed, 38 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/include/asm/pgtable-prot.h b/arch/arm64/include/asm/pgtable-prot.h
> index 4d867c6446c4..28792fdd9627 100644
> --- a/arch/arm64/include/asm/pgtable-prot.h
> +++ b/arch/arm64/include/asm/pgtable-prot.h
> @@ -19,6 +19,13 @@
> #define PTE_DEVMAP (_AT(pteval_t, 1) << 57)
> #define PTE_PROT_NONE (_AT(pteval_t, 1) << 58) /* only when !PTE_VALID */
>
> +/*
> + * This help indicate that the entry is present i.e pmd_page()
> + * still points to a valid huge page in memory even if the pmd
> + * has been invalidated.
> + */
> +#define PMD_PRESENT_INVALID (_AT(pteval_t, 1) << 59) /* only when !PMD_SECT_VALID */
> +
> #ifndef __ASSEMBLY__
>
> #include <asm/cpufeature.h>
> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> index d5d3fbe73953..7aa69cace784 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -145,6 +145,18 @@ static inline pte_t set_pte_bit(pte_t pte, pgprot_t prot)
> return pte;
> }
>
> +static inline pmd_t clr_pmd_bit(pmd_t pmd, pgprot_t prot)
> +{
> + pmd_val(pmd) &= ~pgprot_val(prot);
> + return pmd;
> +}
> +
> +static inline pmd_t set_pmd_bit(pmd_t pmd, pgprot_t prot)
> +{
> + pmd_val(pmd) |= pgprot_val(prot);
> + return pmd;
> +}
> +
> static inline pte_t pte_wrprotect(pte_t pte)
> {
> pte = clear_pte_bit(pte, __pgprot(PTE_WRITE));
> @@ -363,15 +375,24 @@ static inline int pmd_protnone(pmd_t pmd)
> }
> #endif
>
> +#define pmd_present_invalid(pmd) (!!(pmd_val(pmd) & PMD_PRESENT_INVALID))
> +
> +static inline int pmd_present(pmd_t pmd)
> +{
> + return pte_present(pmd_pte(pmd)) || pmd_present_invalid(pmd);
> +}
> +
> /*
> * THP definitions.
> */
>
> #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> -#define pmd_trans_huge(pmd) (pmd_val(pmd) && !(pmd_val(pmd) & PMD_TABLE_BIT))
> +static inline int pmd_trans_huge(pmd_t pmd)
> +{
> + return pmd_val(pmd) && pmd_present(pmd) && !(pmd_val(pmd) & PMD_TABLE_BIT);
> +}
> #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
>
> -#define pmd_present(pmd) pte_present(pmd_pte(pmd))
> #define pmd_dirty(pmd) pte_dirty(pmd_pte(pmd))
> #define pmd_young(pmd) pte_young(pmd_pte(pmd))
> #define pmd_valid(pmd) pte_valid(pmd_pte(pmd))
> @@ -381,7 +402,14 @@ static inline int pmd_protnone(pmd_t pmd)
> #define pmd_mkclean(pmd) pte_pmd(pte_mkclean(pmd_pte(pmd)))
> #define pmd_mkdirty(pmd) pte_pmd(pte_mkdirty(pmd_pte(pmd)))
> #define pmd_mkyoung(pmd) pte_pmd(pte_mkyoung(pmd_pte(pmd)))
> -#define pmd_mkinvalid(pmd) (__pmd(pmd_val(pmd) & ~PMD_SECT_VALID))
> +
> +static inline pmd_t pmd_mkinvalid(pmd_t pmd)
> +{
> + pmd = set_pmd_bit(pmd, __pgprot(PMD_PRESENT_INVALID));
> + pmd = clr_pmd_bit(pmd, __pgprot(PMD_SECT_VALID));
> +
> + return pmd;
> +}
>
> #define pmd_thp_or_huge(pmd) (pmd_huge(pmd) || pmd_trans_huge(pmd))
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 1/2] arm64/mm: Change THP helpers to comply with generic MM semantics
2020-08-18 9:13 ` Jonathan Cameron
@ 2020-08-18 9:41 ` Anshuman Khandual
-1 siblings, 0 replies; 26+ messages in thread
From: Anshuman Khandual @ 2020-08-18 9:41 UTC (permalink / raw)
To: Jonathan Cameron
Cc: linux-mm, linux-arm-kernel, catalin.marinas, will, akpm,
Mark Rutland, Marc Zyngier, Suzuki Poulose, linux-kernel
On 08/18/2020 02:43 PM, Jonathan Cameron wrote:
> On Mon, 17 Aug 2020 14:49:43 +0530
> Anshuman Khandual <anshuman.khandual@arm.com> wrote:
>
>> pmd_present() and pmd_trans_huge() are expected to behave in the following
>> manner during various phases of a given PMD. It is derived from a previous
>> detailed discussion on this topic [1] and present THP documentation [2].
>>
>> pmd_present(pmd):
>>
>> - Returns true if pmd refers to system RAM with a valid pmd_page(pmd)
>> - Returns false if pmd does not refer to system RAM - Invalid pmd_page(pmd)
>>
>> pmd_trans_huge(pmd):
>>
>> - Returns true if pmd refers to system RAM and is a trans huge mapping
>>
>> -------------------------------------------------------------------------
>> | PMD states | pmd_present | pmd_trans_huge |
>> -------------------------------------------------------------------------
>> | Mapped | Yes | Yes |
>> -------------------------------------------------------------------------
>> | Splitting | Yes | Yes |
>> -------------------------------------------------------------------------
>> | Migration/Swap | No | No |
>> -------------------------------------------------------------------------
>>
>> The problem:
>>
>> PMD is first invalidated with pmdp_invalidate() before it's splitting. This
>> invalidation clears PMD_SECT_VALID as below.
>>
>> PMD Split -> pmdp_invalidate() -> pmd_mkinvalid -> Clears PMD_SECT_VALID
>>
>> Once PMD_SECT_VALID gets cleared, it results in pmd_present() return false
>> on the PMD entry. It will need another bit apart from PMD_SECT_VALID to re-
>> affirm pmd_present() as true during the THP split process. To comply with
>> above mentioned semantics, pmd_trans_huge() should also check pmd_present()
>> first before testing presence of an actual transparent huge mapping.
>>
>> The solution:
>>
>> Ideally PMD_TYPE_SECT should have been used here instead. But it shares the
>> bit position with PMD_SECT_VALID which is used for THP invalidation. Hence
>> it will not be there for pmd_present() check after pmdp_invalidate().
>>
>> A new software defined PMD_PRESENT_INVALID (bit 59) can be set on the PMD
>> entry during invalidation which can help pmd_present() return true and in
>> recognizing the fact that it still points to memory.
>>
>> This bit is transient. During the split process it will be overridden by a
>> page table page representing normal pages in place of erstwhile huge page.
>> Other pmdp_invalidate() callers always write a fresh PMD value on the entry
>> overriding this transient PMD_PRESENT_INVALID bit, which makes it safe.
>>
>> [1]: https://lkml.org/lkml/2018/10/17/231
>> [2]: https://www.kernel.org/doc/Documentation/vm/transhuge.txt
>
> Hi Anshuman,
>
> One query on this. From my reading of the ARM ARM, bit 59 is not
> an ignored bit. The exact requirements for hardware to be using
> it are a bit complex though.
>
> It 'might' be safe to use it for this, but if so can we have a comment
> explaining why. Also more than possible I'm misunderstanding things!
We are using this bit 59 only when the entry is not active from MMU
perspective i.e PMD_SECT_VALID is clear.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] arm64/mm: Change THP helpers to comply with generic MM semantics
@ 2020-08-18 9:41 ` Anshuman Khandual
0 siblings, 0 replies; 26+ messages in thread
From: Anshuman Khandual @ 2020-08-18 9:41 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Mark Rutland, Suzuki Poulose, catalin.marinas, linux-kernel,
linux-mm, Marc Zyngier, akpm, will, linux-arm-kernel
On 08/18/2020 02:43 PM, Jonathan Cameron wrote:
> On Mon, 17 Aug 2020 14:49:43 +0530
> Anshuman Khandual <anshuman.khandual@arm.com> wrote:
>
>> pmd_present() and pmd_trans_huge() are expected to behave in the following
>> manner during various phases of a given PMD. It is derived from a previous
>> detailed discussion on this topic [1] and present THP documentation [2].
>>
>> pmd_present(pmd):
>>
>> - Returns true if pmd refers to system RAM with a valid pmd_page(pmd)
>> - Returns false if pmd does not refer to system RAM - Invalid pmd_page(pmd)
>>
>> pmd_trans_huge(pmd):
>>
>> - Returns true if pmd refers to system RAM and is a trans huge mapping
>>
>> -------------------------------------------------------------------------
>> | PMD states | pmd_present | pmd_trans_huge |
>> -------------------------------------------------------------------------
>> | Mapped | Yes | Yes |
>> -------------------------------------------------------------------------
>> | Splitting | Yes | Yes |
>> -------------------------------------------------------------------------
>> | Migration/Swap | No | No |
>> -------------------------------------------------------------------------
>>
>> The problem:
>>
>> PMD is first invalidated with pmdp_invalidate() before it's splitting. This
>> invalidation clears PMD_SECT_VALID as below.
>>
>> PMD Split -> pmdp_invalidate() -> pmd_mkinvalid -> Clears PMD_SECT_VALID
>>
>> Once PMD_SECT_VALID gets cleared, it results in pmd_present() return false
>> on the PMD entry. It will need another bit apart from PMD_SECT_VALID to re-
>> affirm pmd_present() as true during the THP split process. To comply with
>> above mentioned semantics, pmd_trans_huge() should also check pmd_present()
>> first before testing presence of an actual transparent huge mapping.
>>
>> The solution:
>>
>> Ideally PMD_TYPE_SECT should have been used here instead. But it shares the
>> bit position with PMD_SECT_VALID which is used for THP invalidation. Hence
>> it will not be there for pmd_present() check after pmdp_invalidate().
>>
>> A new software defined PMD_PRESENT_INVALID (bit 59) can be set on the PMD
>> entry during invalidation which can help pmd_present() return true and in
>> recognizing the fact that it still points to memory.
>>
>> This bit is transient. During the split process it will be overridden by a
>> page table page representing normal pages in place of erstwhile huge page.
>> Other pmdp_invalidate() callers always write a fresh PMD value on the entry
>> overriding this transient PMD_PRESENT_INVALID bit, which makes it safe.
>>
>> [1]: https://lkml.org/lkml/2018/10/17/231
>> [2]: https://www.kernel.org/doc/Documentation/vm/transhuge.txt
>
> Hi Anshuman,
>
> One query on this. From my reading of the ARM ARM, bit 59 is not
> an ignored bit. The exact requirements for hardware to be using
> it are a bit complex though.
>
> It 'might' be safe to use it for this, but if so can we have a comment
> explaining why. Also more than possible I'm misunderstanding things!
We are using this bit 59 only when the entry is not active from MMU
perspective i.e PMD_SECT_VALID is clear.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] arm64/mm: Change THP helpers to comply with generic MM semantics
2020-08-18 9:41 ` Anshuman Khandual
@ 2020-08-18 12:26 ` Jonathan Cameron
-1 siblings, 0 replies; 26+ messages in thread
From: Jonathan Cameron @ 2020-08-18 12:26 UTC (permalink / raw)
To: Anshuman Khandual
Cc: linux-mm, linux-arm-kernel, catalin.marinas, will, akpm,
Mark Rutland, Marc Zyngier, Suzuki Poulose, linux-kernel
On Tue, 18 Aug 2020 15:11:58 +0530
Anshuman Khandual <anshuman.khandual@arm.com> wrote:
> On 08/18/2020 02:43 PM, Jonathan Cameron wrote:
> > On Mon, 17 Aug 2020 14:49:43 +0530
> > Anshuman Khandual <anshuman.khandual@arm.com> wrote:
> >
> >> pmd_present() and pmd_trans_huge() are expected to behave in the following
> >> manner during various phases of a given PMD. It is derived from a previous
> >> detailed discussion on this topic [1] and present THP documentation [2].
> >>
> >> pmd_present(pmd):
> >>
> >> - Returns true if pmd refers to system RAM with a valid pmd_page(pmd)
> >> - Returns false if pmd does not refer to system RAM - Invalid pmd_page(pmd)
> >>
> >> pmd_trans_huge(pmd):
> >>
> >> - Returns true if pmd refers to system RAM and is a trans huge mapping
> >>
> >> -------------------------------------------------------------------------
> >> | PMD states | pmd_present | pmd_trans_huge |
> >> -------------------------------------------------------------------------
> >> | Mapped | Yes | Yes |
> >> -------------------------------------------------------------------------
> >> | Splitting | Yes | Yes |
> >> -------------------------------------------------------------------------
> >> | Migration/Swap | No | No |
> >> -------------------------------------------------------------------------
> >>
> >> The problem:
> >>
> >> PMD is first invalidated with pmdp_invalidate() before it's splitting. This
> >> invalidation clears PMD_SECT_VALID as below.
> >>
> >> PMD Split -> pmdp_invalidate() -> pmd_mkinvalid -> Clears PMD_SECT_VALID
> >>
> >> Once PMD_SECT_VALID gets cleared, it results in pmd_present() return false
> >> on the PMD entry. It will need another bit apart from PMD_SECT_VALID to re-
> >> affirm pmd_present() as true during the THP split process. To comply with
> >> above mentioned semantics, pmd_trans_huge() should also check pmd_present()
> >> first before testing presence of an actual transparent huge mapping.
> >>
> >> The solution:
> >>
> >> Ideally PMD_TYPE_SECT should have been used here instead. But it shares the
> >> bit position with PMD_SECT_VALID which is used for THP invalidation. Hence
> >> it will not be there for pmd_present() check after pmdp_invalidate().
> >>
> >> A new software defined PMD_PRESENT_INVALID (bit 59) can be set on the PMD
> >> entry during invalidation which can help pmd_present() return true and in
> >> recognizing the fact that it still points to memory.
> >>
> >> This bit is transient. During the split process it will be overridden by a
> >> page table page representing normal pages in place of erstwhile huge page.
> >> Other pmdp_invalidate() callers always write a fresh PMD value on the entry
> >> overriding this transient PMD_PRESENT_INVALID bit, which makes it safe.
> >>
> >> [1]: https://lkml.org/lkml/2018/10/17/231
> >> [2]: https://www.kernel.org/doc/Documentation/vm/transhuge.txt
> >
> > Hi Anshuman,
> >
> > One query on this. From my reading of the ARM ARM, bit 59 is not
> > an ignored bit. The exact requirements for hardware to be using
> > it are a bit complex though.
> >
> > It 'might' be safe to use it for this, but if so can we have a comment
> > explaining why. Also more than possible I'm misunderstanding things!
>
> We are using this bit 59 only when the entry is not active from MMU
> perspective i.e PMD_SECT_VALID is clear.
>
Understood. I guess we ran out of bits that were always ignored so had
to start using ones that are ignored in this particular state.
Jonathan
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] arm64/mm: Change THP helpers to comply with generic MM semantics
@ 2020-08-18 12:26 ` Jonathan Cameron
0 siblings, 0 replies; 26+ messages in thread
From: Jonathan Cameron @ 2020-08-18 12:26 UTC (permalink / raw)
To: Anshuman Khandual
Cc: Mark Rutland, Suzuki Poulose, catalin.marinas, linux-kernel,
linux-mm, Marc Zyngier, akpm, will, linux-arm-kernel
On Tue, 18 Aug 2020 15:11:58 +0530
Anshuman Khandual <anshuman.khandual@arm.com> wrote:
> On 08/18/2020 02:43 PM, Jonathan Cameron wrote:
> > On Mon, 17 Aug 2020 14:49:43 +0530
> > Anshuman Khandual <anshuman.khandual@arm.com> wrote:
> >
> >> pmd_present() and pmd_trans_huge() are expected to behave in the following
> >> manner during various phases of a given PMD. It is derived from a previous
> >> detailed discussion on this topic [1] and present THP documentation [2].
> >>
> >> pmd_present(pmd):
> >>
> >> - Returns true if pmd refers to system RAM with a valid pmd_page(pmd)
> >> - Returns false if pmd does not refer to system RAM - Invalid pmd_page(pmd)
> >>
> >> pmd_trans_huge(pmd):
> >>
> >> - Returns true if pmd refers to system RAM and is a trans huge mapping
> >>
> >> -------------------------------------------------------------------------
> >> | PMD states | pmd_present | pmd_trans_huge |
> >> -------------------------------------------------------------------------
> >> | Mapped | Yes | Yes |
> >> -------------------------------------------------------------------------
> >> | Splitting | Yes | Yes |
> >> -------------------------------------------------------------------------
> >> | Migration/Swap | No | No |
> >> -------------------------------------------------------------------------
> >>
> >> The problem:
> >>
> >> PMD is first invalidated with pmdp_invalidate() before it's splitting. This
> >> invalidation clears PMD_SECT_VALID as below.
> >>
> >> PMD Split -> pmdp_invalidate() -> pmd_mkinvalid -> Clears PMD_SECT_VALID
> >>
> >> Once PMD_SECT_VALID gets cleared, it results in pmd_present() return false
> >> on the PMD entry. It will need another bit apart from PMD_SECT_VALID to re-
> >> affirm pmd_present() as true during the THP split process. To comply with
> >> above mentioned semantics, pmd_trans_huge() should also check pmd_present()
> >> first before testing presence of an actual transparent huge mapping.
> >>
> >> The solution:
> >>
> >> Ideally PMD_TYPE_SECT should have been used here instead. But it shares the
> >> bit position with PMD_SECT_VALID which is used for THP invalidation. Hence
> >> it will not be there for pmd_present() check after pmdp_invalidate().
> >>
> >> A new software defined PMD_PRESENT_INVALID (bit 59) can be set on the PMD
> >> entry during invalidation which can help pmd_present() return true and in
> >> recognizing the fact that it still points to memory.
> >>
> >> This bit is transient. During the split process it will be overridden by a
> >> page table page representing normal pages in place of erstwhile huge page.
> >> Other pmdp_invalidate() callers always write a fresh PMD value on the entry
> >> overriding this transient PMD_PRESENT_INVALID bit, which makes it safe.
> >>
> >> [1]: https://lkml.org/lkml/2018/10/17/231
> >> [2]: https://www.kernel.org/doc/Documentation/vm/transhuge.txt
> >
> > Hi Anshuman,
> >
> > One query on this. From my reading of the ARM ARM, bit 59 is not
> > an ignored bit. The exact requirements for hardware to be using
> > it are a bit complex though.
> >
> > It 'might' be safe to use it for this, but if so can we have a comment
> > explaining why. Also more than possible I'm misunderstanding things!
>
> We are using this bit 59 only when the entry is not active from MMU
> perspective i.e PMD_SECT_VALID is clear.
>
Understood. I guess we ran out of bits that were always ignored so had
to start using ones that are ignored in this particular state.
Jonathan
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] arm64/mm: Change THP helpers to comply with generic MM semantics
2020-08-18 12:26 ` Jonathan Cameron
@ 2020-08-19 9:10 ` Anshuman Khandual
-1 siblings, 0 replies; 26+ messages in thread
From: Anshuman Khandual @ 2020-08-19 9:10 UTC (permalink / raw)
To: Jonathan Cameron
Cc: linux-mm, linux-arm-kernel, catalin.marinas, will, akpm,
Mark Rutland, Marc Zyngier, Suzuki Poulose, linux-kernel
On 08/18/2020 05:56 PM, Jonathan Cameron wrote:
> On Tue, 18 Aug 2020 15:11:58 +0530
> Anshuman Khandual <anshuman.khandual@arm.com> wrote:
>
>> On 08/18/2020 02:43 PM, Jonathan Cameron wrote:
>>> On Mon, 17 Aug 2020 14:49:43 +0530
>>> Anshuman Khandual <anshuman.khandual@arm.com> wrote:
>>>
>>>> pmd_present() and pmd_trans_huge() are expected to behave in the following
>>>> manner during various phases of a given PMD. It is derived from a previous
>>>> detailed discussion on this topic [1] and present THP documentation [2].
>>>>
>>>> pmd_present(pmd):
>>>>
>>>> - Returns true if pmd refers to system RAM with a valid pmd_page(pmd)
>>>> - Returns false if pmd does not refer to system RAM - Invalid pmd_page(pmd)
>>>>
>>>> pmd_trans_huge(pmd):
>>>>
>>>> - Returns true if pmd refers to system RAM and is a trans huge mapping
>>>>
>>>> -------------------------------------------------------------------------
>>>> | PMD states | pmd_present | pmd_trans_huge |
>>>> -------------------------------------------------------------------------
>>>> | Mapped | Yes | Yes |
>>>> -------------------------------------------------------------------------
>>>> | Splitting | Yes | Yes |
>>>> -------------------------------------------------------------------------
>>>> | Migration/Swap | No | No |
>>>> -------------------------------------------------------------------------
>>>>
>>>> The problem:
>>>>
>>>> PMD is first invalidated with pmdp_invalidate() before it's splitting. This
>>>> invalidation clears PMD_SECT_VALID as below.
>>>>
>>>> PMD Split -> pmdp_invalidate() -> pmd_mkinvalid -> Clears PMD_SECT_VALID
>>>>
>>>> Once PMD_SECT_VALID gets cleared, it results in pmd_present() return false
>>>> on the PMD entry. It will need another bit apart from PMD_SECT_VALID to re-
>>>> affirm pmd_present() as true during the THP split process. To comply with
>>>> above mentioned semantics, pmd_trans_huge() should also check pmd_present()
>>>> first before testing presence of an actual transparent huge mapping.
>>>>
>>>> The solution:
>>>>
>>>> Ideally PMD_TYPE_SECT should have been used here instead. But it shares the
>>>> bit position with PMD_SECT_VALID which is used for THP invalidation. Hence
>>>> it will not be there for pmd_present() check after pmdp_invalidate().
>>>>
>>>> A new software defined PMD_PRESENT_INVALID (bit 59) can be set on the PMD
>>>> entry during invalidation which can help pmd_present() return true and in
>>>> recognizing the fact that it still points to memory.
>>>>
>>>> This bit is transient. During the split process it will be overridden by a
>>>> page table page representing normal pages in place of erstwhile huge page.
>>>> Other pmdp_invalidate() callers always write a fresh PMD value on the entry
>>>> overriding this transient PMD_PRESENT_INVALID bit, which makes it safe.
>>>>
>>>> [1]: https://lkml.org/lkml/2018/10/17/231
>>>> [2]: https://www.kernel.org/doc/Documentation/vm/transhuge.txt
>>>
>>> Hi Anshuman,
>>>
>>> One query on this. From my reading of the ARM ARM, bit 59 is not
>>> an ignored bit. The exact requirements for hardware to be using
>>> it are a bit complex though.
>>>
>>> It 'might' be safe to use it for this, but if so can we have a comment
>>> explaining why. Also more than possible I'm misunderstanding things!
>>
>> We are using this bit 59 only when the entry is not active from MMU
>> perspective i.e PMD_SECT_VALID is clear.
>>
>
> Understood. I guess we ran out of bits that were always ignored so had
> to start using ones that are ignored in this particular state.
Right, there are no more available SW PTE bits.
#define PTE_DIRTY (_AT(pteval_t, 1) << 55)
#define PTE_SPECIAL (_AT(pteval_t, 1) << 56)
#define PTE_DEVMAP (_AT(pteval_t, 1) << 57)
#define PTE_PROT_NONE (_AT(pteval_t, 1) << 58) /* only when !PTE_VALID */
Earlier I had proposed using PTE_SPECIAL at PMD level for this purpose.
But Catalin prefers these unused bits as the entry is anyway invalid
and which also leaves aside PTE_SPECIAL at mapped PMD for later use.
There is already one comment near PMD_PRESENT_INVALID definition which
explains this situation.
+/*
+ * This help indicate that the entry is present i.e pmd_page()
+ * still points to a valid huge page in memory even if the pmd
+ * has been invalidated.
+ */
+#define PMD_PRESENT_INVALID (_AT(pteval_t, 1) << 59) /* only when !PMD_SECT_VALID */
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] arm64/mm: Change THP helpers to comply with generic MM semantics
@ 2020-08-19 9:10 ` Anshuman Khandual
0 siblings, 0 replies; 26+ messages in thread
From: Anshuman Khandual @ 2020-08-19 9:10 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Mark Rutland, Suzuki Poulose, catalin.marinas, linux-kernel,
linux-mm, Marc Zyngier, akpm, will, linux-arm-kernel
On 08/18/2020 05:56 PM, Jonathan Cameron wrote:
> On Tue, 18 Aug 2020 15:11:58 +0530
> Anshuman Khandual <anshuman.khandual@arm.com> wrote:
>
>> On 08/18/2020 02:43 PM, Jonathan Cameron wrote:
>>> On Mon, 17 Aug 2020 14:49:43 +0530
>>> Anshuman Khandual <anshuman.khandual@arm.com> wrote:
>>>
>>>> pmd_present() and pmd_trans_huge() are expected to behave in the following
>>>> manner during various phases of a given PMD. It is derived from a previous
>>>> detailed discussion on this topic [1] and present THP documentation [2].
>>>>
>>>> pmd_present(pmd):
>>>>
>>>> - Returns true if pmd refers to system RAM with a valid pmd_page(pmd)
>>>> - Returns false if pmd does not refer to system RAM - Invalid pmd_page(pmd)
>>>>
>>>> pmd_trans_huge(pmd):
>>>>
>>>> - Returns true if pmd refers to system RAM and is a trans huge mapping
>>>>
>>>> -------------------------------------------------------------------------
>>>> | PMD states | pmd_present | pmd_trans_huge |
>>>> -------------------------------------------------------------------------
>>>> | Mapped | Yes | Yes |
>>>> -------------------------------------------------------------------------
>>>> | Splitting | Yes | Yes |
>>>> -------------------------------------------------------------------------
>>>> | Migration/Swap | No | No |
>>>> -------------------------------------------------------------------------
>>>>
>>>> The problem:
>>>>
>>>> PMD is first invalidated with pmdp_invalidate() before it's splitting. This
>>>> invalidation clears PMD_SECT_VALID as below.
>>>>
>>>> PMD Split -> pmdp_invalidate() -> pmd_mkinvalid -> Clears PMD_SECT_VALID
>>>>
>>>> Once PMD_SECT_VALID gets cleared, it results in pmd_present() return false
>>>> on the PMD entry. It will need another bit apart from PMD_SECT_VALID to re-
>>>> affirm pmd_present() as true during the THP split process. To comply with
>>>> above mentioned semantics, pmd_trans_huge() should also check pmd_present()
>>>> first before testing presence of an actual transparent huge mapping.
>>>>
>>>> The solution:
>>>>
>>>> Ideally PMD_TYPE_SECT should have been used here instead. But it shares the
>>>> bit position with PMD_SECT_VALID which is used for THP invalidation. Hence
>>>> it will not be there for pmd_present() check after pmdp_invalidate().
>>>>
>>>> A new software defined PMD_PRESENT_INVALID (bit 59) can be set on the PMD
>>>> entry during invalidation which can help pmd_present() return true and in
>>>> recognizing the fact that it still points to memory.
>>>>
>>>> This bit is transient. During the split process it will be overridden by a
>>>> page table page representing normal pages in place of erstwhile huge page.
>>>> Other pmdp_invalidate() callers always write a fresh PMD value on the entry
>>>> overriding this transient PMD_PRESENT_INVALID bit, which makes it safe.
>>>>
>>>> [1]: https://lkml.org/lkml/2018/10/17/231
>>>> [2]: https://www.kernel.org/doc/Documentation/vm/transhuge.txt
>>>
>>> Hi Anshuman,
>>>
>>> One query on this. From my reading of the ARM ARM, bit 59 is not
>>> an ignored bit. The exact requirements for hardware to be using
>>> it are a bit complex though.
>>>
>>> It 'might' be safe to use it for this, but if so can we have a comment
>>> explaining why. Also more than possible I'm misunderstanding things!
>>
>> We are using this bit 59 only when the entry is not active from MMU
>> perspective i.e PMD_SECT_VALID is clear.
>>
>
> Understood. I guess we ran out of bits that were always ignored so had
> to start using ones that are ignored in this particular state.
Right, there are no more available SW PTE bits.
#define PTE_DIRTY (_AT(pteval_t, 1) << 55)
#define PTE_SPECIAL (_AT(pteval_t, 1) << 56)
#define PTE_DEVMAP (_AT(pteval_t, 1) << 57)
#define PTE_PROT_NONE (_AT(pteval_t, 1) << 58) /* only when !PTE_VALID */
Earlier I had proposed using PTE_SPECIAL at PMD level for this purpose.
But Catalin prefers these unused bits as the entry is anyway invalid
and which also leaves aside PTE_SPECIAL at mapped PMD for later use.
There is already one comment near PMD_PRESENT_INVALID definition which
explains this situation.
+/*
+ * This help indicate that the entry is present i.e pmd_page()
+ * still points to a valid huge page in memory even if the pmd
+ * has been invalidated.
+ */
+#define PMD_PRESENT_INVALID (_AT(pteval_t, 1) << 59) /* only when !PMD_SECT_VALID */
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] arm64/mm: Change THP helpers to comply with generic MM semantics
2020-08-17 9:19 ` Anshuman Khandual
@ 2020-09-03 16:56 ` Catalin Marinas
-1 siblings, 0 replies; 26+ messages in thread
From: Catalin Marinas @ 2020-09-03 16:56 UTC (permalink / raw)
To: Anshuman Khandual
Cc: linux-mm, linux-arm-kernel, will, akpm, Mark Rutland,
Marc Zyngier, Suzuki Poulose, linux-kernel
On Mon, Aug 17, 2020 at 02:49:43PM +0530, Anshuman Khandual wrote:
> pmd_present() and pmd_trans_huge() are expected to behave in the following
> manner during various phases of a given PMD. It is derived from a previous
> detailed discussion on this topic [1] and present THP documentation [2].
>
> pmd_present(pmd):
>
> - Returns true if pmd refers to system RAM with a valid pmd_page(pmd)
> - Returns false if pmd does not refer to system RAM - Invalid pmd_page(pmd)
The second bullet doesn't make much sense. If you have a pmd mapping of
some I/O memory, pmd_present() still returns true (as does
pte_present()).
> diff --git a/arch/arm64/include/asm/pgtable-prot.h b/arch/arm64/include/asm/pgtable-prot.h
> index 4d867c6446c4..28792fdd9627 100644
> --- a/arch/arm64/include/asm/pgtable-prot.h
> +++ b/arch/arm64/include/asm/pgtable-prot.h
> @@ -19,6 +19,13 @@
> #define PTE_DEVMAP (_AT(pteval_t, 1) << 57)
> #define PTE_PROT_NONE (_AT(pteval_t, 1) << 58) /* only when !PTE_VALID */
>
> +/*
> + * This help indicate that the entry is present i.e pmd_page()
Nit: add another . after i.e
> + * still points to a valid huge page in memory even if the pmd
> + * has been invalidated.
> + */
> +#define PMD_PRESENT_INVALID (_AT(pteval_t, 1) << 59) /* only when !PMD_SECT_VALID */
> +
> #ifndef __ASSEMBLY__
>
> #include <asm/cpufeature.h>
> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> index d5d3fbe73953..7aa69cace784 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -145,6 +145,18 @@ static inline pte_t set_pte_bit(pte_t pte, pgprot_t prot)
> return pte;
> }
>
> +static inline pmd_t clr_pmd_bit(pmd_t pmd, pgprot_t prot)
> +{
> + pmd_val(pmd) &= ~pgprot_val(prot);
> + return pmd;
> +}
Could you use clear_pmd_bit (instead of clr) for consistency with
clear_pte_bit()?
It would be good if the mm folk can do a sanity check on the assumptions
about pmd_present/pmdp_invalidate/pmd_trans_huge.
The patch looks fine to me otherwise, feel free to add:
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
--
Catalin
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 1/2] arm64/mm: Change THP helpers to comply with generic MM semantics
@ 2020-09-03 16:56 ` Catalin Marinas
0 siblings, 0 replies; 26+ messages in thread
From: Catalin Marinas @ 2020-09-03 16:56 UTC (permalink / raw)
To: Anshuman Khandual
Cc: Mark Rutland, Suzuki Poulose, Marc Zyngier, linux-kernel,
linux-mm, akpm, will, linux-arm-kernel
On Mon, Aug 17, 2020 at 02:49:43PM +0530, Anshuman Khandual wrote:
> pmd_present() and pmd_trans_huge() are expected to behave in the following
> manner during various phases of a given PMD. It is derived from a previous
> detailed discussion on this topic [1] and present THP documentation [2].
>
> pmd_present(pmd):
>
> - Returns true if pmd refers to system RAM with a valid pmd_page(pmd)
> - Returns false if pmd does not refer to system RAM - Invalid pmd_page(pmd)
The second bullet doesn't make much sense. If you have a pmd mapping of
some I/O memory, pmd_present() still returns true (as does
pte_present()).
> diff --git a/arch/arm64/include/asm/pgtable-prot.h b/arch/arm64/include/asm/pgtable-prot.h
> index 4d867c6446c4..28792fdd9627 100644
> --- a/arch/arm64/include/asm/pgtable-prot.h
> +++ b/arch/arm64/include/asm/pgtable-prot.h
> @@ -19,6 +19,13 @@
> #define PTE_DEVMAP (_AT(pteval_t, 1) << 57)
> #define PTE_PROT_NONE (_AT(pteval_t, 1) << 58) /* only when !PTE_VALID */
>
> +/*
> + * This help indicate that the entry is present i.e pmd_page()
Nit: add another . after i.e
> + * still points to a valid huge page in memory even if the pmd
> + * has been invalidated.
> + */
> +#define PMD_PRESENT_INVALID (_AT(pteval_t, 1) << 59) /* only when !PMD_SECT_VALID */
> +
> #ifndef __ASSEMBLY__
>
> #include <asm/cpufeature.h>
> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> index d5d3fbe73953..7aa69cace784 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -145,6 +145,18 @@ static inline pte_t set_pte_bit(pte_t pte, pgprot_t prot)
> return pte;
> }
>
> +static inline pmd_t clr_pmd_bit(pmd_t pmd, pgprot_t prot)
> +{
> + pmd_val(pmd) &= ~pgprot_val(prot);
> + return pmd;
> +}
Could you use clear_pmd_bit (instead of clr) for consistency with
clear_pte_bit()?
It would be good if the mm folk can do a sanity check on the assumptions
about pmd_present/pmdp_invalidate/pmd_trans_huge.
The patch looks fine to me otherwise, feel free to add:
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
--
Catalin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 1/2] arm64/mm: Change THP helpers to comply with generic MM semantics
2020-09-03 16:56 ` Catalin Marinas
@ 2020-09-03 17:31 ` Ralph Campbell
-1 siblings, 0 replies; 26+ messages in thread
From: Ralph Campbell @ 2020-09-03 17:31 UTC (permalink / raw)
To: Catalin Marinas, Anshuman Khandual
Cc: linux-mm, linux-arm-kernel, will, akpm, Mark Rutland,
Marc Zyngier, Suzuki Poulose, linux-kernel
On 9/3/20 9:56 AM, Catalin Marinas wrote:
> On Mon, Aug 17, 2020 at 02:49:43PM +0530, Anshuman Khandual wrote:
>> pmd_present() and pmd_trans_huge() are expected to behave in the following
>> manner during various phases of a given PMD. It is derived from a previous
>> detailed discussion on this topic [1] and present THP documentation [2].
>>
>> pmd_present(pmd):
>>
>> - Returns true if pmd refers to system RAM with a valid pmd_page(pmd)
>> - Returns false if pmd does not refer to system RAM - Invalid pmd_page(pmd)
>
> The second bullet doesn't make much sense. If you have a pmd mapping of
> some I/O memory, pmd_present() still returns true (as does
> pte_present()).
>
>> diff --git a/arch/arm64/include/asm/pgtable-prot.h b/arch/arm64/include/asm/pgtable-prot.h
>> index 4d867c6446c4..28792fdd9627 100644
>> --- a/arch/arm64/include/asm/pgtable-prot.h
>> +++ b/arch/arm64/include/asm/pgtable-prot.h
>> @@ -19,6 +19,13 @@
>> #define PTE_DEVMAP (_AT(pteval_t, 1) << 57)
>> #define PTE_PROT_NONE (_AT(pteval_t, 1) << 58) /* only when !PTE_VALID */
>>
>> +/*
>> + * This help indicate that the entry is present i.e pmd_page()
>
> Nit: add another . after i.e
Another nit: "This help indicate" => "This helper indicates"
Maybe I should look at the series more. :-)
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] arm64/mm: Change THP helpers to comply with generic MM semantics
@ 2020-09-03 17:31 ` Ralph Campbell
0 siblings, 0 replies; 26+ messages in thread
From: Ralph Campbell @ 2020-09-03 17:31 UTC (permalink / raw)
To: Catalin Marinas, Anshuman Khandual
Cc: Mark Rutland, Suzuki Poulose, Marc Zyngier, linux-kernel,
linux-mm, akpm, will, linux-arm-kernel
On 9/3/20 9:56 AM, Catalin Marinas wrote:
> On Mon, Aug 17, 2020 at 02:49:43PM +0530, Anshuman Khandual wrote:
>> pmd_present() and pmd_trans_huge() are expected to behave in the following
>> manner during various phases of a given PMD. It is derived from a previous
>> detailed discussion on this topic [1] and present THP documentation [2].
>>
>> pmd_present(pmd):
>>
>> - Returns true if pmd refers to system RAM with a valid pmd_page(pmd)
>> - Returns false if pmd does not refer to system RAM - Invalid pmd_page(pmd)
>
> The second bullet doesn't make much sense. If you have a pmd mapping of
> some I/O memory, pmd_present() still returns true (as does
> pte_present()).
>
>> diff --git a/arch/arm64/include/asm/pgtable-prot.h b/arch/arm64/include/asm/pgtable-prot.h
>> index 4d867c6446c4..28792fdd9627 100644
>> --- a/arch/arm64/include/asm/pgtable-prot.h
>> +++ b/arch/arm64/include/asm/pgtable-prot.h
>> @@ -19,6 +19,13 @@
>> #define PTE_DEVMAP (_AT(pteval_t, 1) << 57)
>> #define PTE_PROT_NONE (_AT(pteval_t, 1) << 58) /* only when !PTE_VALID */
>>
>> +/*
>> + * This help indicate that the entry is present i.e pmd_page()
>
> Nit: add another . after i.e
Another nit: "This help indicate" => "This helper indicates"
Maybe I should look at the series more. :-)
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] arm64/mm: Change THP helpers to comply with generic MM semantics
2020-09-03 17:31 ` Ralph Campbell
@ 2020-09-08 10:25 ` Anshuman Khandual
-1 siblings, 0 replies; 26+ messages in thread
From: Anshuman Khandual @ 2020-09-08 10:25 UTC (permalink / raw)
To: Ralph Campbell, Catalin Marinas
Cc: linux-mm, linux-arm-kernel, will, akpm, Mark Rutland,
Marc Zyngier, Suzuki Poulose, linux-kernel
On 09/03/2020 11:01 PM, Ralph Campbell wrote:
>
> On 9/3/20 9:56 AM, Catalin Marinas wrote:
>> On Mon, Aug 17, 2020 at 02:49:43PM +0530, Anshuman Khandual wrote:
>>> pmd_present() and pmd_trans_huge() are expected to behave in the following
>>> manner during various phases of a given PMD. It is derived from a previous
>>> detailed discussion on this topic [1] and present THP documentation [2].
>>>
>>> pmd_present(pmd):
>>>
>>> - Returns true if pmd refers to system RAM with a valid pmd_page(pmd)
>>> - Returns false if pmd does not refer to system RAM - Invalid pmd_page(pmd)
>>
>> The second bullet doesn't make much sense. If you have a pmd mapping of
>> some I/O memory, pmd_present() still returns true (as does
>> pte_present()).
>>
>>> diff --git a/arch/arm64/include/asm/pgtable-prot.h b/arch/arm64/include/asm/pgtable-prot.h
>>> index 4d867c6446c4..28792fdd9627 100644
>>> --- a/arch/arm64/include/asm/pgtable-prot.h
>>> +++ b/arch/arm64/include/asm/pgtable-prot.h
>>> @@ -19,6 +19,13 @@
>>> #define PTE_DEVMAP (_AT(pteval_t, 1) << 57)
>>> #define PTE_PROT_NONE (_AT(pteval_t, 1) << 58) /* only when !PTE_VALID */
>>> +/*
>>> + * This help indicate that the entry is present i.e pmd_page()
>>
>> Nit: add another . after i.e
>
> Another nit: "This help indicate" => "This helper indicates"
>
> Maybe I should look at the series more. :-)
It is talking about the new PTE bit being used here not any
helper. Though the following replacement might be better.
s/This help indicate/This bit indicates/
/*
* This help indicate that the entry is present i.e pmd_page()
* still points to a valid huge page in memory even if the pmd
* has been invalidated.
*/
#define PMD_PRESENT_INVALID (_AT(pteval_t, 1) << 59) /* only when !PMD_SECT_VALID */
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] arm64/mm: Change THP helpers to comply with generic MM semantics
@ 2020-09-08 10:25 ` Anshuman Khandual
0 siblings, 0 replies; 26+ messages in thread
From: Anshuman Khandual @ 2020-09-08 10:25 UTC (permalink / raw)
To: Ralph Campbell, Catalin Marinas
Cc: Mark Rutland, Suzuki Poulose, Marc Zyngier, linux-kernel,
linux-mm, akpm, will, linux-arm-kernel
On 09/03/2020 11:01 PM, Ralph Campbell wrote:
>
> On 9/3/20 9:56 AM, Catalin Marinas wrote:
>> On Mon, Aug 17, 2020 at 02:49:43PM +0530, Anshuman Khandual wrote:
>>> pmd_present() and pmd_trans_huge() are expected to behave in the following
>>> manner during various phases of a given PMD. It is derived from a previous
>>> detailed discussion on this topic [1] and present THP documentation [2].
>>>
>>> pmd_present(pmd):
>>>
>>> - Returns true if pmd refers to system RAM with a valid pmd_page(pmd)
>>> - Returns false if pmd does not refer to system RAM - Invalid pmd_page(pmd)
>>
>> The second bullet doesn't make much sense. If you have a pmd mapping of
>> some I/O memory, pmd_present() still returns true (as does
>> pte_present()).
>>
>>> diff --git a/arch/arm64/include/asm/pgtable-prot.h b/arch/arm64/include/asm/pgtable-prot.h
>>> index 4d867c6446c4..28792fdd9627 100644
>>> --- a/arch/arm64/include/asm/pgtable-prot.h
>>> +++ b/arch/arm64/include/asm/pgtable-prot.h
>>> @@ -19,6 +19,13 @@
>>> #define PTE_DEVMAP (_AT(pteval_t, 1) << 57)
>>> #define PTE_PROT_NONE (_AT(pteval_t, 1) << 58) /* only when !PTE_VALID */
>>> +/*
>>> + * This help indicate that the entry is present i.e pmd_page()
>>
>> Nit: add another . after i.e
>
> Another nit: "This help indicate" => "This helper indicates"
>
> Maybe I should look at the series more. :-)
It is talking about the new PTE bit being used here not any
helper. Though the following replacement might be better.
s/This help indicate/This bit indicates/
/*
* This help indicate that the entry is present i.e pmd_page()
* still points to a valid huge page in memory even if the pmd
* has been invalidated.
*/
#define PMD_PRESENT_INVALID (_AT(pteval_t, 1) << 59) /* only when !PMD_SECT_VALID */
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] arm64/mm: Change THP helpers to comply with generic MM semantics
2020-09-03 16:56 ` Catalin Marinas
@ 2020-09-08 10:18 ` Anshuman Khandual
-1 siblings, 0 replies; 26+ messages in thread
From: Anshuman Khandual @ 2020-09-08 10:18 UTC (permalink / raw)
To: Catalin Marinas
Cc: linux-mm, linux-arm-kernel, will, akpm, Mark Rutland,
Marc Zyngier, Suzuki Poulose, linux-kernel
On 09/03/2020 10:26 PM, Catalin Marinas wrote:
> On Mon, Aug 17, 2020 at 02:49:43PM +0530, Anshuman Khandual wrote:
>> pmd_present() and pmd_trans_huge() are expected to behave in the following
>> manner during various phases of a given PMD. It is derived from a previous
>> detailed discussion on this topic [1] and present THP documentation [2].
>>
>> pmd_present(pmd):
>>
>> - Returns true if pmd refers to system RAM with a valid pmd_page(pmd)
>> - Returns false if pmd does not refer to system RAM - Invalid pmd_page(pmd)
>
> The second bullet doesn't make much sense. If you have a pmd mapping of
> some I/O memory, pmd_present() still returns true (as does
> pte_present()).
Derived this from an earlier discussion (https://lkml.org/lkml/2018/10/17/231)
but current representation here might not be accurate.
Would this be any better ?
pmd_present(pmd):
- Returns true if pmd refers to system RAM with a valid pmd_page(pmd)
- Returns false if pmd refers to a migration or swap entry
>
>> diff --git a/arch/arm64/include/asm/pgtable-prot.h b/arch/arm64/include/asm/pgtable-prot.h
>> index 4d867c6446c4..28792fdd9627 100644
>> --- a/arch/arm64/include/asm/pgtable-prot.h
>> +++ b/arch/arm64/include/asm/pgtable-prot.h
>> @@ -19,6 +19,13 @@
>> #define PTE_DEVMAP (_AT(pteval_t, 1) << 57)
>> #define PTE_PROT_NONE (_AT(pteval_t, 1) << 58) /* only when !PTE_VALID */
>>
>> +/*
>> + * This help indicate that the entry is present i.e pmd_page()
>
> Nit: add another . after i.e
Will fix.
>
>> + * still points to a valid huge page in memory even if the pmd
>> + * has been invalidated.
>> + */
>> +#define PMD_PRESENT_INVALID (_AT(pteval_t, 1) << 59) /* only when !PMD_SECT_VALID */
>> +
>> #ifndef __ASSEMBLY__
>>
>> #include <asm/cpufeature.h>
>> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
>> index d5d3fbe73953..7aa69cace784 100644
>> --- a/arch/arm64/include/asm/pgtable.h
>> +++ b/arch/arm64/include/asm/pgtable.h
>> @@ -145,6 +145,18 @@ static inline pte_t set_pte_bit(pte_t pte, pgprot_t prot)
>> return pte;
>> }
>>
>> +static inline pmd_t clr_pmd_bit(pmd_t pmd, pgprot_t prot)
>> +{
>> + pmd_val(pmd) &= ~pgprot_val(prot);
>> + return pmd;
>> +}
>
> Could you use clear_pmd_bit (instead of clr) for consistency with
> clear_pte_bit()?
Sure, will do.
>
> It would be good if the mm folk can do a sanity check on the assumptions
> about pmd_present/pmdp_invalidate/pmd_trans_huge.
>
> The patch looks fine to me otherwise, feel free to add:
>
> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 1/2] arm64/mm: Change THP helpers to comply with generic MM semantics
@ 2020-09-08 10:18 ` Anshuman Khandual
0 siblings, 0 replies; 26+ messages in thread
From: Anshuman Khandual @ 2020-09-08 10:18 UTC (permalink / raw)
To: Catalin Marinas
Cc: Mark Rutland, Suzuki Poulose, Marc Zyngier, linux-kernel,
linux-mm, akpm, will, linux-arm-kernel
On 09/03/2020 10:26 PM, Catalin Marinas wrote:
> On Mon, Aug 17, 2020 at 02:49:43PM +0530, Anshuman Khandual wrote:
>> pmd_present() and pmd_trans_huge() are expected to behave in the following
>> manner during various phases of a given PMD. It is derived from a previous
>> detailed discussion on this topic [1] and present THP documentation [2].
>>
>> pmd_present(pmd):
>>
>> - Returns true if pmd refers to system RAM with a valid pmd_page(pmd)
>> - Returns false if pmd does not refer to system RAM - Invalid pmd_page(pmd)
>
> The second bullet doesn't make much sense. If you have a pmd mapping of
> some I/O memory, pmd_present() still returns true (as does
> pte_present()).
Derived this from an earlier discussion (https://lkml.org/lkml/2018/10/17/231)
but current representation here might not be accurate.
Would this be any better ?
pmd_present(pmd):
- Returns true if pmd refers to system RAM with a valid pmd_page(pmd)
- Returns false if pmd refers to a migration or swap entry
>
>> diff --git a/arch/arm64/include/asm/pgtable-prot.h b/arch/arm64/include/asm/pgtable-prot.h
>> index 4d867c6446c4..28792fdd9627 100644
>> --- a/arch/arm64/include/asm/pgtable-prot.h
>> +++ b/arch/arm64/include/asm/pgtable-prot.h
>> @@ -19,6 +19,13 @@
>> #define PTE_DEVMAP (_AT(pteval_t, 1) << 57)
>> #define PTE_PROT_NONE (_AT(pteval_t, 1) << 58) /* only when !PTE_VALID */
>>
>> +/*
>> + * This help indicate that the entry is present i.e pmd_page()
>
> Nit: add another . after i.e
Will fix.
>
>> + * still points to a valid huge page in memory even if the pmd
>> + * has been invalidated.
>> + */
>> +#define PMD_PRESENT_INVALID (_AT(pteval_t, 1) << 59) /* only when !PMD_SECT_VALID */
>> +
>> #ifndef __ASSEMBLY__
>>
>> #include <asm/cpufeature.h>
>> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
>> index d5d3fbe73953..7aa69cace784 100644
>> --- a/arch/arm64/include/asm/pgtable.h
>> +++ b/arch/arm64/include/asm/pgtable.h
>> @@ -145,6 +145,18 @@ static inline pte_t set_pte_bit(pte_t pte, pgprot_t prot)
>> return pte;
>> }
>>
>> +static inline pmd_t clr_pmd_bit(pmd_t pmd, pgprot_t prot)
>> +{
>> + pmd_val(pmd) &= ~pgprot_val(prot);
>> + return pmd;
>> +}
>
> Could you use clear_pmd_bit (instead of clr) for consistency with
> clear_pte_bit()?
Sure, will do.
>
> It would be good if the mm folk can do a sanity check on the assumptions
> about pmd_present/pmdp_invalidate/pmd_trans_huge.
>
> The patch looks fine to me otherwise, feel free to add:
>
> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 1/2] arm64/mm: Change THP helpers to comply with generic MM semantics
2020-09-08 10:18 ` Anshuman Khandual
@ 2020-09-08 11:32 ` Catalin Marinas
-1 siblings, 0 replies; 26+ messages in thread
From: Catalin Marinas @ 2020-09-08 11:32 UTC (permalink / raw)
To: Anshuman Khandual
Cc: linux-mm, linux-arm-kernel, will, akpm, Mark Rutland,
Marc Zyngier, Suzuki Poulose, linux-kernel
On Tue, Sep 08, 2020 at 03:48:08PM +0530, Anshuman Khandual wrote:
> On 09/03/2020 10:26 PM, Catalin Marinas wrote:
> > On Mon, Aug 17, 2020 at 02:49:43PM +0530, Anshuman Khandual wrote:
> >> pmd_present() and pmd_trans_huge() are expected to behave in the following
> >> manner during various phases of a given PMD. It is derived from a previous
> >> detailed discussion on this topic [1] and present THP documentation [2].
> >>
> >> pmd_present(pmd):
> >>
> >> - Returns true if pmd refers to system RAM with a valid pmd_page(pmd)
> >> - Returns false if pmd does not refer to system RAM - Invalid pmd_page(pmd)
> >
> > The second bullet doesn't make much sense. If you have a pmd mapping of
> > some I/O memory, pmd_present() still returns true (as does
> > pte_present()).
>
> Derived this from an earlier discussion (https://lkml.org/lkml/2018/10/17/231)
> but current representation here might not be accurate.
>
> Would this be any better ?
>
> pmd_present(pmd):
>
> - Returns true if pmd refers to system RAM with a valid pmd_page(pmd)
> - Returns false if pmd refers to a migration or swap entry
Yes, that's better
--
Catalin
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] arm64/mm: Change THP helpers to comply with generic MM semantics
@ 2020-09-08 11:32 ` Catalin Marinas
0 siblings, 0 replies; 26+ messages in thread
From: Catalin Marinas @ 2020-09-08 11:32 UTC (permalink / raw)
To: Anshuman Khandual
Cc: Mark Rutland, Suzuki Poulose, Marc Zyngier, linux-kernel,
linux-mm, akpm, will, linux-arm-kernel
On Tue, Sep 08, 2020 at 03:48:08PM +0530, Anshuman Khandual wrote:
> On 09/03/2020 10:26 PM, Catalin Marinas wrote:
> > On Mon, Aug 17, 2020 at 02:49:43PM +0530, Anshuman Khandual wrote:
> >> pmd_present() and pmd_trans_huge() are expected to behave in the following
> >> manner during various phases of a given PMD. It is derived from a previous
> >> detailed discussion on this topic [1] and present THP documentation [2].
> >>
> >> pmd_present(pmd):
> >>
> >> - Returns true if pmd refers to system RAM with a valid pmd_page(pmd)
> >> - Returns false if pmd does not refer to system RAM - Invalid pmd_page(pmd)
> >
> > The second bullet doesn't make much sense. If you have a pmd mapping of
> > some I/O memory, pmd_present() still returns true (as does
> > pte_present()).
>
> Derived this from an earlier discussion (https://lkml.org/lkml/2018/10/17/231)
> but current representation here might not be accurate.
>
> Would this be any better ?
>
> pmd_present(pmd):
>
> - Returns true if pmd refers to system RAM with a valid pmd_page(pmd)
> - Returns false if pmd refers to a migration or swap entry
Yes, that's better
--
Catalin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 26+ messages in thread