Skip to contentSkip to conversation

mm: Update mask post pxd_clear_bad()

PATCH In review Dev Jain 3 participants 6 messages Mar 13, 2025 – Mar 17, 2025

Subsystems (3)
Stable branch Memory management MM core
lore
Share & tools
lore

Dev Jain

author
#0 lore raw

PATCH 1/1 mm: Update mask post pxd_clear_bad()

Since pxd_clear_bad() is an operation changing the state of the page tables, we should call arch_sync_kernel_mappings() post this.

Fixes e80d3909be42 ("mm: track page table modifications in __apply_to_page_range()")
Cc <[email protected]>
Signed-off-by Dev Jain <[email protected]>
---
 mm/memory.c | 4 ++++
 1 file changed, 4 insertions(+)
mm/memory.c +4 -0
index 78c7ee62795e..9a4a8c710be0 100644
@@ -2987,6 +2987,7 @@ static int apply_to_pmd_range(struct mm_struct *mm, pud_t *pud,
 			if (!create)
 				continue;
 			pmd_clear_bad(pmd);
+			*mask = PGTBL_PMD_MODIFIED;
 		}
 		err = apply_to_pte_range(mm, pmd, addr, next,
 					 fn, data, create, mask);

@@ -3023,6 +3024,7 @@ static int apply_to_pud_range(struct mm_struct *mm, p4d_t *p4d,
 			if (!create)
 				continue;
 			pud_clear_bad(pud);
+			*mask = PGTBL_PUD_MODIFIED;
 		}
 		err = apply_to_pmd_range(mm, pud, addr, next,
 					 fn, data, create, mask);

@@ -3059,6 +3061,7 @@ static int apply_to_p4d_range(struct mm_struct *mm, pgd_t *pgd,
 			if (!create)
 				continue;
 			p4d_clear_bad(p4d);
+			*mask = PGTBL_P4D_MODIFIED;
 		}
 		err = apply_to_pud_range(mm, p4d, addr, next,
 					 fn, data, create, mask);

@@ -3095,6 +3098,7 @@ static int __apply_to_page_range(struct mm_struct *mm, unsigned long addr,
 			if (!create)
 				continue;
 			pgd_clear_bad(pgd);
+			mask = PGTBL_PGD_MODIFIED;
 		}
 		err = apply_to_p4d_range(mm, pgd, addr, next,
 					 fn, data, create, &mask);
Signature · 1 line
2.30.2
author
#1 lore raw
Quoted 15 lines · Dev Jain

Since pxd_clear_bad() is an operation changing the state of the page tables, we should call arch_sync_kernel_mappings() post this.

Fixes e80d3909be42 ("mm: track page table modifications in __apply_to_page_range()")
Cc <[email protected]>
Signed-off-by Dev Jain <[email protected]>
---
  mm/memory.c | 4 ++++
  1 file changed, 4 insertions(+)
mm/memory.c +0 -0
index 78c7ee62795e..9a4a8c710be0 100644

