Minor improvements for pagewalk code

PATCH In review Rolf Eike Beer 3 participants 10 messages Aug 22, 2022 – Aug 24, 2022

Subsystems (2)

Memory management MM core

lore

Share & tools

lore

Rolf Eike Beer

author

2022-08-22 13:39 UTC #0 lore raw

cover Minor improvements for pagewalk code

For some project I had to use the pagewalk API for certain things and during this have read through the code multiple times. Our usage has changed several times depending on our current state of research as well.

During all of this I have made some tweaks to the code to be able to follow it better when hunting my own problems, and not call into some things that I actually don't need. The patches are more or less independent of each other. Especially the last one may heavily depend on personal taste, so if you don't like it, just ignore it.

I would welcome if you could just pick those that you think are fitting and provide feedback on either of the remaining ones. At the end none of them should make any functional difference.

Regards,

Eike

Signature · 7 lines

Rolf Eike Beer, emlix GmbH, http://www.emlix.com
Fon +49 551 30664-0, Fax +49 551 30664-11
Gothaer Platz 3, 37083 Göttingen, Germany
Sitz der Gesellschaft: Göttingen, Amtsgericht Göttingen HR B 3160
Geschäftsführung: Heike Jordan, Dr. Uwe Kracke – Ust-IdNr.: DE 205 198 055

emlix - smart embedded open source

Rolf Eike Beer

author

2022-08-22 13:39 UTC #1 lore raw

PATCH 1/6 mm: pagewalk: make error checks more obvious

The err variable only needs to be checked when it was assigned directly before, it is not carried on to any later checks. Move the checks into the same "if" conditions where they are assigned. Also just return the error at the relevant places. While at it move these err variables to a more local scope at some places.

Signed-off-by Rolf Eike Beer <[email protected]>

---
 mm/pagewalk.c | 140 +++++++++++++++++++++++++++-----------------------
 1 file changed, 77 insertions(+), 63 deletions(-)

mm/pagewalk.c +77 ~~-63~~

