linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [bug report] mm-add-apply_to_existing_pages-helper-fix
@ 2019-12-12  9:29 Dan Carpenter
  2019-12-13  1:03 ` Andrew Morton
  0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2019-12-12  9:29 UTC (permalink / raw)
  To: akpm; +Cc: linux-mm

Hello Andrew Morton,

The patch 3264ba9274f5: "mm-add-apply_to_existing_pages-helper-fix"
from Dec 11, 2019, leads to the following static checker warning:

	mm/memory.c:2166 __apply_to_page_range()
	error: uninitialized symbol 'err'.

mm/memory.c
  2144  static int __apply_to_page_range(struct mm_struct *mm, unsigned long addr,
  2145                                   unsigned long size, pte_fn_t fn,
  2146                                   void *data, bool create)
  2147  {
  2148          pgd_t *pgd;
  2149          unsigned long next;
  2150          unsigned long end = addr + size;
  2151          int err;
  2152  
  2153          if (WARN_ON(addr >= end))
  2154                  return -EINVAL;
  2155  
  2156          pgd = pgd_offset(mm, addr);
  2157          do {
  2158                  next = pgd_addr_end(addr, end);
  2159                  if (!create && pgd_none_or_clear_bad(pgd))
  2160                          continue;
                                ^^^^^^^^^
It feels unlikely that we would always hit this continue but Smatch
complains.

  2161                  err = apply_to_p4d_range(mm, pgd, addr, next, fn, data, create);
  2162                  if (err)
  2163                          break;
  2164          } while (pgd++, addr = next, addr != end);
  2165  
  2166          return err;
  2167  }

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [bug report] mm-add-apply_to_existing_pages-helper-fix
  2019-12-12  9:29 [bug report] mm-add-apply_to_existing_pages-helper-fix Dan Carpenter
@ 2019-12-13  1:03 ` Andrew Morton
  2019-12-13  2:04   ` Daniel Axtens
  0 siblings, 1 reply; 3+ messages in thread
From: Andrew Morton @ 2019-12-13  1:03 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: linux-mm, Daniel Axtens

On Thu, 12 Dec 2019 12:29:49 +0300 Dan Carpenter <dan.carpenter@oracle.com> wrote:

> Hello Andrew Morton,
> 
> The patch 3264ba9274f5: "mm-add-apply_to_existing_pages-helper-fix"
> from Dec 11, 2019, leads to the following static checker warning:
> 
> 	mm/memory.c:2166 __apply_to_page_range()
> 	error: uninitialized symbol 'err'.
> 
> mm/memory.c
>   2144  static int __apply_to_page_range(struct mm_struct *mm, unsigned long addr,
>   2145                                   unsigned long size, pte_fn_t fn,
>   2146                                   void *data, bool create)
>   2147  {
>   2148          pgd_t *pgd;
>   2149          unsigned long next;
>   2150          unsigned long end = addr + size;
>   2151          int err;
>   2152  
>   2153          if (WARN_ON(addr >= end))
>   2154                  return -EINVAL;
>   2155  
>   2156          pgd = pgd_offset(mm, addr);
>   2157          do {
>   2158                  next = pgd_addr_end(addr, end);
>   2159                  if (!create && pgd_none_or_clear_bad(pgd))
>   2160                          continue;
>                                 ^^^^^^^^^
> It feels unlikely that we would always hit this continue but Smatch
> complains.
> 

Thanks.

--- a/mm/memory.c~mm-add-apply_to_existing_pages-helper-fix-fix-fix
+++ a/mm/memory.c
@@ -2148,7 +2148,7 @@ static int __apply_to_page_range(struct
 	pgd_t *pgd;
 	unsigned long next;
 	unsigned long end = addr + size;
-	int err;
+	int err = 0;
 
 	if (WARN_ON(addr >= end))
 		return -EINVAL;

Daniel, have you had a chance to runtime test these various fiddles?

From: Andrew Morton <akpm@linux-foundation.org>
Subject: mm-add-apply_to_existing_pages-helper-fix

reduce code duplication

Cc: Daniel Axtens <dja@axtens.net>
Cc: Alexander Potapenko <glider@google.com>
Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Qian Cai <cai@lca.pw>
Cc: Uladzislau Rezki (Sony) <urezki@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 mm/memory.c |   43 +++++++++++++++++--------------------------
 1 file changed, 17 insertions(+), 26 deletions(-)

--- a/mm/memory.c~mm-add-apply_to_existing_pages-helper-fix
+++ a/mm/memory.c
@@ -2141,12 +2141,9 @@ static int apply_to_p4d_range(struct mm_
 	return err;
 }
 
-/*
- * Scan a region of virtual memory, filling in page tables as necessary
- * and calling a provided function on each leaf page table.
- */
-int apply_to_page_range(struct mm_struct *mm, unsigned long addr,
-			unsigned long size, pte_fn_t fn, void *data)
+static int __apply_to_page_range(struct mm_struct *mm, unsigned long addr,
+				 unsigned long size, pte_fn_t fn,
+				 void *data, bool create)
 {
 	pgd_t *pgd;
 	unsigned long next;
@@ -2159,13 +2156,25 @@ int apply_to_page_range(struct mm_struct
 	pgd = pgd_offset(mm, addr);
 	do {
 		next = pgd_addr_end(addr, end);
-		err = apply_to_p4d_range(mm, pgd, addr, next, fn, data, true);
+		if (!create && pgd_none_or_clear_bad(pgd))
+			continue;
+		err = apply_to_p4d_range(mm, pgd, addr, next, fn, data, create);
 		if (err)
 			break;
 	} while (pgd++, addr = next, addr != end);
 
 	return err;
 }
+
+/*
+ * Scan a region of virtual memory, filling in page tables as necessary
+ * and calling a provided function on each leaf page table.
+ */
+int apply_to_page_range(struct mm_struct *mm, unsigned long addr,
+			unsigned long size, pte_fn_t fn, void *data)
+{
+	return __apply_to_page_range(mm, addr, size, fn, data, true);
+}
 EXPORT_SYMBOL_GPL(apply_to_page_range);
 
 /*
@@ -2178,25 +2187,7 @@ EXPORT_SYMBOL_GPL(apply_to_page_range);
 int apply_to_existing_pages(struct mm_struct *mm, unsigned long addr,
 			    unsigned long size, pte_fn_t fn, void *data)
 {
-	pgd_t *pgd;
-	unsigned long next;
-	unsigned long end = addr + size;
-	int err = 0;
-
-	if (WARN_ON(addr >= end))
-		return -EINVAL;
-
-	pgd = pgd_offset(mm, addr);
-	do {
-		next = pgd_addr_end(addr, end);
-		if (pgd_none_or_clear_bad(pgd))
-			continue;
-		err = apply_to_p4d_range(mm, pgd, addr, next, fn, data, false);
-		if (err)
-			break;
-	} while (pgd++, addr = next, addr != end);
-
-	return err;
+	return __apply_to_page_range(mm, addr, size, fn, data, false);
 }
 EXPORT_SYMBOL_GPL(apply_to_existing_pages);
 
_


From: Andrew Morton <akpm@linux-foundation.org>
Subject: mm-add-apply_to_existing_pages-helper-fix-fix

s/apply_to_existing_pages/apply_to_existing_page_range/

Cc: Daniel Axtens <dja@axtens.net>
Cc: Alexander Potapenko <glider@google.com>
Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Qian Cai <cai@lca.pw>
Cc: Uladzislau Rezki (Sony) <urezki@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 include/linux/mm.h |    6 +++---
 mm/memory.c        |    6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

--- a/include/linux/mm.h~mm-add-apply_to_existing_pages-helper-fix-fix
+++ a/include/linux/mm.h
@@ -2621,9 +2621,9 @@ static inline int vm_fault_to_errno(vm_f
 typedef int (*pte_fn_t)(pte_t *pte, unsigned long addr, void *data);
 extern int apply_to_page_range(struct mm_struct *mm, unsigned long address,
 			       unsigned long size, pte_fn_t fn, void *data);
-extern int apply_to_existing_pages(struct mm_struct *mm, unsigned long address,
-				   unsigned long size, pte_fn_t fn,
-				   void *data);
+extern int apply_to_existing_page_range(struct mm_struct *mm,
+				   unsigned long address, unsigned long size,
+				   pte_fn_t fn, void *data);
 
 #ifdef CONFIG_PAGE_POISONING
 extern bool page_poisoning_enabled(void);
--- a/mm/memory.c~mm-add-apply_to_existing_pages-helper-fix-fix
+++ a/mm/memory.c
@@ -2184,12 +2184,12 @@ EXPORT_SYMBOL_GPL(apply_to_page_range);
  * Unlike apply_to_page_range, this does _not_ fill in page tables
  * where they are absent.
  */
-int apply_to_existing_pages(struct mm_struct *mm, unsigned long addr,
-			    unsigned long size, pte_fn_t fn, void *data)
+int apply_to_existing_page_range(struct mm_struct *mm, unsigned long addr,
+				 unsigned long size, pte_fn_t fn, void *data)
 {
 	return __apply_to_page_range(mm, addr, size, fn, data, false);
 }
-EXPORT_SYMBOL_GPL(apply_to_existing_pages);
+EXPORT_SYMBOL_GPL(apply_to_existing_page_range);
 
 /*
  * handle_pte_fault chooses page fault handler according to an entry which was
_

From: Andrew Morton <akpm@linux-foundation.org>
Subject: mm-add-apply_to_existing_pages-helper-fix-fix-fix

initialize __apply_to_page_range::err

Cc: Alexander Potapenko <glider@google.com>
Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>
Cc: Daniel Axtens <dja@axtens.net>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Qian Cai <cai@lca.pw>
Cc: Uladzislau Rezki (Sony) <urezki@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

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

--- a/mm/memory.c~mm-add-apply_to_existing_pages-helper-fix-fix-fix
+++ a/mm/memory.c
@@ -2148,7 +2148,7 @@ static int __apply_to_page_range(struct
 	pgd_t *pgd;
 	unsigned long next;
 	unsigned long end = addr + size;
-	int err;
+	int err = 0;
 
 	if (WARN_ON(addr >= end))
 		return -EINVAL;
_



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [bug report] mm-add-apply_to_existing_pages-helper-fix
  2019-12-13  1:03 ` Andrew Morton
