diff mbox series

mm/mapping_dirty_helpers: Update huge page-table entry callbacks

Message ID 20200131100052.58761-1-thomas_os@shipmail.org
State New, archived
Headers show
Series mm/mapping_dirty_helpers: Update huge page-table entry callbacks | expand

Commit Message

Thomas Hellström (Intel) Jan. 31, 2020, 10 a.m. UTC
From: Thomas Hellstrom <thellstrom@vmware.com>

Following the update of pagewalk code
commit a07984d48146 ("mm: pagewalk: add p4d_entry() and pgd_entry()")
we can modify the mapping_dirty_helpers' huge page-table entry callbacks
to avoid splitting when a huge pud or -pmd is encountered.

Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com>
Cc: Steven Price <steven.price@arm.com>
---
 mm/mapping_dirty_helpers.c | 42 ++++++++++++++++++++++++++++++++++----
 1 file changed, 38 insertions(+), 4 deletions(-)

Comments

Steven Price Jan. 31, 2020, 11:33 a.m. UTC | #1
On Fri, Jan 31, 2020 at 10:00:52AM +0000, Thomas Hellström (VMware) wrote:
> From: Thomas Hellstrom <thellstrom@vmware.com>
> 
> Following the update of pagewalk code
> commit a07984d48146 ("mm: pagewalk: add p4d_entry() and pgd_entry()")
> we can modify the mapping_dirty_helpers' huge page-table entry callbacks
> to avoid splitting when a huge pud or -pmd is encountered.
> 
> Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com>
> Cc: Steven Price <steven.price@arm.com>

LGTM

Reviewed-by: Steven Price <steven.price@arm.com>