index 9b3db11a4d1d..c67b2532db8f 100644
@@ -24,25 +24,24 @@ static int walk_pte_range_inner(pte_t *pte, unsigned long addr,
 				unsigned long end, struct mm_walk *walk)
 {
 	const struct mm_walk_ops *ops = walk->ops;
-	int err = 0;
 
 	for (;;) {
-		err = ops->pte_entry(pte, addr, addr + PAGE_SIZE, walk);
+		int err = ops->pte_entry(pte, addr, addr + PAGE_SIZE, walk);
 		if (err)
-		       break;
+			return err;
 		if (addr >= end - PAGE_SIZE)
 			break;
 		addr += PAGE_SIZE;
 		pte++;
 	}
-	return err;
+	return 0;
 }
 
 static int walk_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
 			  struct mm_walk *walk)
 {
 	pte_t *pte;
-	int err = 0;
+	int err;
 	spinlock_t *ptl;
 
 	if (walk->no_vma) {

@@ -62,7 +61,6 @@ static int walk_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
 static int walk_hugepd_range(hugepd_t *phpd, unsigned long addr,
 			     unsigned long end, struct mm_walk *walk, int pdshift)
 {
-	int err = 0;
 	const struct mm_walk_ops *ops = walk->ops;
 	int shift = hugepd_shift(*phpd);
 	int page_size = 1 << shift;

@@ -75,6 +73,7 @@ static int walk_hugepd_range(hugepd_t *phpd, unsigned long addr,
 
 	for (;;) {
 		pte_t *pte;
+		int err;
 
 		spin_lock(&walk->mm->page_table_lock);
 		pte = hugepte_offset(*phpd, addr, pdshift);

@@ -82,12 +81,12 @@ static int walk_hugepd_range(hugepd_t *phpd, unsigned long addr,
 		spin_unlock(&walk->mm->page_table_lock);
 
 		if (err)
-			break;
+			return err;
 		if (addr >= end - page_size)
 			break;
 		addr += page_size;
 	}
-	return err;
+	return 0;
 }
 #else
 static int walk_hugepd_range(hugepd_t *phpd, unsigned long addr,

@@ -103,18 +102,20 @@ static int walk_pmd_range(pud_t *pud, unsigned long addr, unsigned long end,
 	pmd_t *pmd;
 	unsigned long next;
 	const struct mm_walk_ops *ops = walk->ops;
-	int err = 0;
 	int depth = real_depth(3);
 
 	pmd = pmd_offset(pud, addr);
 	do {
-again:
+		int err;
+
+ again:
 		next = pmd_addr_end(addr, end);
 		if (pmd_none(*pmd) || (!walk->vma && !walk->no_vma)) {
-			if (ops->pte_hole)
+			if (ops->pte_hole) {
 				err = ops->pte_hole(addr, next, depth, walk);
-			if (err)
-				break;
+				if (err)
+					return err;
+			}
 			continue;
 		}

@@ -124,10 +125,11 @@ static int walk_pmd_range(pud_t *pud, unsigned long addr, unsigned long end,
 		 * This implies that each ->pmd_entry() handler
 		 * needs to know about pmd_trans_huge() pmds
 		 */
-		if (ops->pmd_entry)
+		if (ops->pmd_entry) {
 			err = ops->pmd_entry(pmd, addr, next, walk);
-		if (err)
-			break;
+			if (err)
+				return err;
+		}
 
 		if (walk->action == ACTION_AGAIN)
 			goto again;

@@ -152,10 +154,10 @@ static int walk_pmd_range(pud_t *pud, unsigned long addr, unsigned long end,
 		else
 			err = walk_pte_range(pmd, addr, next, walk);
 		if (err)
-			break;
+			return err;
 	} while (pmd++, addr = next, addr != end);
 
-	return err;
+	return 0;
 }
 
 static int walk_pud_range(p4d_t *p4d, unsigned long addr, unsigned long end,

@@ -164,27 +166,30 @@ static int walk_pud_range(p4d_t *p4d, unsigned long addr, unsigned long end,
 	pud_t *pud;
 	unsigned long next;
 	const struct mm_walk_ops *ops = walk->ops;
-	int err = 0;
 	int depth = real_depth(2);
 
 	pud = pud_offset(p4d, addr);
 	do {
+		int err;
+
  again:
 		next = pud_addr_end(addr, end);
 		if (pud_none(*pud) || (!walk->vma && !walk->no_vma)) {
-			if (ops->pte_hole)
+			if (ops->pte_hole) {
 				err = ops->pte_hole(addr, next, depth, walk);
-			if (err)
-				break;
+				if (err)
+					return err;
+			}
 			continue;
 		}
 
 		walk->action = ACTION_SUBTREE;
 
-		if (ops->pud_entry)
+		if (ops->pud_entry) {
 			err = ops->pud_entry(pud, addr, next, walk);
-		if (err)
-			break;
+			if (err)
+				return err;
+		}
 
 		if (walk->action == ACTION_AGAIN)
 			goto again;

@@ -204,10 +209,10 @@ static int walk_pud_range(p4d_t *p4d, unsigned long addr, unsigned long end,
 		else
 			err = walk_pmd_range(pud, addr, next, walk);
 		if (err)
-			break;
+			return err;
 	} while (pud++, addr = next, addr != end);
 
-	return err;
+	return 0;
 }
 
 static int walk_p4d_range(pgd_t *pgd, unsigned long addr, unsigned long end,

@@ -216,33 +221,35 @@ static int walk_p4d_range(pgd_t *pgd, unsigned long addr, unsigned long end,
 	p4d_t *p4d;
 	unsigned long next;
 	const struct mm_walk_ops *ops = walk->ops;
-	int err = 0;
 	int depth = real_depth(1);
 
 	p4d = p4d_offset(pgd, addr);
 	do {
+		int err;
+
 		next = p4d_addr_end(addr, end);
 		if (p4d_none_or_clear_bad(p4d)) {
-			if (ops->pte_hole)
+			if (ops->pte_hole) {
 				err = ops->pte_hole(addr, next, depth, walk);
-			if (err)
-				break;
+				if (err)
+					return err;
+			}
 			continue;
 		}
 		if (ops->p4d_entry) {
 			err = ops->p4d_entry(p4d, addr, next, walk);
 			if (err)
-				break;
+				return err;
 		}
 		if (is_hugepd(__hugepd(p4d_val(*p4d))))
 			err = walk_hugepd_range((hugepd_t *)p4d, addr, next, walk, P4D_SHIFT);
 		else if (ops->pud_entry || ops->pmd_entry || ops->pte_entry)
 			err = walk_pud_range(p4d, addr, next, walk);
 		if (err)
-			break;
+			return err;
 	} while (p4d++, addr = next, addr != end);
 
-	return err;
+	return 0;
 }
 
 static int walk_pgd_range(unsigned long addr, unsigned long end,

@@ -251,35 +258,37 @@ static int walk_pgd_range(unsigned long addr, unsigned long end,
 	pgd_t *pgd;
 	unsigned long next;
 	const struct mm_walk_ops *ops = walk->ops;
-	int err = 0;
 
 	if (walk->pgd)
 		pgd = walk->pgd + pgd_index(addr);
 	else
 		pgd = pgd_offset(walk->mm, addr);
 	do {
+		int err;
+
 		next = pgd_addr_end(addr, end);
 		if (pgd_none_or_clear_bad(pgd)) {
-			if (ops->pte_hole)
+			if (ops->pte_hole) {
 				err = ops->pte_hole(addr, next, 0, walk);
-			if (err)
-				break;
+				if (err)
+					return err;
+			}
 			continue;
 		}
 		if (ops->pgd_entry) {
 			err = ops->pgd_entry(pgd, addr, next, walk);
 			if (err)
-				break;
+				return err;
 		}
 		if (is_hugepd(__hugepd(pgd_val(*pgd))))
 			err = walk_hugepd_range((hugepd_t *)pgd, addr, next, walk, PGDIR_SHIFT);
 		else if (ops->p4d_entry || ops->pud_entry || ops->pmd_entry || ops->pte_entry)
 			err = walk_p4d_range(pgd, addr, next, walk);
 		if (err)
-			break;
+			return err;
 	} while (pgd++, addr = next, addr != end);
 
-	return err;
+	return 0;
 }
 
 #ifdef CONFIG_HUGETLB_PAGE

@@ -300,9 +309,10 @@ static int walk_hugetlb_range(unsigned long addr, unsigned long end,
 	unsigned long sz = huge_page_size(h);
 	pte_t *pte;
 	const struct mm_walk_ops *ops = walk->ops;
-	int err = 0;
 
 	do {
+		int err;
+
 		next = hugetlb_entry_end(h, addr, end);
 		pte = huge_pte_offset(walk->mm, addr & hmask, sz);

@@ -312,10 +322,10 @@ static int walk_hugetlb_range(unsigned long addr, unsigned long end,
 			err = ops->pte_hole(addr, next, -1, walk);
 
 		if (err)
-			break;
+			return err;
 	} while (addr = next, addr != end);
 
-	return err;
+	return 0;
 }
 
 #else /* CONFIG_HUGETLB_PAGE */

@@ -351,10 +361,13 @@ static int walk_page_test(unsigned long start, unsigned long end,
 	 * vma(VM_PFNMAP).
 	 */
 	if (vma->vm_flags & VM_PFNMAP) {
-		int err = 1;
-		if (ops->pte_hole)
-			err = ops->pte_hole(start, end, -1, walk);
-		return err ? err : 1;
+		if (ops->pte_hole) {
+			int err = ops->pte_hole(start, end, -1, walk);
+
+			return err ? err : 1;
+		}
+
+		return 1;
 	}
 	return 0;
 }

@@ -428,7 +441,6 @@ int walk_page_range(struct mm_struct *mm, unsigned long start,
 		unsigned long end, const struct mm_walk_ops *ops,
 		void *private)
 {
-	int err = 0;
 	unsigned long next;
 	struct vm_area_struct *vma;
 	struct mm_walk walk = {

@@ -447,6 +459,8 @@ int walk_page_range(struct mm_struct *mm, unsigned long start,
 
 	vma = find_vma(walk.mm, start);
 	do {
+		int err;
+
 		if (!vma) { /* after the last vma */
 			walk.vma = NULL;
 			next = end;

@@ -465,18 +479,18 @@ int walk_page_range(struct mm_struct *mm, unsigned long start,
 				 * controlling the pagewalk, so should never
 				 * be passed to the callers.
 				 */
-				err = 0;
 				continue;
 			}
 			if (err < 0)
-				break;
+				return err;
 		}
-		if (walk.vma || walk.ops->pte_hole)
+		if (walk.vma || walk.ops->pte_hole) {
 			err = __walk_page_range(start, next, &walk);
-		if (err)
-			break;
+			if (err)
+				return err;
+		}
 	} while (start = next, start < end);
-	return err;
+	return 0;
 }
 
 /*

@@ -571,11 +585,12 @@ int walk_page_mapping(struct address_space *mapping, pgoff_t first_index,
 	struct vm_area_struct *vma;
 	pgoff_t vba, vea, cba, cea;
 	unsigned long start_addr, end_addr;
-	int err = 0;
 
 	lockdep_assert_held(&mapping->i_mmap_rwsem);
 	vma_interval_tree_foreach(vma, &mapping->i_mmap, first_index,
 				  first_index + nr - 1) {
+		int err;
+
 		/* Clip to the vma */
 		vba = vma->vm_pgoff;
 		vea = vba + vma_pages(vma);

@@ -593,16 +608,15 @@ int walk_page_mapping(struct address_space *mapping, pgoff_t first_index,
 		walk.mm = vma->vm_mm;
 
 		err = walk_page_test(vma->vm_start, vma->vm_end, &walk);
-		if (err > 0) {
-			err = 0;
-			break;
-		} else if (err < 0)
-			break;
+		if (err > 0)
+			return 0;
+		else if (err < 0)
+			return err;
 
 		err = __walk_page_range(start_addr, end_addr, &walk);
 		if (err)
-			break;
+			return err;
 	}
 
-	return err;
+	return 0;
 }

Signature · 12 lines

2.37.2



-- 
Rolf Eike Beer, emlix GmbH, http://www.emlix.com
Fon +49 551 30664-0, Fax +49 551 30664-11
Gothaer Platz 3, 37083 Göttingen, Germany
Sitz der Gesellschaft: Göttingen, Amtsgericht Göttingen HR B 3160
Geschäftsführung: Heike Jordan, Dr. Uwe Kracke – Ust-IdNr.: DE 205 198 055

emlix - smart embedded open source

Andrew Morton

maintainer

2022-08-22 20:53 UTC #2 lore raw

Re: [PATCH 1/6] mm: pagewalk: make error checks more obvious

Quoted 21 lines · On Mon, 22 Aug 2022 15:00:05 +0200 Rolf Eike Beer <[email protected]> wrote:

The err variable only needs to be checked when it was assigned directly before, it is not carried on to any later checks. Move the checks into the same "if" conditions where they are assigned. Also just return the error at the relevant places. While at it move these err variables to a more local scope at some places.
...
mm/pagewalk.c +4 -5
@@ -593,16 +608,15 @@ int walk_page_mapping(struct address_space *mapping, pgoff_t first_index,
 		walk.mm = vma->vm_mm;
 
 		err = walk_page_test(vma->vm_start, vma->vm_end, &walk);
-		if (err > 0) {
-			err = 0;
-			break;
-		} else if (err < 0)
-			break;
+		if (err > 0)
+			return 0;
+		else if (err < 0)
+			return err;

mm/pagewalk.c int walk_page_mapping(struct address_space *mapping, pgoff_t first_index, · line 622

Quoted 10 lines

mm/pagewalk.c +2 -2

 
 		err = __walk_page_range(start_addr, end_addr, &walk);
 		if (err)
-			break;
+			return err;
 	}
 
-	return err;
+	return 0;
 }

I'm not really a fan of multiple return points - it tends to lead to locking/resource leaks as the code evolves. I don't really think it's worth redoing the patch for this reason though; the rest looks good.

Rolf Eike Beer

author

2022-08-22 13:34 UTC #3 lore raw

PATCH 2/6 mm: pagewalk: don't check vma in walk_page_range_novma()

In reply to Rolf Eike Beer

Directly call walk_pgd_range() as that is everything that will actually happen in __walk_page_range() besides checking if the vma is set.

Signed-off-by Rolf Eike Beer <[email protected]>

---
 mm/pagewalk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

mm/pagewalk.c +1 -1

index c67b2532db8f..a06eb100fe4e 100644
@@ -517,7 +517,7 @@ int walk_page_range_novma(struct mm_struct *mm, unsigned long start,
 
 	mmap_assert_locked(walk.mm);
 
-	return __walk_page_range(start, end, &walk);
+	return walk_pgd_range(start, end, &walk);
 }
 
 int walk_page_vma(struct vm_area_struct *vma, const struct mm_walk_ops *ops,

Signature · 11 lines

2.37.2


-- 
Rolf Eike Beer, emlix GmbH, http://www.emlix.com
Fon +49 551 30664-0, Fax +49 551 30664-11
Gothaer Platz 3, 37083 Göttingen, Germany
Sitz der Gesellschaft: Göttingen, Amtsgericht Göttingen HR B 3160
Geschäftsführung: Heike Jordan, Dr. Uwe Kracke – Ust-IdNr.: DE 205 198 055

emlix - smart embedded open source

Rolf Eike Beer

author

2022-08-22 13:36 UTC #4 lore raw

PATCH 3/6 mm: pagewalk: fix documentation of PTE hole handling

In reply to Rolf Eike Beer

Empty PTEs are passed to the pte_entry callback, not to pte_hole.

Signed-off-by Rolf Eike Beer <[email protected]>

---
 include/linux/pagewalk.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

include/linux/pagewalk.h +5 -5

index ac7b38ad5903..f3fafb731ffd 100644
@@ -15,12 +15,12 @@ struct mm_walk;
  *			this handler is required to be able to handle
  *			pmd_trans_huge() pmds.  They may simply choose to
  *			split_huge_page() instead of handling it explicitly.
- * @pte_entry:		if set, called for each non-empty PTE (lowest-level)
- *			entry
+ * @pte_entry:		if set, called for each PTE (lowest-level) entry,
+ *			including empty ones
  * @pte_hole:		if set, called for each hole at all levels,
- *			depth is -1 if not known, 0:PGD, 1:P4D, 2:PUD, 3:PMD
- *			4:PTE. Any folded depths (where PTRS_PER_P?D is equal
- *			to 1) are skipped.
+ *			depth is -1 if not known, 0:PGD, 1:P4D, 2:PUD, 3:PMD.
+ *			Any folded depths (where PTRS_PER_P?D is equal to 1)
+ *			are skipped.
  * @hugetlb_entry:	if set, called for each hugetlb entry
  * @test_walk:		caller specific callback function to determine whether
  *			we walk over the current vma or not. Returning 0 means

Signature · 10 lines

2.37.2

-- 
Rolf Eike Beer, emlix GmbH, http://www.emlix.com
Fon +49 551 30664-0, Fax +49 551 30664-11
Gothaer Platz 3, 37083 Göttingen, Germany
Sitz der Gesellschaft: Göttingen, Amtsgericht Göttingen HR B 3160
Geschäftsführung: Heike Jordan, Dr. Uwe Kracke – Ust-IdNr.: DE 205 198 055

emlix - smart embedded open source

Rolf Eike Beer

author

2022-08-22 13:29 UTC #5 lore raw

PATCH 4/6 mm: pagewalk: add api documentation for walk_page_range_novma()

In reply to Rolf Eike Beer

Signed-off-by Rolf Eike Beer <[email protected]>

---
 mm/pagewalk.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

mm/pagewalk.c +9 -1

index a06eb100fe4e..dc3c850cd148 100644
@@ -493,7 +493,15 @@ int walk_page_range(struct mm_struct *mm, unsigned long start,
 	return 0;
 }
 
-/*
+/**
+ * walk_page_range_novma - walk a range of pagetables not backed by a vma
+ * @mm:		mm_struct representing the target process of page table walk
+ * @start:	start address of the virtual address range
+ * @end:	end address of the virtual address range
+ * @ops:	operation to call during the walk
+ * @pgd:	pgd to walk if different from mm->pgd
+ * @private:	private data for callbacks' usage
+ *
  * Similar to walk_page_range() but can walk any page tables even if they are
  * not backed by VMAs. Because 'unusual' entries may be walked this function
  * will also not lock the PTEs for the pte_entry() callback. This is useful for

Signature · 10 lines

2.37.2

-- 
Rolf Eike Beer, emlix GmbH, http://www.emlix.com
Fon +49 551 30664-0, Fax +49 551 30664-11
Gothaer Platz 3, 37083 Göttingen, Germany
Sitz der Gesellschaft: Göttingen, Amtsgericht Göttingen HR B 3160
Geschäftsführung: Heike Jordan, Dr. Uwe Kracke – Ust-IdNr.: DE 205 198 055

emlix - smart embedded open source

Rolf Eike Beer

author

2022-08-22 13:24 UTC #6 lore raw

PATCH 5/6 mm: pagewalk: allow walk_page_range_novma() without mm

In reply to Rolf Eike Beer

Since e47690d756a7 ("x86: mm: avoid allocating struct mm_struct on the stack") a pgd can be passed to walk_page_range_novma(). In case it is set no place in the pagewalk code use the walk.mm anymore, so permit to pass a NULL mm instead. It is up to the caller to ensure proper locking on the pgd in this case.

Signed-off-by Rolf Eike Beer <[email protected]>

---
 mm/pagewalk.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

mm/pagewalk.c +5 -2

index dc3c850cd148..f816f86588be 100644
@@ -506,6 +506,8 @@ int walk_page_range(struct mm_struct *mm, unsigned long start,
  * not backed by VMAs. Because 'unusual' entries may be walked this function
  * will also not lock the PTEs for the pte_entry() callback. This is useful for
  * walking the kernel pages tables or page tables for firmware.
+ *
+ * Either mm or pgd may be NULL, but not both.
  */
 int walk_page_range_novma(struct mm_struct *mm, unsigned long start,
 			  unsigned long end, const struct mm_walk_ops *ops,

@@ -520,10 +522,11 @@ int walk_page_range_novma(struct mm_struct *mm, unsigned long start,
 		.no_vma		= true
 	};
 
-	if (start >= end || !walk.mm)
+	if (start >= end || (!walk.mm && !walk.pgd))
 		return -EINVAL;
 
-	mmap_assert_locked(walk.mm);
+	if (walk.mm)
+		mmap_assert_locked(walk.mm);
 
 	return walk_pgd_range(start, end, &walk);
 }

Signature · 10 lines

2.37.2

-- 
Rolf Eike Beer, emlix GmbH, http://www.emlix.com
Fon +49 551 30664-0, Fax +49 551 30664-11
Gothaer Platz 3, 37083 Göttingen, Germany
Sitz der Gesellschaft: Göttingen, Amtsgericht Göttingen HR B 3160
Geschäftsführung: Heike Jordan, Dr. Uwe Kracke – Ust-IdNr.: DE 205 198 055

emlix - smart embedded open source

Rolf Eike Beer

author

2022-08-22 13:24 UTC #7 lore raw

PATCH 6/6 mm: pagewalk: move variables to more local scope, tweak loops

In reply to Rolf Eike Beer

Move some variables to more local scopes to make it obvious that they don't carry state. Put the end additions into the for loop instructions to make them easier to read.

Signed-off-by Rolf Eike Beer <[email protected]>

---
 mm/pagewalk.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

mm/pagewalk.c +9 ~~-13~~

index f816f86588be..d8be8a30b272 100644
@@ -25,14 +25,12 @@ static int walk_pte_range_inner(pte_t *pte, unsigned long addr,
 {
 	const struct mm_walk_ops *ops = walk->ops;
 
-	for (;;) {
+	for (;; addr += PAGE_SIZE, pte++) {
 		int err = ops->pte_entry(pte, addr, addr + PAGE_SIZE, walk);
 		if (err)
 			return err;
 		if (addr >= end - PAGE_SIZE)
 			break;
-		addr += PAGE_SIZE;
-		pte++;
 	}
 	return 0;
 }

@@ -42,13 +40,14 @@ static int walk_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
 {
 	pte_t *pte;
 	int err;
-	spinlock_t *ptl;
 
 	if (walk->no_vma) {
 		pte = pte_offset_map(pmd, addr);
 		err = walk_pte_range_inner(pte, addr, end, walk);
 		pte_unmap(pte);
 	} else {
+		spinlock_t *ptl;
+
 		pte = pte_offset_map_lock(walk->mm, pmd, addr, &ptl);
 		err = walk_pte_range_inner(pte, addr, end, walk);
 		pte_unmap_unlock(pte, ptl);

@@ -71,7 +70,7 @@ static int walk_hugepd_range(hugepd_t *phpd, unsigned long addr,
 	if (addr & (page_size - 1))
 		return 0;
 
-	for (;;) {
+	for (;; addr += page_size) {
 		pte_t *pte;
 		int err;

@@ -84,7 +83,6 @@ static int walk_hugepd_range(hugepd_t *phpd, unsigned long addr,
 			return err;
 		if (addr >= end - page_size)
 			break;
-		addr += page_size;
 	}
 	return 0;
 }

@@ -307,14 +305,13 @@ static int walk_hugetlb_range(unsigned long addr, unsigned long end,
 	unsigned long next;
 	unsigned long hmask = huge_page_mask(h);
 	unsigned long sz = huge_page_size(h);
-	pte_t *pte;
 	const struct mm_walk_ops *ops = walk->ops;
 
-	do {
+	for (; addr < end; addr = next) {
 		int err;
+		pte_t *pte = huge_pte_offset(walk->mm, addr & hmask, sz);
 
 		next = hugetlb_entry_end(h, addr, end);
-		pte = huge_pte_offset(walk->mm, addr & hmask, sz);
 
 		if (pte)
 			err = ops->hugetlb_entry(pte, hmask, addr, next, walk);

@@ -323,7 +320,7 @@ static int walk_hugetlb_range(unsigned long addr, unsigned long end,
 
 		if (err)
 			return err;
-	} while (addr = next, addr != end);
+	}
 
 	return 0;
 }

@@ -461,14 +458,13 @@ int walk_page_range(struct mm_struct *mm, unsigned long start,
 	do {
 		int err;
 
+		walk.vma = vma;
 		if (!vma) { /* after the last vma */
-			walk.vma = NULL;
 			next = end;
 		} else if (start < vma->vm_start) { /* outside vma */
 			walk.vma = NULL;
 			next = min(end, vma->vm_start);
 		} else { /* inside vma */
-			walk.vma = vma;
 			next = min(end, vma->vm_end);
 			vma = vma->vm_next;

@@ -595,11 +591,11 @@ int walk_page_mapping(struct address_space *mapping, pgoff_t first_index,
 	};
 	struct vm_area_struct *vma;
 	pgoff_t vba, vea, cba, cea;
-	unsigned long start_addr, end_addr;
 
 	lockdep_assert_held(&mapping->i_mmap_rwsem);
 	vma_interval_tree_foreach(vma, &mapping->i_mmap, first_index,
 				  first_index + nr - 1) {
+		unsigned long start_addr, end_addr;
 		int err;
 
 		/* Clip to the vma */

Signature · 10 lines

2.37.2

-- 
Rolf Eike Beer, emlix GmbH, http://www.emlix.com
Fon +49 551 30664-0, Fax +49 551 30664-11
Gothaer Platz 3, 37083 Göttingen, Germany
Sitz der Gesellschaft: Göttingen, Amtsgericht Göttingen HR B 3160
Geschäftsführung: Heike Jordan, Dr. Uwe Kracke – Ust-IdNr.: DE 205 198 055

emlix - smart embedded open source

Rolf Eike Beer

author

2022-08-24 11:00 UTC #8 lore raw

[PATCH 7/6] mm: pagewalk: add back missing variable initializations

In reply to Rolf Eike Beer

These initializations accidentially got lost during refactoring.

The first one can't actually be used without initialization, because walk_p4d_range() is only called when one of the 4 callbacks is set, but relying on this seems fragile.

Reported-by Linux Kernel Functional Testing <[email protected]>

Reported-by kernel test robot <[email protected]>

Reported-by Dan Carpenter <[email protected]>

Signed-off-by Rolf Eike Beer <[email protected]>

---
 mm/pagewalk.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Looks like my testing had just the right callbacks set so I never hit this. In case you want to have this squashed into the original one just drop me a note and I'll redo the whole series.

mm/pagewalk.c +3 -3

index d8be8a30b272..22e4bcca9bee 100644
@@ -223,7 +223,7 @@ static int walk_p4d_range(pgd_t *pgd, unsigned long addr, unsigned long end,
 
 	p4d = p4d_offset(pgd, addr);
 	do {
-		int err;
+		int err = 0;
 
 		next = p4d_addr_end(addr, end);
 		if (p4d_none_or_clear_bad(p4d)) {

@@ -262,7 +262,7 @@ static int walk_pgd_range(unsigned long addr, unsigned long end,
 	else
 		pgd = pgd_offset(walk->mm, addr);
 	do {
-		int err;
+		int err = 0;
 
 		next = pgd_addr_end(addr, end);
 		if (pgd_none_or_clear_bad(pgd)) {

@@ -308,7 +308,7 @@ static int walk_hugetlb_range(unsigned long addr, unsigned long end,
 	const struct mm_walk_ops *ops = walk->ops;
 
 	for (; addr < end; addr = next) {
-		int err;
+		int err = 0;
 		pte_t *pte = huge_pte_offset(walk->mm, addr & hmask, sz);
 
 		next = hugetlb_entry_end(h, addr, end);

Signature · 10 lines

2.37.2

-- 
Rolf Eike Beer, emlix GmbH, https://www.emlix.com
Fon +49 551 30664-0, Fax +49 551 30664-11
Gothaer Platz 3, 37083 Göttingen, Germany
Sitz der Gesellschaft: Göttingen, Amtsgericht Göttingen HR B 3160
Geschäftsführung: Heike Jordan, Dr. Uwe Kracke – Ust-IdNr.: DE 205 198 055

emlix - smart embedded open source

Nathan Chancellor

Reviewed-by

2022-08-24 15:08 UTC #9 lore raw

Re: [PATCH 7/6] mm: pagewalk: add back missing variable initializations

Quoted 10 lines · Rolf Eike Beer

These initializations accidentially got lost during refactoring.
The first one can't actually be used without initialization, because walk_p4d_range() is only called when one of the 4 callbacks is set, but relying on this seems fragile.
Reported-by Linux Kernel Functional Testing <[email protected]>
Reported-by kernel test robot <[email protected]>
Reported-by Dan Carpenter <[email protected]>
Signed-off-by Rolf Eike Beer <[email protected]>

Reviewed-by Nathan Chancellor <[email protected]>

Quoted 52 lines

---
 mm/pagewalk.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Looks like my testing had just the right callbacks set so I never hit this. In case you want to have this squashed into the original one just drop me a note and I'll redo the whole series.

mm/pagewalk.c +3 -3

index d8be8a30b272..22e4bcca9bee 100644
@@ -223,7 +223,7 @@ static int walk_p4d_range(pgd_t *pgd, unsigned long addr, unsigned long end,
 
 	p4d = p4d_offset(pgd, addr);
 	do {
-		int err;
+		int err = 0;
 
 		next = p4d_addr_end(addr, end);
 		if (p4d_none_or_clear_bad(p4d)) {

@@ -262,7 +262,7 @@ static int walk_pgd_range(unsigned long addr, unsigned long end,
 	else
 		pgd = pgd_offset(walk->mm, addr);
 	do {
-		int err;
+		int err = 0;
 
 		next = pgd_addr_end(addr, end);
 		if (pgd_none_or_clear_bad(pgd)) {

@@ -308,7 +308,7 @@ static int walk_hugetlb_range(unsigned long addr, unsigned long end,
 	const struct mm_walk_ops *ops = walk->ops;
 
 	for (; addr < end; addr = next) {
-		int err;
+		int err = 0;
 		pte_t *pte = huge_pte_offset(walk->mm, addr & hmask, sz);
 
 		next = hugetlb_entry_end(h, addr, end);

Signature · 10 lines

2.37.2

-- 
Rolf Eike Beer, emlix GmbH, https://www.emlix.com
Fon +49 551 30664-0, Fax +49 551 30664-11
Gothaer Platz 3, 37083 Göttingen, Germany
Sitz der Gesellschaft: Göttingen, Amtsgericht Göttingen HR B 3160
Geschäftsführung: Heike Jordan, Dr. Uwe Kracke – Ust-IdNr.: DE 205 198 055

emlix - smart embedded open source