@ 2019-12-13  2:04   ` Daniel Axtens
  0 siblings, 0 replies; 3+ messages in thread
From: Daniel Axtens @ 2019-12-13  2:04 UTC (permalink / raw)
  To: Andrew Morton, Dan Carpenter; +Cc: linux-mm

Hi Andrew,

> Daniel, have you had a chance to runtime test these various fiddles?

Yes, I've been testing with -fix and -fix-fix since they hit next. They
work fine for me on x86 and powerpc.

Regards,
Daniel

>
> From: Andrew Morton <akpm@linux-foundation.org>
> Subject: mm-add-apply_to_existing_pages-helper-fix
>
> reduce code duplication
>
> Cc: Daniel Axtens <dja@axtens.net>
> Cc: Alexander Potapenko <glider@google.com>
> Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>
> Cc: Dmitry Vyukov <dvyukov@google.com>
> Cc: Qian Cai <cai@lca.pw>
> Cc: Uladzislau Rezki (Sony) <urezki@gmail.com>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
>
>  mm/memory.c |   43 +++++++++++++++++--------------------------
>  1 file changed, 17 insertions(+), 26 deletions(-)
>
> --- a/mm/memory.c~mm-add-apply_to_existing_pages-helper-fix
> +++ a/mm/memory.c
> @@ -2141,12 +2141,9 @@ static int apply_to_p4d_range(struct mm_
>  	return err;
>  }
>  
> -/*
> - * Scan a region of virtual memory, filling in page tables as necessary
> - * and calling a provided function on each leaf page table.
> - */
> -int apply_to_page_range(struct mm_struct *mm, unsigned long addr,
> -			unsigned long size, pte_fn_t fn, void *data)
> +static int __apply_to_page_range(struct mm_struct *mm, unsigned long addr,
> +				 unsigned long size, pte_fn_t fn,
> +				 void *data, bool create)
>  {
>  	pgd_t *pgd;
>  	unsigned long next;
> @@ -2159,13 +2156,25 @@ int apply_to_page_range(struct mm_struct
>  	pgd = pgd_offset(mm, addr);
>  	do {
>  		next = pgd_addr_end(addr, end);
> -		err = apply_to_p4d_range(mm, pgd, addr, next, fn, data, true);
> +		if (!create && pgd_none_or_clear_bad(pgd))
> +			continue;
> +		err = apply_to_p4d_range(mm, pgd, addr, next, fn, data, create);
>  		if (err)
>  			break;
>  	} while (pgd++, addr = next, addr != end);
>  
>  	return err;
>  }
> +
> +/*
> + * Scan a region of virtual memory, filling in page tables as necessary
> + * and calling a provided function on each leaf page table.
> + */
> +int apply_to_page_range(struct mm_struct *mm, unsigned long addr,
> +			unsigned long size, pte_fn_t fn, void *data)
> +{
> +	return __apply_to_page_range(mm, addr, size, fn, data, true);
> +}
>  EXPORT_SYMBOL_GPL(apply_to_page_range);
>  
>  /*
> @@ -2178,25 +2187,7 @@ EXPORT_SYMBOL_GPL(apply_to_page_range);
>  int apply_to_existing_pages(struct mm_struct *mm, unsigned long addr,
>  			    unsigned long size, pte_fn_t fn, void *data)
>  {
> -	pgd_t *pgd;
> -	unsigned long next;
> -	unsigned long end = addr + size;
> -	int err = 0;
> -
> -	if (WARN_ON(addr >= end))
> -		return -EINVAL;
> -
> -	pgd = pgd_offset(mm, addr);
> -	do {
> -		next = pgd_addr_end(addr, end);
> -		if (pgd_none_or_clear_bad(pgd))
> -			continue;
> -		err = apply_to_p4d_range(mm, pgd, addr, next, fn, data, false);
> -		if (err)
> -			break;
> -	} while (pgd++, addr = next, addr != end);
> -
> -	return err;
> +	return __apply_to_page_range(mm, addr, size, fn, data, false);
>  }
>  EXPORT_SYMBOL_GPL(apply_to_existing_pages);
>  
> _
>
>
> From: Andrew Morton <akpm@linux-foundation.org>
> Subject: mm-add-apply_to_existing_pages-helper-fix-fix
>
> s/apply_to_existing_pages/apply_to_existing_page_range/
>
> Cc: Daniel Axtens <dja@axtens.net>
> Cc: Alexander Potapenko <glider@google.com>
> Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>
> Cc: Dmitry Vyukov <dvyukov@google.com>
> Cc: Qian Cai <cai@lca.pw>
> Cc: Uladzislau Rezki (Sony) <urezki@gmail.com>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
>
>  include/linux/mm.h |    6 +++---
>  mm/memory.c        |    6 +++---
>  2 files changed, 6 insertions(+), 6 deletions(-)
>
> --- a/include/linux/mm.h~mm-add-apply_to_existing_pages-helper-fix-fix
> +++ a/include/linux/mm.h
> @@ -2621,9 +2621,9 @@ static inline int vm_fault_to_errno(vm_f
>  typedef int (*pte_fn_t)(pte_t *pte, unsigned long addr, void *data);
>  extern int apply_to_page_range(struct mm_struct *mm, unsigned long address,
>  			       unsigned long size, pte_fn_t fn, void *data);
> -extern int apply_to_existing_pages(struct mm_struct *mm, unsigned long address,
> -				   unsigned long size, pte_fn_t fn,
> -				   void *data);
> +extern int apply_to_existing_page_range(struct mm_struct *mm,
> +				   unsigned long address, unsigned long size,
> +				   pte_fn_t fn, void *data);
>  
>  #ifdef CONFIG_PAGE_POISONING
>  extern bool page_poisoning_enabled(void);
> --- a/mm/memory.c~mm-add-apply_to_existing_pages-helper-fix-fix
> +++ a/mm/memory.c
> @@ -2184,12 +2184,12 @@ EXPORT_SYMBOL_GPL(apply_to_page_range);
>   * Unlike apply_to_page_range, this does _not_ fill in page tables
>   * where they are absent.
>   */
> -int apply_to_existing_pages(struct mm_struct *mm, unsigned long addr,
> -			    unsigned long size, pte_fn_t fn, void *data)
> +int apply_to_existing_page_range(struct mm_struct *mm, unsigned long addr,
> +				 unsigned long size, pte_fn_t fn, void *data)
>  {
>  	return __apply_to_page_range(mm, addr, size, fn, data, false);
>  }
> -EXPORT_SYMBOL_GPL(apply_to_existing_pages);
> +EXPORT_SYMBOL_GPL(apply_to_existing_page_range);
>  
>  /*
>   * handle_pte_fault chooses page fault handler according to an entry which was
> _
>
> From: Andrew Morton <akpm@linux-foundation.org>
> Subject: mm-add-apply_to_existing_pages-helper-fix-fix-fix
>
> initialize __apply_to_page_range::err
>
> Cc: Alexander Potapenko <glider@google.com>
> Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>
> Cc: Daniel Axtens <dja@axtens.net>
> Cc: Dmitry Vyukov <dvyukov@google.com>
> Cc: Qian Cai <cai@lca.pw>
> Cc: Uladzislau Rezki (Sony) <urezki@gmail.com>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
>
>  mm/memory.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- a/mm/memory.c~mm-add-apply_to_existing_pages-helper-fix-fix-fix
> +++ a/mm/memory.c
> @@ -2148,7 +2148,7 @@ static int __apply_to_page_range(struct
>  	pgd_t *pgd;
>  	unsigned long next;
>  	unsigned long end = addr + size;
> -	int err;
> +	int err = 0;
>  
>  	if (WARN_ON(addr >= end))
>  		return -EINVAL;
> _


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-12-13  2:04 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-12  9:29 [bug report] mm-add-apply_to_existing_pages-helper-fix Dan Carpenter
2019-12-13  1:03 ` Andrew Morton
2019-12-13  2:04   ` Daniel Axtens

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox