Thread (3 messages) 3 messages, 3 authors, 2017-01-06

Re: [PATCH] mm: introduce kv[mz]alloc helpers

From: Michal Hocko <mhocko@kernel.org>
Date: 2017-01-05 10:59:38
Also in: linux-mm

On Thu 05-01-17 19:40:10, Tetsuo Handa wrote:
On 2017/01/04 23:20, Michal Hocko wrote:
quoted
OK, so I've checked the open coded implementations and converted most of
them. There are few which are either confused and need some special
handling or need double checking.
diff --git a/drivers/md/bcache/util.h b/drivers/md/bcache/util.h
index cf2cbc211d83..9dc0f0ff0321 100644
--- a/drivers/md/bcache/util.h
+++ b/drivers/md/bcache/util.h
@@ -44,10 +44,7 @@ struct closure;
 	(heap)->size = (_size);						\
 	_bytes = (heap)->size * sizeof(*(heap)->data);			\
 	(heap)->data = NULL;						\
-	if (_bytes < KMALLOC_MAX_SIZE)					\
-		(heap)->data = kmalloc(_bytes, (gfp));			\
-	if ((!(heap)->data) && ((gfp) & GFP_KERNEL))			\
-		(heap)->data = vmalloc(_bytes);				\
+	(heap)->data = kvmalloc(_bytes, (gfp) & GFP_KERNEL);		\
 	(heap)->data;							\
 })
 
@@ -138,10 +135,7 @@ do {									\
 	(fifo)->front = (fifo)->back = 0;				\
 	(fifo)->data = NULL;						\
 									\
-	if (_bytes < KMALLOC_MAX_SIZE)					\
-		(fifo)->data = kmalloc(_bytes, (gfp));			\
-	if ((!(fifo)->data) && ((gfp) & GFP_KERNEL))			\
-		(fifo)->data = vmalloc(_bytes);				\
+	(fifo)->data = kvmalloc(_bytes, (gfp) & GFP_KERNEL);		\
 	(fifo)->data;							\
 })
These macros are doing strange checks.
((gfp) & GFP_KERNEL) means any bit in GFP_KERNEL is set.
((gfp) & GFP_KERNEL) == GFP_KERNEL might make sense. Actually,
all callers seems to be passing GFP_KERNEL to these macros.
Yes the code is confused. I've seen worse when going through the drivers
code...
Kent, how do you want to correct this? You want to apply
a patch that removes gfp argument before applying this patch?
Or, you want Michal to directly overwrite by this patch?
I would just get rid of it here as init_heap has just one caller with
GFP_KERNEL and __init_fifo has GFP_KERNEL users as well. But if it is
preferable to clean up this first then I can do that.
 
Michal, "(fifo)->data = NULL;" line will become redundant
and "(gfp) & GFP_KERNEL" will become "GFP_KERNEL".
true. will remove it.

-- 
Michal Hocko
SUSE Labs

--
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>
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help