[PATCH 0/6] Minor improvements for pagewalk code

[PATCH 0/6] Minor improvements for pagewalk code

[Rolf Eike Beer]

[-- Attachment #1: Type: text/plain, Size: 1107 bytes --]

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
-- 
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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 313 bytes --]

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

[Rolf Eike Beer]

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 <eb@emlix.com>
---
 mm/pagewalk.c | 140 +++++++++++++++++++++++++++-----------------------
 1 file changed, 77 insertions(+), 63 deletions(-)

diff --git a/mm/pagewalk.c b/mm/pagewalk.c
index 9b3db11a4d1d..c67b2532db8f 100644
--- a/mm/pagewalk.c
+++ b/mm/pagewalk.c
@@ -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;
 }
-- 
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

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

[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 <eb@emlix.com>
---
 mm/pagewalk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/pagewalk.c b/mm/pagewalk.c
index c67b2532db8f..a06eb100fe4e 100644
--- a/mm/pagewalk.c
+++ b/mm/pagewalk.c
@@ -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,
-- 
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

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

[Rolf Eike Beer]

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

Signed-off-by: Rolf Eike Beer <eb@emlix.com>
---
 include/linux/pagewalk.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/linux/pagewalk.h b/include/linux/pagewalk.h
index ac7b38ad5903..f3fafb731ffd 100644
--- a/include/linux/pagewalk.h
+++ b/include/linux/pagewalk.h
@@ -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
-- 
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

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

[Rolf Eike Beer]

Signed-off-by: Rolf Eike Beer <eb@emlix.com>
---
 mm/pagewalk.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/mm/pagewalk.c b/mm/pagewalk.c
index a06eb100fe4e..dc3c850cd148 100644
--- a/mm/pagewalk.c
+++ b/mm/pagewalk.c
@@ -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
-- 
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

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

[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 <eb@emlix.com>
---
 mm/pagewalk.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/mm/pagewalk.c b/mm/pagewalk.c
index dc3c850cd148..f816f86588be 100644
--- a/mm/pagewalk.c
+++ b/mm/pagewalk.c
@@ -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);
 }
-- 
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

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

[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 <eb@emlix.com>
---
 mm/pagewalk.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/mm/pagewalk.c b/mm/pagewalk.c
index f816f86588be..d8be8a30b272 100644
--- a/mm/pagewalk.c
+++ b/mm/pagewalk.c
@@ -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 */
-- 
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

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

[Andrew Morton]

On Mon, 22 Aug 2022 15:00:05 +0200 Rolf Eike Beer <eb@emlix.com> 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.
> 
> ...
>
> @@ -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;
>  }

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.

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

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 10189 bytes --]

BCC: lkp(a)intel.com
CC: kbuild-all(a)lists.01.org
In-Reply-To: <2203731.iZASKD2KPV@devpool047>
References: <2203731.iZASKD2KPV@devpool047>
TO: Rolf Eike Beer <eb@emlix.com>

