[PATCH] mm/mempool: warn about __GFP_ZERO usage

[PATCH] mm/mempool: warn about __GFP_ZERO usage

[Sebastian Ott]

Hello,

I recently found out the hard way, that using mempool_alloc together with
__GFP_ZERO is not a good idea since memory which comes from the pool of
preallocated elemtents is not zeroed. Fixing this doesn't seem to be trivial
since mempool is not aware of the size of the objects it manages.

Last time someone addressed this on lkml just the callers of mempool_alloc
were fixed which obviously didn't help new users of mempool...
How about the following patch?

Regards,
Sebastian
---

mm/mempool: warn about __GFP_ZERO usage

Memory obtained via mempool_alloc is not always zeroed even when
called with __GFP_ZERO. Add a note and VM_BUG_ON statement to make
that clear.

Signed-off-by: Sebastian Ott <sebott@linux.vnet.ibm.com>
---
 mm/mempool.c |    2 ++
 1 file changed, 2 insertions(+)

--- a/mm/mempool.c
+++ b/mm/mempool.c
@@ -192,6 +192,7 @@ EXPORT_SYMBOL(mempool_resize);
  * returns NULL. Note that due to preallocation, this function
  * *never* fails when called from process contexts. (it might
  * fail if called from an IRQ context.)
+ * Note: using __GFP_ZERO is not supported.
  */
 void * mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
 {
@@ -200,6 +201,7 @@ void * mempool_alloc(mempool_t *pool, gf
 	wait_queue_t wait;
 	gfp_t gfp_temp;
 
+	VM_BUG_ON(gfp_mask & __GFP_ZERO);
 	might_sleep_if(gfp_mask & __GFP_WAIT);
 
 	gfp_mask |= __GFP_NOMEMALLOC;	/* don't allocate emergency reserves */

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm/mempool: warn about __GFP_ZERO usage

[Andrew Morton]

On Mon, 14 Apr 2014 19:23:34 +0200 (CEST) Sebastian Ott <sebott@linux.vnet.ibm.com> wrote:

> Memory obtained via mempool_alloc is not always zeroed even when
> called with __GFP_ZERO. Add a note and VM_BUG_ON statement to make
> that clear.
> 
> ..
>
> @@ -200,6 +201,7 @@ void * mempool_alloc(mempool_t *pool, gf
>  	wait_queue_t wait;
>  	gfp_t gfp_temp;
>  
> +	VM_BUG_ON(gfp_mask & __GFP_ZERO);
>  	might_sleep_if(gfp_mask & __GFP_WAIT);
>  
>  	gfp_mask |= __GFP_NOMEMALLOC;	/* don't allocate emergency reserves */

hm, BUG is a bit harsh.  How about

--- a/mm/mempool.c~mm-mempool-warn-about-__gfp_zero-usage-fix
+++ a/mm/mempool.c
@@ -201,7 +201,9 @@ void * mempool_alloc(mempool_t *pool, gf
 	wait_queue_t wait;
 	gfp_t gfp_temp;
 
-	VM_BUG_ON(gfp_mask & __GFP_ZERO);
+#ifdef CONFIG_DEBUG_VM
+	WARN_ON_ONCE(gfp_mask & __GFP_ZERO);	/* Where's VM_WARN_ON_ONCE()? */
+#endif
 	might_sleep_if(gfp_mask & __GFP_WAIT);
 
 	gfp_mask |= __GFP_NOMEMALLOC;	/* don't allocate emergency reserves */

?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm/mempool: warn about __GFP_ZERO usage

[Andrew Morton]

On Wed, 16 Apr 2014 14:15:56 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:

> @@ -201,7 +201,9 @@ void * mempool_alloc(mempool_t *pool, gf
>  	wait_queue_t wait;
>  	gfp_t gfp_temp;
>  
> -	VM_BUG_ON(gfp_mask & __GFP_ZERO);
> +#ifdef CONFIG_DEBUG_VM
> +	WARN_ON_ONCE(gfp_mask & __GFP_ZERO);	/* Where's VM_WARN_ON_ONCE()? */
> +#endif

bah, don't be lazy.

From: Andrew Morton <akpm@linux-foundation.org>
Subject: include/linux/mmdebug.h: add VM_WARN_ON() and VM_WARN_ON_ONCE()

WARN_ON() and WARN_ON_ONCE(), dependent on CONFIG_DEBUG_VM

Cc: Sebastian Ott <sebott@linux.vnet.ibm.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 include/linux/mmdebug.h |    4 ++++
 1 file changed, 4 insertions(+)

diff -puN include/linux/mmdebug.h~include-linux-mmdebugh-add-vm_warn_on-and-vm_warn_on_once include/linux/mmdebug.h
--- a/include/linux/mmdebug.h~include-linux-mmdebugh-add-vm_warn_on-and-vm_warn_on_once
+++ a/include/linux/mmdebug.h
@@ -11,9 +11,13 @@ extern void dump_page_badflags(struct pa
 #define VM_BUG_ON(cond) BUG_ON(cond)
 #define VM_BUG_ON_PAGE(cond, page) \
 	do { if (unlikely(cond)) { dump_page(page, NULL); BUG(); } } while (0)
+#define VM_WARN_ON(cond) WARN_ON(cond)
+#define VM_WARN_ON_ONCE(cond) WARN_ON_ONCE(cond)
 #else
 #define VM_BUG_ON(cond) BUILD_BUG_ON_INVALID(cond)
 #define VM_BUG_ON_PAGE(cond, page) VM_BUG_ON(cond)
+#define VM_WARN_ON(cond) BUILD_BUG_ON_INVALID(cond)
+#define VM_WARN_ON_ONCE(cond) BUILD_BUG_ON_INVALID(cond)
 #endif
 
 #ifdef CONFIG_DEBUG_VIRTUAL
_

From: Andrew Morton <akpm@linux-foundation.org>
Subject: mm-mempool-warn-about-__gfp_zero-usage-fix

use VM_WARN_ON_ONCE

Cc: Sebastian Ott <sebott@linux.vnet.ibm.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

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

diff -puN mm/mempool.c~mm-mempool-warn-about-__gfp_zero-usage-fix mm/mempool.c
--- a/mm/mempool.c~mm-mempool-warn-about-__gfp_zero-usage-fix
+++ a/mm/mempool.c
@@ -201,7 +201,7 @@ void * mempool_alloc(mempool_t *pool, gf
 	wait_queue_t wait;
 	gfp_t gfp_temp;
 
-	VM_BUG_ON(gfp_mask & __GFP_ZERO);
+	VM_WARN_ON_ONCE(gfp_mask & __GFP_ZERO);
 	might_sleep_if(gfp_mask & __GFP_WAIT);
 
 	gfp_mask |= __GFP_NOMEMALLOC;	/* don't allocate emergency reserves */
_

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>