mm/memory.c static int apply_to_pmd_range(struct mm_struct *mm, pud_t *pud, · line 2990

Quoted 4 lines
mm/memory.c +1 -0
@@ -2987,6 +2987,7 @@ static int apply_to_pmd_range(struct mm_struct *mm, pud_t *pud,
  			if (!create)
  				continue;
  			pmd_clear_bad(pmd);
+			*mask = PGTBL_PMD_MODIFIED;

Oh well, I guess these should have been *mask |= PGTBL_PMD_MODIFIED.

Quoted 27 lines
mm/memory.c +3 -0
  		}
  		err = apply_to_pte_range(mm, pmd, addr, next,
  					 fn, data, create, mask);

@@ -3023,6 +3024,7 @@ static int apply_to_pud_range(struct mm_struct *mm, p4d_t *p4d,
  			if (!create)
  				continue;
  			pud_clear_bad(pud);
+			*mask = PGTBL_PUD_MODIFIED;
  		}
  		err = apply_to_pmd_range(mm, pud, addr, next,
  					 fn, data, create, mask);

@@ -3059,6 +3061,7 @@ static int apply_to_p4d_range(struct mm_struct *mm, pgd_t *pgd,
  			if (!create)
  				continue;
  			p4d_clear_bad(p4d);
+			*mask = PGTBL_P4D_MODIFIED;
  		}
  		err = apply_to_pud_range(mm, p4d, addr, next,
  					 fn, data, create, mask);

@@ -3095,6 +3098,7 @@ static int __apply_to_page_range(struct mm_struct *mm, unsigned long addr,
  			if (!create)
  				continue;
  			pgd_clear_bad(pgd);
+			mask = PGTBL_PGD_MODIFIED;
  		}
  		err = apply_to_p4d_range(mm, pgd, addr, next,
  					 fn, data, create, &mask);
#2 lore raw

Hi Dev,

Quoted 39 lines
Quoted 15 lines

Since pxd_clear_bad() is an operation changing the state of the page tables, we should call arch_sync_kernel_mappings() post this.

Fixes e80d3909be42 ("mm: track page table modifications in __apply_to_page_range()")
Cc <[email protected]>
Signed-off-by Dev Jain <[email protected]>
---
  mm/memory.c | 4 ++++
  1 file changed, 4 insertions(+)
mm/memory.c +0 -0
index 78c7ee62795e..9a4a8c710be0 100644

mm/memory.c static int apply_to_pmd_range(struct mm_struct *mm, pud_t *pud, · line 2990

Quoted 4 lines
mm/memory.c +1 -0
@@ -2987,6 +2987,7 @@ static int apply_to_pmd_range(struct mm_struct *mm, pud_t *pud,
                      if (!create)
                              continue;
                      pmd_clear_bad(pmd);
+                     *mask = PGTBL_PMD_MODIFIED;

Oh well, I guess these should have been *mask |= PGTBL_PMD_MODIFIED.

Quoted 16 lines
mm/memory.c +2 -0
              }
              err = apply_to_pte_range(mm, pmd, addr, next,
                                       fn, data, create, mask);

@@ -3023,6 +3024,7 @@ static int apply_to_pud_range(struct mm_struct *mm, p4d_t *p4d,
                      if (!create)
                              continue;
                      pud_clear_bad(pud);
+                     *mask = PGTBL_PUD_MODIFIED;
              }
              err = apply_to_pmd_range(mm, pud, addr, next,
                                       fn, data, create, mask);

@@ -3059,6 +3061,7 @@ static int apply_to_p4d_range(struct mm_struct *mm, pgd_t *pgd,
                      if (!create)
                              continue;
                      p4d_clear_bad(p4d);
+                     *mask = PGTBL_P4D_MODIFIED;
Quoted 11 lines

mm/memory.c static int __apply_to_page_range(struct mm_struct *mm, unsigned long addr, · line 3100

Quoted 7 lines
mm/memory.c +0 -0
              }
              err = apply_to_pud_range(mm, p4d, addr, next,
                                       fn, data, create, mask);

@@ -3095,6 +3098,7 @@ static int __apply_to_page_range(struct mm_struct *mm, unsigned long addr,
                      if (!create)
                              continue;
                      pgd_clear_bad(pgd);

+ mask = PGTBL_PGD_MODIFIED;

Quoted 3 lines
mm/memory.c +0 -0
              }
              err = apply_to_p4d_range(mm, pgd, addr, next,
                                       fn, data, create, &mask);

I don't think this wouldn't need.
the pXd_clear_bad() is only called at creation of each level of page table, and when it clear, the following, apply_to_pXd_range() function would be set the make properly via pXd_alloc() and apply_to_pte_range().

Thanks.

author
#3 lore raw
Quoted 49 lines · Yeo Reum Yun

Hi Dev,

Quoted 47 lines
Quoted 15 lines

Since pxd_clear_bad() is an operation changing the state of the page tables, we should call arch_sync_kernel_mappings() post this.

Fixes e80d3909be42 ("mm: track page table modifications in __apply_to_page_range()")
Cc <[email protected]>
Signed-off-by Dev Jain <[email protected]>
---
   mm/memory.c | 4 ++++
   1 file changed, 4 insertions(+)
mm/memory.c +0 -0
index 78c7ee62795e..9a4a8c710be0 100644

mm/memory.c static int apply_to_pmd_range(struct mm_struct *mm, pud_t *pud, · line 2990

Quoted 4 lines
mm/memory.c +1 -0
@@ -2987,6 +2987,7 @@ static int apply_to_pmd_range(struct mm_struct *mm, pud_t *pud,
                       if (!create)
                               continue;
                       pmd_clear_bad(pmd);
+                     *mask = PGTBL_PMD_MODIFIED;

Oh well, I guess these should have been *mask |= PGTBL_PMD_MODIFIED.

Quoted 20 lines
mm/memory.c +2 -0
               }
               err = apply_to_pte_range(mm, pmd, addr, next,
                                        fn, data, create, mask);

@@ -3023,6 +3024,7 @@ static int apply_to_pud_range(struct mm_struct *mm, p4d_t *p4d,
                       if (!create)
                               continue;
                       pud_clear_bad(pud);
+                     *mask = PGTBL_PUD_MODIFIED;
               }
               err = apply_to_pmd_range(mm, pud, addr, next,
                                        fn, data, create, mask);

@@ -3059,6 +3061,7 @@ static int apply_to_p4d_range(struct mm_struct *mm, pgd_t *pgd,
                       if (!create)
                               continue;
                       p4d_clear_bad(p4d);
+                     *mask = PGTBL_P4D_MODIFIED;
               }
               err = apply_to_pud_range(mm, p4d, addr, next,
                                        fn, data, create, mask);

mm/memory.c static int __apply_to_page_range(struct mm_struct *mm, unsigned long addr, · line 3100

Quoted 3 lines
mm/memory.c +0 -0
@@ -3095,6 +3098,7 @@ static int __apply_to_page_range(struct mm_struct *mm, unsigned long addr,
                       if (!create)
                               continue;
                       pgd_clear_bad(pgd);

+ mask = PGTBL_PGD_MODIFIED;

Quoted 8 lines
Quoted 3 lines

mm/memory.c static int __apply_to_page_range(struct mm_struct *mm, unsigned long addr, · line 3104

Quoted 3 lines
mm/memory.c +0 -0
               }
               err = apply_to_p4d_range(mm, pgd, addr, next,
                                        fn, data, create, &mask);

I don't think this wouldn't need.
the pXd_clear_bad() is only called at creation of each level of page table, and when it clear, the following, apply_to_pXd_range() function would be set the make properly via pXd_alloc() and apply_to_pte_range().

Makes sense. But pxd_clear_bad() gets called in case of !pxd_none(), so while creating, why would the page containing the page table not be none? I believe it should be cleared already?

Quoted 4 lines

Thanks.

#4 lore raw
In reply to Dev Jain
Quoted 2 lines · Dev Jain

Since pxd_clear_bad() is an operation changing the state of the page tables, we should call arch_sync_kernel_mappings() post this.

Could you explain why? What effect does not calling arch_sync_kernel_mappings() have in this case?

author
#5 lore raw
Quoted 6 lines · Matthew Wilcox
Quoted 2 lines · Dev Jain

Since pxd_clear_bad() is an operation changing the state of the page tables, we should call arch_sync_kernel_mappings() post this.

Could you explain why? What effect does not calling arch_sync_kernel_mappings() have in this case?

Apologies, I again forgot to explain the userspace effect.
I just found this by code inspection, using the logic the fixes commit uses: we should sync when we change the pxd.

The question I have been pondering on is, what is the use of the pxd_bad() macros, when do we actually hit a bad state, and why don't we just trigger a BUG when we hit pxd_bad()?