Hi Rolf,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on akpm-mm/mm-everything]
[also build test WARNING on linus/master v6.0-rc2]
[cannot apply to next-20220823]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Rolf-Eike-Beer/Minor-improvements-for-pagewalk-code/20220822-214107
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
:::::: branch date: 19 hours ago
:::::: commit date: 19 hours ago
config: x86_64-randconfig-m001 (https://download.01.org/0day-ci/archive/20220823/202208231649.7QKavhA3-lkp(a)intel.com/config)
compiler: gcc-11 (Debian 11.3.0-5) 11.3.0

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
mm/pagewalk.c:248 walk_p4d_range() error: uninitialized symbol 'err'.
mm/pagewalk.c:287 walk_pgd_range() error: uninitialized symbol 'err'.
mm/pagewalk.c:324 walk_hugetlb_range() error: uninitialized symbol 'err'.

vim +/err +248 mm/pagewalk.c

e6473092bd9116 Matt Mackall       2008-02-04  217  
c2febafc67734a Kirill A. Shutemov 2017-03-09  218  static int walk_p4d_range(pgd_t *pgd, unsigned long addr, unsigned long end,
c2febafc67734a Kirill A. Shutemov 2017-03-09  219  			  struct mm_walk *walk)
c2febafc67734a Kirill A. Shutemov 2017-03-09  220  {
c2febafc67734a Kirill A. Shutemov 2017-03-09  221  	p4d_t *p4d;
c2febafc67734a Kirill A. Shutemov 2017-03-09  222  	unsigned long next;
7b86ac3371b70c Christoph Hellwig  2019-08-28  223  	const struct mm_walk_ops *ops = walk->ops;
b7a16c7ad790d0 Steven Price       2020-02-03  224  	int depth = real_depth(1);
c2febafc67734a Kirill A. Shutemov 2017-03-09  225  
c2febafc67734a Kirill A. Shutemov 2017-03-09  226  	p4d = p4d_offset(pgd, addr);
c2febafc67734a Kirill A. Shutemov 2017-03-09  227  	do {
40d6cb9177b6a9 Rolf Eike Beer     2022-08-22  228  		int err;
40d6cb9177b6a9 Rolf Eike Beer     2022-08-22  229  
c2febafc67734a Kirill A. Shutemov 2017-03-09  230  		next = p4d_addr_end(addr, end);
c2febafc67734a Kirill A. Shutemov 2017-03-09  231  		if (p4d_none_or_clear_bad(p4d)) {
40d6cb9177b6a9 Rolf Eike Beer     2022-08-22  232  			if (ops->pte_hole) {
b7a16c7ad790d0 Steven Price       2020-02-03  233  				err = ops->pte_hole(addr, next, depth, walk);
c2febafc67734a Kirill A. Shutemov 2017-03-09  234  				if (err)
40d6cb9177b6a9 Rolf Eike Beer     2022-08-22  235  					return err;
40d6cb9177b6a9 Rolf Eike Beer     2022-08-22  236  			}
c2febafc67734a Kirill A. Shutemov 2017-03-09  237  			continue;
c2febafc67734a Kirill A. Shutemov 2017-03-09  238  		}
3afc423632a194 Steven Price       2020-02-03  239  		if (ops->p4d_entry) {
3afc423632a194 Steven Price       2020-02-03  240  			err = ops->p4d_entry(p4d, addr, next, walk);
3afc423632a194 Steven Price       2020-02-03  241  			if (err)
40d6cb9177b6a9 Rolf Eike Beer     2022-08-22  242  				return err;
3afc423632a194 Steven Price       2020-02-03  243  		}
e17eae2b839937 Christophe Leroy   2021-06-28  244  		if (is_hugepd(__hugepd(p4d_val(*p4d))))
e17eae2b839937 Christophe Leroy   2021-06-28  245  			err = walk_hugepd_range((hugepd_t *)p4d, addr, next, walk, P4D_SHIFT);
e17eae2b839937 Christophe Leroy   2021-06-28  246  		else if (ops->pud_entry || ops->pmd_entry || ops->pte_entry)
c2febafc67734a Kirill A. Shutemov 2017-03-09  247  			err = walk_pud_range(p4d, addr, next, walk);
c2febafc67734a Kirill A. Shutemov 2017-03-09 @248  		if (err)
40d6cb9177b6a9 Rolf Eike Beer     2022-08-22  249  			return err;
c2febafc67734a Kirill A. Shutemov 2017-03-09  250  	} while (p4d++, addr = next, addr != end);
c2febafc67734a Kirill A. Shutemov 2017-03-09  251  
40d6cb9177b6a9 Rolf Eike Beer     2022-08-22  252  	return 0;
c2febafc67734a Kirill A. Shutemov 2017-03-09  253  }
c2febafc67734a Kirill A. Shutemov 2017-03-09  254  
fafaa4264eba49 Naoya Horiguchi    2015-02-11  255  static int walk_pgd_range(unsigned long addr, unsigned long end,
fafaa4264eba49 Naoya Horiguchi    2015-02-11  256  			  struct mm_walk *walk)
fafaa4264eba49 Naoya Horiguchi    2015-02-11  257  {
fafaa4264eba49 Naoya Horiguchi    2015-02-11  258  	pgd_t *pgd;
fafaa4264eba49 Naoya Horiguchi    2015-02-11  259  	unsigned long next;
7b86ac3371b70c Christoph Hellwig  2019-08-28  260  	const struct mm_walk_ops *ops = walk->ops;
fafaa4264eba49 Naoya Horiguchi    2015-02-11  261  
e47690d756a760 Steven Price       2020-02-03  262  	if (walk->pgd)
e47690d756a760 Steven Price       2020-02-03  263  		pgd = walk->pgd + pgd_index(addr);
e47690d756a760 Steven Price       2020-02-03  264  	else
fafaa4264eba49 Naoya Horiguchi    2015-02-11  265  		pgd = pgd_offset(walk->mm, addr);
fafaa4264eba49 Naoya Horiguchi    2015-02-11  266  	do {
40d6cb9177b6a9 Rolf Eike Beer     2022-08-22  267  		int err;
40d6cb9177b6a9 Rolf Eike Beer     2022-08-22  268  
fafaa4264eba49 Naoya Horiguchi    2015-02-11  269  		next = pgd_addr_end(addr, end);
fafaa4264eba49 Naoya Horiguchi    2015-02-11  270  		if (pgd_none_or_clear_bad(pgd)) {
40d6cb9177b6a9 Rolf Eike Beer     2022-08-22  271  			if (ops->pte_hole) {
b7a16c7ad790d0 Steven Price       2020-02-03  272  				err = ops->pte_hole(addr, next, 0, walk);
fafaa4264eba49 Naoya Horiguchi    2015-02-11  273  				if (err)
40d6cb9177b6a9 Rolf Eike Beer     2022-08-22  274  					return err;
40d6cb9177b6a9 Rolf Eike Beer     2022-08-22  275  			}
fafaa4264eba49 Naoya Horiguchi    2015-02-11  276  			continue;
fafaa4264eba49 Naoya Horiguchi    2015-02-11  277  		}
3afc423632a194 Steven Price       2020-02-03  278  		if (ops->pgd_entry) {
3afc423632a194 Steven Price       2020-02-03  279  			err = ops->pgd_entry(pgd, addr, next, walk);
3afc423632a194 Steven Price       2020-02-03  280  			if (err)
40d6cb9177b6a9 Rolf Eike Beer     2022-08-22  281  				return err;
3afc423632a194 Steven Price       2020-02-03  282  		}
e17eae2b839937 Christophe Leroy   2021-06-28  283  		if (is_hugepd(__hugepd(pgd_val(*pgd))))
e17eae2b839937 Christophe Leroy   2021-06-28  284  			err = walk_hugepd_range((hugepd_t *)pgd, addr, next, walk, PGDIR_SHIFT);
e17eae2b839937 Christophe Leroy   2021-06-28  285  		else if (ops->p4d_entry || ops->pud_entry || ops->pmd_entry || ops->pte_entry)
c2febafc67734a Kirill A. Shutemov 2017-03-09  286  			err = walk_p4d_range(pgd, addr, next, walk);
fafaa4264eba49 Naoya Horiguchi    2015-02-11 @287  		if (err)
40d6cb9177b6a9 Rolf Eike Beer     2022-08-22  288  			return err;
fafaa4264eba49 Naoya Horiguchi    2015-02-11  289  	} while (pgd++, addr = next, addr != end);
fafaa4264eba49 Naoya Horiguchi    2015-02-11  290  
40d6cb9177b6a9 Rolf Eike Beer     2022-08-22  291  	return 0;
fafaa4264eba49 Naoya Horiguchi    2015-02-11  292  }
fafaa4264eba49 Naoya Horiguchi    2015-02-11  293  
116354d177ba2d Naoya Horiguchi    2010-04-06  294  #ifdef CONFIG_HUGETLB_PAGE
116354d177ba2d Naoya Horiguchi    2010-04-06  295  static unsigned long hugetlb_entry_end(struct hstate *h, unsigned long addr,
116354d177ba2d Naoya Horiguchi    2010-04-06  296  				       unsigned long end)
116354d177ba2d Naoya Horiguchi    2010-04-06  297  {
116354d177ba2d Naoya Horiguchi    2010-04-06  298  	unsigned long boundary = (addr & huge_page_mask(h)) + huge_page_size(h);
116354d177ba2d Naoya Horiguchi    2010-04-06  299  	return boundary < end ? boundary : end;
116354d177ba2d Naoya Horiguchi    2010-04-06  300  }
116354d177ba2d Naoya Horiguchi    2010-04-06  301  
fafaa4264eba49 Naoya Horiguchi    2015-02-11  302  static int walk_hugetlb_range(unsigned long addr, unsigned long end,
116354d177ba2d Naoya Horiguchi    2010-04-06  303  			      struct mm_walk *walk)
116354d177ba2d Naoya Horiguchi    2010-04-06  304  {
fafaa4264eba49 Naoya Horiguchi    2015-02-11  305  	struct vm_area_struct *vma = walk->vma;
116354d177ba2d Naoya Horiguchi    2010-04-06  306  	struct hstate *h = hstate_vma(vma);
116354d177ba2d Naoya Horiguchi    2010-04-06  307  	unsigned long next;
116354d177ba2d Naoya Horiguchi    2010-04-06  308  	unsigned long hmask = huge_page_mask(h);
7868a2087ec13e Punit Agrawal      2017-07-06  309  	unsigned long sz = huge_page_size(h);
116354d177ba2d Naoya Horiguchi    2010-04-06  310  	pte_t *pte;
7b86ac3371b70c Christoph Hellwig  2019-08-28  311  	const struct mm_walk_ops *ops = walk->ops;
116354d177ba2d Naoya Horiguchi    2010-04-06  312  
116354d177ba2d Naoya Horiguchi    2010-04-06  313  	do {
40d6cb9177b6a9 Rolf Eike Beer     2022-08-22  314  		int err;
40d6cb9177b6a9 Rolf Eike Beer     2022-08-22  315  
116354d177ba2d Naoya Horiguchi    2010-04-06  316  		next = hugetlb_entry_end(h, addr, end);
7868a2087ec13e Punit Agrawal      2017-07-06  317  		pte = huge_pte_offset(walk->mm, addr & hmask, sz);
373c4557d2aa36 Jann Horn          2017-11-14  318  
373c4557d2aa36 Jann Horn          2017-11-14  319  		if (pte)
7b86ac3371b70c Christoph Hellwig  2019-08-28  320  			err = ops->hugetlb_entry(pte, hmask, addr, next, walk);
7b86ac3371b70c Christoph Hellwig  2019-08-28  321  		else if (ops->pte_hole)
b7a16c7ad790d0 Steven Price       2020-02-03  322  			err = ops->pte_hole(addr, next, -1, walk);
373c4557d2aa36 Jann Horn          2017-11-14  323  
116354d177ba2d Naoya Horiguchi    2010-04-06 @324  		if (err)
40d6cb9177b6a9 Rolf Eike Beer     2022-08-22  325  			return err;
116354d177ba2d Naoya Horiguchi    2010-04-06  326  	} while (addr = next, addr != end);
116354d177ba2d Naoya Horiguchi    2010-04-06  327  
40d6cb9177b6a9 Rolf Eike Beer     2022-08-22  328  	return 0;
116354d177ba2d Naoya Horiguchi    2010-04-06  329  }
6c6d5280431544 KOSAKI Motohiro    2011-07-25  330  

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

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

[Dan Carpenter]

[-- Attachment #1: Type: text/plain, Size: 9860 bytes --]

Hi Rolf,

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Rolf-Eike-Beer/Minor-improvements-for-pagewalk-code/20220822-214107
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
config: x86_64-randconfig-m001 (https://download.01.org/0day-ci/archive/20220823/202208231649.7QKavhA3-lkp(a)intel.com/config)
compiler: gcc-11 (Debian 11.3.0-5) 11.3.0

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
mm/pagewalk.c:248 walk_p4d_range() error: uninitialized symbol 'err'.
mm/pagewalk.c:287 walk_pgd_range() error: uninitialized symbol 'err'.
mm/pagewalk.c:324 walk_hugetlb_range() error: uninitialized symbol 'err'.

vim +/err +248 mm/pagewalk.c

c2febafc67734a Kirill A. Shutemov 2017-03-09  218  static int walk_p4d_range(pgd_t *pgd, unsigned long addr, unsigned long end,
c2febafc67734a Kirill A. Shutemov 2017-03-09  219  			  struct mm_walk *walk)
c2febafc67734a Kirill A. Shutemov 2017-03-09  220  {
c2febafc67734a Kirill A. Shutemov 2017-03-09  221  	p4d_t *p4d;
c2febafc67734a Kirill A. Shutemov 2017-03-09  222  	unsigned long next;
7b86ac3371b70c Christoph Hellwig  2019-08-28  223  	const struct mm_walk_ops *ops = walk->ops;
b7a16c7ad790d0 Steven Price       2020-02-03  224  	int depth = real_depth(1);
c2febafc67734a Kirill A. Shutemov 2017-03-09  225  
c2febafc67734a Kirill A. Shutemov 2017-03-09  226  	p4d = p4d_offset(pgd, addr);
c2febafc67734a Kirill A. Shutemov 2017-03-09  227  	do {
40d6cb9177b6a9 Rolf Eike Beer     2022-08-22  228  		int err;
40d6cb9177b6a9 Rolf Eike Beer     2022-08-22  229  
c2febafc67734a Kirill A. Shutemov 2017-03-09  230  		next = p4d_addr_end(addr, end);
c2febafc67734a Kirill A. Shutemov 2017-03-09  231  		if (p4d_none_or_clear_bad(p4d)) {
40d6cb9177b6a9 Rolf Eike Beer     2022-08-22  232  			if (ops->pte_hole) {
b7a16c7ad790d0 Steven Price       2020-02-03  233  				err = ops->pte_hole(addr, next, depth, walk);
c2febafc67734a Kirill A. Shutemov 2017-03-09  234  				if (err)
40d6cb9177b6a9 Rolf Eike Beer     2022-08-22  235  					return err;
40d6cb9177b6a9 Rolf Eike Beer     2022-08-22  236  			}
c2febafc67734a Kirill A. Shutemov 2017-03-09  237  			continue;
c2febafc67734a Kirill A. Shutemov 2017-03-09  238  		}
3afc423632a194 Steven Price       2020-02-03  239  		if (ops->p4d_entry) {
3afc423632a194 Steven Price       2020-02-03  240  			err = ops->p4d_entry(p4d, addr, next, walk);
3afc423632a194 Steven Price       2020-02-03  241  			if (err)
40d6cb9177b6a9 Rolf Eike Beer     2022-08-22  242  				return err;
3afc423632a194 Steven Price       2020-02-03  243  		}
e17eae2b839937 Christophe Leroy   2021-06-28  244  		if (is_hugepd(__hugepd(p4d_val(*p4d))))
e17eae2b839937 Christophe Leroy   2021-06-28  245  			err = walk_hugepd_range((hugepd_t *)p4d, addr, next, walk, P4D_SHIFT);
e17eae2b839937 Christophe Leroy   2021-06-28  246  		else if (ops->pud_entry || ops->pmd_entry || ops->pte_entry)
c2febafc67734a Kirill A. Shutemov 2017-03-09  247  			err = walk_pud_range(p4d, addr, next, walk);

Smatch complaining that every if statement might be false.  Seems
unlikely, but I don't know the code.  Btw, eventually we'll turn on the
GCC extension to zero out stack variables to avoid uninitialized
variables so there is not really a performance impact of setting err = 0
at the start.

c2febafc67734a Kirill A. Shutemov 2017-03-09 @248  		if (err)
40d6cb9177b6a9 Rolf Eike Beer     2022-08-22  249  			return err;
c2febafc67734a Kirill A. Shutemov 2017-03-09  250  	} while (p4d++, addr = next, addr != end);
c2febafc67734a Kirill A. Shutemov 2017-03-09  251  
40d6cb9177b6a9 Rolf Eike Beer     2022-08-22  252  	return 0;
c2febafc67734a Kirill A. Shutemov 2017-03-09  253  }
c2febafc67734a Kirill A. Shutemov 2017-03-09  254  
fafaa4264eba49 Naoya Horiguchi    2015-02-11  255  static int walk_pgd_range(unsigned long addr, unsigned long end,
fafaa4264eba49 Naoya Horiguchi    2015-02-11  256  			  struct mm_walk *walk)
fafaa4264eba49 Naoya Horiguchi    2015-02-11  257  {
fafaa4264eba49 Naoya Horiguchi    2015-02-11  258  	pgd_t *pgd;
fafaa4264eba49 Naoya Horiguchi    2015-02-11  259  	unsigned long next;
7b86ac3371b70c Christoph Hellwig  2019-08-28  260  	const struct mm_walk_ops *ops = walk->ops;
fafaa4264eba49 Naoya Horiguchi    2015-02-11  261  
e47690d756a760 Steven Price       2020-02-03  262  	if (walk->pgd)
e47690d756a760 Steven Price       2020-02-03  263  		pgd = walk->pgd + pgd_index(addr);
e47690d756a760 Steven Price       2020-02-03  264  	else
fafaa4264eba49 Naoya Horiguchi    2015-02-11  265  		pgd = pgd_offset(walk->mm, addr);
fafaa4264eba49 Naoya Horiguchi    2015-02-11  266  	do {
40d6cb9177b6a9 Rolf Eike Beer     2022-08-22  267  		int err;
40d6cb9177b6a9 Rolf Eike Beer     2022-08-22  268  
fafaa4264eba49 Naoya Horiguchi    2015-02-11  269  		next = pgd_addr_end(addr, end);
fafaa4264eba49 Naoya Horiguchi    2015-02-11  270  		if (pgd_none_or_clear_bad(pgd)) {
40d6cb9177b6a9 Rolf Eike Beer     2022-08-22  271  			if (ops->pte_hole) {
b7a16c7ad790d0 Steven Price       2020-02-03  272  				err = ops->pte_hole(addr, next, 0, walk);
fafaa4264eba49 Naoya Horiguchi    2015-02-11  273  				if (err)
40d6cb9177b6a9 Rolf Eike Beer     2022-08-22  274  					return err;
40d6cb9177b6a9 Rolf Eike Beer     2022-08-22  275  			}
fafaa4264eba49 Naoya Horiguchi    2015-02-11  276  			continue;
fafaa4264eba49 Naoya Horiguchi    2015-02-11  277  		}
3afc423632a194 Steven Price       2020-02-03  278  		if (ops->pgd_entry) {
3afc423632a194 Steven Price       2020-02-03  279  			err = ops->pgd_entry(pgd, addr, next, walk);
3afc423632a194 Steven Price       2020-02-03  280  			if (err)
40d6cb9177b6a9 Rolf Eike Beer     2022-08-22  281  				return err;
3afc423632a194 Steven Price       2020-02-03  282  		}
e17eae2b839937 Christophe Leroy   2021-06-28  283  		if (is_hugepd(__hugepd(pgd_val(*pgd))))
e17eae2b839937 Christophe Leroy   2021-06-28  284  			err = walk_hugepd_range((hugepd_t *)pgd, addr, next, walk, PGDIR_SHIFT);
e17eae2b839937 Christophe Leroy   2021-06-28  285  		else if (ops->p4d_entry || ops->pud_entry || ops->pmd_entry || ops->pte_entry)
c2febafc67734a Kirill A. Shutemov 2017-03-09  286  			err = walk_p4d_range(pgd, addr, next, walk);
fafaa4264eba49 Naoya Horiguchi    2015-02-11 @287  		if (err)

Same.

40d6cb9177b6a9 Rolf Eike Beer     2022-08-22  288  			return err;
fafaa4264eba49 Naoya Horiguchi    2015-02-11  289  	} while (pgd++, addr = next, addr != end);
fafaa4264eba49 Naoya Horiguchi    2015-02-11  290  
40d6cb9177b6a9 Rolf Eike Beer     2022-08-22  291  	return 0;
fafaa4264eba49 Naoya Horiguchi    2015-02-11  292  }
fafaa4264eba49 Naoya Horiguchi    2015-02-11  293  
116354d177ba2d Naoya Horiguchi    2010-04-06  294  #ifdef CONFIG_HUGETLB_PAGE
116354d177ba2d Naoya Horiguchi    2010-04-06  295  static unsigned long hugetlb_entry_end(struct hstate *h, unsigned long addr,
116354d177ba2d Naoya Horiguchi    2010-04-06  296  				       unsigned long end)
116354d177ba2d Naoya Horiguchi    2010-04-06  297  {
116354d177ba2d Naoya Horiguchi    2010-04-06  298  	unsigned long boundary = (addr & huge_page_mask(h)) + huge_page_size(h);
116354d177ba2d Naoya Horiguchi    2010-04-06  299  	return boundary < end ? boundary : end;
116354d177ba2d Naoya Horiguchi    2010-04-06  300  }
116354d177ba2d Naoya Horiguchi    2010-04-06  301  
fafaa4264eba49 Naoya Horiguchi    2015-02-11  302  static int walk_hugetlb_range(unsigned long addr, unsigned long end,
116354d177ba2d Naoya Horiguchi    2010-04-06  303  			      struct mm_walk *walk)
116354d177ba2d Naoya Horiguchi    2010-04-06  304  {
fafaa4264eba49 Naoya Horiguchi    2015-02-11  305  	struct vm_area_struct *vma = walk->vma;
116354d177ba2d Naoya Horiguchi    2010-04-06  306  	struct hstate *h = hstate_vma(vma);
116354d177ba2d Naoya Horiguchi    2010-04-06  307  	unsigned long next;
116354d177ba2d Naoya Horiguchi    2010-04-06  308  	unsigned long hmask = huge_page_mask(h);
7868a2087ec13e Punit Agrawal      2017-07-06  309  	unsigned long sz = huge_page_size(h);
116354d177ba2d Naoya Horiguchi    2010-04-06  310  	pte_t *pte;
7b86ac3371b70c Christoph Hellwig  2019-08-28  311  	const struct mm_walk_ops *ops = walk->ops;
116354d177ba2d Naoya Horiguchi    2010-04-06  312  
116354d177ba2d Naoya Horiguchi    2010-04-06  313  	do {
40d6cb9177b6a9 Rolf Eike Beer     2022-08-22  314  		int err;
40d6cb9177b6a9 Rolf Eike Beer     2022-08-22  315  
116354d177ba2d Naoya Horiguchi    2010-04-06  316  		next = hugetlb_entry_end(h, addr, end);
7868a2087ec13e Punit Agrawal      2017-07-06  317  		pte = huge_pte_offset(walk->mm, addr & hmask, sz);
373c4557d2aa36 Jann Horn          2017-11-14  318  
373c4557d2aa36 Jann Horn          2017-11-14  319  		if (pte)
7b86ac3371b70c Christoph Hellwig  2019-08-28  320  			err = ops->hugetlb_entry(pte, hmask, addr, next, walk);
7b86ac3371b70c Christoph Hellwig  2019-08-28  321  		else if (ops->pte_hole)
b7a16c7ad790d0 Steven Price       2020-02-03  322  			err = ops->pte_hole(addr, next, -1, walk);

Same.  Although this seems more like a real bug.

373c4557d2aa36 Jann Horn          2017-11-14  323  
116354d177ba2d Naoya Horiguchi    2010-04-06 @324  		if (err)
40d6cb9177b6a9 Rolf Eike Beer     2022-08-22  325  			return err;
116354d177ba2d Naoya Horiguchi    2010-04-06  326  	} while (addr = next, addr != end);
116354d177ba2d Naoya Horiguchi    2010-04-06  327  
40d6cb9177b6a9 Rolf Eike Beer     2022-08-22  328  	return 0;
116354d177ba2d Naoya Horiguchi    2010-04-06  329  }

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

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

[Rolf Eike Beer]

[-- Attachment #1: Type: text/plain, Size: 1605 bytes --]

Am Dienstag, 23. August 2022, 11:16:42 CEST schrieb Dan Carpenter:
> Hi Rolf,
> 
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> 
> url:   
> https://github.com/intel-lab-lkp/linux/commits/Rolf-Eike-Beer/Minor-improve
> ments-for-pagewalk-code/20220822-214107 base:  
> https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
> config: x86_64-randconfig-m001
> (https://download.01.org/0day-ci/archive/20220823/202208231649.7QKavhA3-lkp
> @intel.com/config) compiler: gcc-11 (Debian 11.3.0-5) 11.3.0
> 
> If you fix the issue, kindly add following tag where applicable
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> smatch warnings:
> mm/pagewalk.c:248 walk_p4d_range() error: uninitialized symbol 'err'.
> mm/pagewalk.c:287 walk_pgd_range() error: uninitialized symbol 'err'.
> mm/pagewalk.c:324 walk_hugetlb_range() error: uninitialized symbol 'err'.
> 
> vim +/err +248 mm/pagewalk.c

[...]

> Same.  Although this seems more like a real bug.

They all are. I wonder how they got lost and why this never showed up during 
my tests, but still.

Andrew, do you want me to resend the series with the fixes squashed in or add a 
v7?

Eike
-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 313 bytes --]

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

[Rolf Eike Beer]

[-- Attachment #1: Type: text/plain, Size: 1605 bytes --]

Am Dienstag, 23. August 2022, 11:16:42 CEST schrieb Dan Carpenter:
> Hi Rolf,
> 
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> 
> url:   
> https://github.com/intel-lab-lkp/linux/commits/Rolf-Eike-Beer/Minor-improve
> ments-for-pagewalk-code/20220822-214107 base:  
> https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
> config: x86_64-randconfig-m001
> (https://download.01.org/0day-ci/archive/20220823/202208231649.7QKavhA3-lkp
> @intel.com/config) compiler: gcc-11 (Debian 11.3.0-5) 11.3.0
> 
> If you fix the issue, kindly add following tag where applicable
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> smatch warnings:
> mm/pagewalk.c:248 walk_p4d_range() error: uninitialized symbol 'err'.
> mm/pagewalk.c:287 walk_pgd_range() error: uninitialized symbol 'err'.
> mm/pagewalk.c:324 walk_hugetlb_range() error: uninitialized symbol 'err'.
> 
> vim +/err +248 mm/pagewalk.c

[...]

> Same.  Although this seems more like a real bug.

They all are. I wonder how they got lost and why this never showed up during 
my tests, but still.

Andrew, do you want me to resend the series with the fixes squashed in or add a 
v7?

Eike
-- 
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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 313 bytes --]

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

[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 <lkft@linaro.org>
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Rolf Eike Beer <eb@emlix.com>
---
 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.

diff --git a/mm/pagewalk.c b/mm/pagewalk.c
index d8be8a30b272..22e4bcca9bee 100644
--- a/mm/pagewalk.c
+++ b/mm/pagewalk.c
@@ -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);
-- 
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

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

[Nathan Chancellor]

On Wed, Aug 24, 2022 at 01:00:11PM +0200, Rolf Eike Beer wrote:
> 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 <lkft@linaro.org>
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Rolf Eike Beer <eb@emlix.com>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  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.
> 
> diff --git a/mm/pagewalk.c b/mm/pagewalk.c
> index d8be8a30b272..22e4bcca9bee 100644
> --- a/mm/pagewalk.c
> +++ b/mm/pagewalk.c
> @@ -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);
> -- 
> 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
> 
>