> ---
>  mm/mapping_dirty_helpers.c | 42 ++++++++++++++++++++++++++++++++++----
>  1 file changed, 38 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/mapping_dirty_helpers.c b/mm/mapping_dirty_helpers.c
> index 71070dda9643..2c7d03675903 100644
> --- a/mm/mapping_dirty_helpers.c
> +++ b/mm/mapping_dirty_helpers.c
> @@ -111,26 +111,60 @@ static int clean_record_pte(pte_t *pte, unsigned long addr,
>  	return 0;
>  }
>  
> -/* wp_clean_pmd_entry - The pagewalk pmd callback. */
> +/*
> + * wp_clean_pmd_entry - The pagewalk pmd callback.
> + *
> + * Dirty-tracking should take place on the PTE level, so
> + * WARN() if encountering a dirty huge pmd.
> + * Furthermore, never split huge pmds, since that currently
> + * causes dirty info loss. The pagefault handler should do
> + * that if needed.
> + */
>  static int wp_clean_pmd_entry(pmd_t *pmd, unsigned long addr, unsigned long end,
>  			      struct mm_walk *walk)
>  {
> -	/* Dirty-tracking should be handled on the pte level */
>  	pmd_t pmdval = pmd_read_atomic(pmd);
>  
> +	if (!pmd_trans_unstable(&pmdval))
> +		return 0;
> +
> +	if (pmd_none(pmdval)) {
> +		walk->action = ACTION_AGAIN;
> +		return 0;
> +	}
> +
> +	/* Huge pmd, present or migrated */
> +	walk->action = ACTION_CONTINUE;
>  	if (pmd_trans_huge(pmdval) || pmd_devmap(pmdval))
>  		WARN_ON(pmd_write(pmdval) || pmd_dirty(pmdval));
>  
>  	return 0;
>  }
>  
> -/* wp_clean_pud_entry - The pagewalk pud callback. */
> +/*
> + * wp_clean_pud_entry - The pagewalk pud callback.
> + *
> + * Dirty-tracking should take place on the PTE level, so
> + * WARN() if encountering a dirty huge puds.
> + * Furthermore, never split huge puds, since that currently
> + * causes dirty info loss. The pagefault handler should do
> + * that if needed.
> + */
>  static int wp_clean_pud_entry(pud_t *pud, unsigned long addr, unsigned long end,
>  			      struct mm_walk *walk)
>  {
> -	/* Dirty-tracking should be handled on the pte level */
>  	pud_t pudval = READ_ONCE(*pud);
>  
> +	if (!pud_trans_unstable(&pudval))
> +		return 0;
> +
> +	if (pud_none(pudval)) {
> +		walk->action = ACTION_AGAIN;
> +		return 0;
> +	}
> +
> +	/* Huge pud */
> +	walk->action = ACTION_CONTINUE;
>  	if (pud_trans_huge(pudval) || pud_devmap(pudval))
>  		WARN_ON(pud_write(pudval) || pud_dirty(pudval));
>  
> -- 
> 2.21.1
>
Thomas Hellström (Intel) Jan. 31, 2020, 2:55 p.m. UTC | #2
On 1/31/20 12:33 PM, Steven Price wrote:
> On Fri, Jan 31, 2020 at 10:00:52AM +0000, Thomas Hellström (VMware) wrote:
>> From: Thomas Hellstrom <thellstrom@vmware.com>
>>
>> Following the update of pagewalk code
>> commit a07984d48146 ("mm: pagewalk: add p4d_entry() and pgd_entry()")
>> we can modify the mapping_dirty_helpers' huge page-table entry callbacks
>> to avoid splitting when a huge pud or -pmd is encountered.
>>
>> Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com>
>> Cc: Steven Price <steven.price@arm.com>
> LGTM
>
> Reviewed-by: Steven Price <steven.price@arm.com>

Thanks for reviewing, Steven!

/Thomas
diff mbox series

Patch

diff --git a/mm/mapping_dirty_helpers.c b/mm/mapping_dirty_helpers.c
index 71070dda9643..2c7d03675903 100644
--- a/mm/mapping_dirty_helpers.c
+++ b/mm/mapping_dirty_helpers.c
@@ -111,26 +111,60 @@  static int clean_record_pte(pte_t *pte, unsigned long addr,
 	return 0;
 }
 
-/* wp_clean_pmd_entry - The pagewalk pmd callback. */
+/*
+ * wp_clean_pmd_entry - The pagewalk pmd callback.
+ *
+ * Dirty-tracking should take place on the PTE level, so
+ * WARN() if encountering a dirty huge pmd.
+ * Furthermore, never split huge pmds, since that currently
+ * causes dirty info loss. The pagefault handler should do
+ * that if needed.
+ */
 static int wp_clean_pmd_entry(pmd_t *pmd, unsigned long addr, unsigned long end,
 			      struct mm_walk *walk)
 {
-	/* Dirty-tracking should be handled on the pte level */
 	pmd_t pmdval = pmd_read_atomic(pmd);
 
+	if (!pmd_trans_unstable(&pmdval))
+		return 0;
+
+	if (pmd_none(pmdval)) {
+		walk->action = ACTION_AGAIN;
+		return 0;
+	}
+
+	/* Huge pmd, present or migrated */
+	walk->action = ACTION_CONTINUE;
 	if (pmd_trans_huge(pmdval) || pmd_devmap(pmdval))
 		WARN_ON(pmd_write(pmdval) || pmd_dirty(pmdval));
 
 	return 0;
 }
 
-/* wp_clean_pud_entry - The pagewalk pud callback. */
+/*
+ * wp_clean_pud_entry - The pagewalk pud callback.
+ *
+ * Dirty-tracking should take place on the PTE level, so
+ * WARN() if encountering a dirty huge puds.
+ * Furthermore, never split huge puds, since that currently
+ * causes dirty info loss. The pagefault handler should do
+ * that if needed.
+ */
 static int wp_clean_pud_entry(pud_t *pud, unsigned long addr, unsigned long end,
 			      struct mm_walk *walk)
 {
-	/* Dirty-tracking should be handled on the pte level */
 	pud_t pudval = READ_ONCE(*pud);
 
+	if (!pud_trans_unstable(&pudval))
+		return 0;
+
+	if (pud_none(pudval)) {
+		walk->action = ACTION_AGAIN;
+		return 0;
+	}
+
+	/* Huge pud */
+	walk->action = ACTION_CONTINUE;
 	if (pud_trans_huge(pudval) || pud_devmap(pudval))
 		WARN_ON(pud_write(pudval) || pud_dirty(pudval));