Re: [PATCH 1/6] mm: add kmalloc_array_node and kcalloc_node

[Vlastimil Babka]

On 09/27/2017 10:20 AM, Johannes Thumshirn wrote:
> We have kmalloc_array() and kcalloc() wrappers on top of kmalloc() which
> ensure us overflow free multiplication for the size of a memory
> allocation but these implementations are not NUMA-aware.
> 
> Likewise we have kmalloc_node() which is a NUMA-aware version of
> kmalloc() but the implementation is not aware of any possible overflows in
> eventual size calculations.
> 
> Introduce a combination of the two above cases to have a NUMA-node aware
> version of kmalloc_array() and kcalloc().
> 
> Signed-off-by: Johannes Thumshirn 

Sounds better than custom open-coded stuff indeed.

Acked-by: Vlastimil Babka 

> ---
>  include/linux/slab.h | 16 
>  1 file changed, 16 insertions(+)
> 
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index 41473df6dfb0..aaf4723e41b3 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -635,6 +635,22 @@ extern void *__kmalloc_track_caller(size_t, gfp_t, 
> unsigned long);
>  #define kmalloc_track_caller(size, flags) \
>   __kmalloc_track_caller(size, flags, _RET_IP_)
>  
> +static inline void *kmalloc_array_node(size_t n, size_t size, gfp_t flags,
> +int node)
> +{
> + if (size != 0 && n > SIZE_MAX / size)
> + return NULL;
> + if (__builtin_constant_p(n) && __builtin_constant_p(size))
> + return kmalloc_node(n * size, flags, node);
> + return __kmalloc_node(n * size, flags, node);
> +}
> +
> +static inline void *kcalloc_node(size_t n, size_t size, gfp_t flags, int 
> node)
> +{
> + return kmalloc_array_node(n, size, flags | __GFP_ZERO, node);
> +}
> +
> +
>  #ifdef CONFIG_NUMA
>  extern void *__kmalloc_node_track_caller(size_t, gfp_t, int, unsigned long);
>  #define kmalloc_node_track_caller(size, flags, node) \
> 

Re: [PATCH 1/6] mm: add kmalloc_array_node and kcalloc_node

[Vlastimil Babka]

On 09/27/2017 10:20 AM, Johannes Thumshirn wrote:
> We have kmalloc_array() and kcalloc() wrappers on top of kmalloc() which
> ensure us overflow free multiplication for the size of a memory
> allocation but these implementations are not NUMA-aware.
> 
> Likewise we have kmalloc_node() which is a NUMA-aware version of
> kmalloc() but the implementation is not aware of any possible overflows in
> eventual size calculations.
> 
> Introduce a combination of the two above cases to have a NUMA-node aware
> version of kmalloc_array() and kcalloc().
> 
> Signed-off-by: Johannes Thumshirn 

Sounds better than custom open-coded stuff indeed.

Acked-by: Vlastimil Babka 

> ---
>  include/linux/slab.h | 16 
>  1 file changed, 16 insertions(+)
> 
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index 41473df6dfb0..aaf4723e41b3 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -635,6 +635,22 @@ extern void *__kmalloc_track_caller(size_t, gfp_t, 
> unsigned long);
>  #define kmalloc_track_caller(size, flags) \
>   __kmalloc_track_caller(size, flags, _RET_IP_)
>  
> +static inline void *kmalloc_array_node(size_t n, size_t size, gfp_t flags,
> +int node)
> +{
> + if (size != 0 && n > SIZE_MAX / size)
> + return NULL;
> + if (__builtin_constant_p(n) && __builtin_constant_p(size))
> + return kmalloc_node(n * size, flags, node);
> + return __kmalloc_node(n * size, flags, node);
> +}
> +
> +static inline void *kcalloc_node(size_t n, size_t size, gfp_t flags, int 
> node)
> +{
> + return kmalloc_array_node(n, size, flags | __GFP_ZERO, node);
> +}
> +
> +
>  #ifdef CONFIG_NUMA
>  extern void *__kmalloc_node_track_caller(size_t, gfp_t, int, unsigned long);
>  #define kmalloc_node_track_caller(size, flags, node) \
> 

Re: [PATCH 1/6] mm: add kmalloc_array_node and kcalloc_node

[Michal Hocko]

On Wed 27-09-17 04:03:01, Cristopher Lameter wrote:
> On Wed, 27 Sep 2017, Michal Hocko wrote:
> 
> > > Introduce a combination of the two above cases to have a NUMA-node aware
> > > version of kmalloc_array() and kcalloc().
> >
> > Yes, this is helpful. I am just wondering why we cannot have
> > kmalloc_array to call kmalloc_array_node with the local node as a
> > parameter. Maybe some sort of an optimization?
> 
> Well the regular kmalloc without node is supposed to follow memory
> policies. An explicit mentioning of a node requires allocation from that
> node and will override memory allocation policies.

I see. Thanks for the clarification

-- 
Michal Hocko
SUSE Labs

Re: [PATCH 1/6] mm: add kmalloc_array_node and kcalloc_node

[Michal Hocko]

On Wed 27-09-17 04:03:01, Cristopher Lameter wrote:
> On Wed, 27 Sep 2017, Michal Hocko wrote:
> 
> > > Introduce a combination of the two above cases to have a NUMA-node aware
> > > version of kmalloc_array() and kcalloc().
> >
> > Yes, this is helpful. I am just wondering why we cannot have
> > kmalloc_array to call kmalloc_array_node with the local node as a
> > parameter. Maybe some sort of an optimization?
> 
> Well the regular kmalloc without node is supposed to follow memory
> policies. An explicit mentioning of a node requires allocation from that
> node and will override memory allocation policies.

I see. Thanks for the clarification

-- 
Michal Hocko
SUSE Labs

Re: [PATCH 1/6] mm: add kmalloc_array_node and kcalloc_node

[Christopher Lameter]

On Wed, 27 Sep 2017, Michal Hocko wrote:

> > Introduce a combination of the two above cases to have a NUMA-node aware
> > version of kmalloc_array() and kcalloc().
>
> Yes, this is helpful. I am just wondering why we cannot have
> kmalloc_array to call kmalloc_array_node with the local node as a
> parameter. Maybe some sort of an optimization?

Well the regular kmalloc without node is supposed to follow memory
policies. An explicit mentioning of a node requires allocation from that
node and will override memory allocation policies.

Note that node local policy is the default for allocations but that can be
overridden by the application or at the command line level. Assumptions
that this is always the case come up frequently but if we do that we will
loose the ability to control memory locality for user space.

Re: [PATCH 1/6] mm: add kmalloc_array_node and kcalloc_node

[Christopher Lameter]

On Wed, 27 Sep 2017, Michal Hocko wrote:

> > Introduce a combination of the two above cases to have a NUMA-node aware
> > version of kmalloc_array() and kcalloc().
>
> Yes, this is helpful. I am just wondering why we cannot have
> kmalloc_array to call kmalloc_array_node with the local node as a
> parameter. Maybe some sort of an optimization?

Well the regular kmalloc without node is supposed to follow memory
policies. An explicit mentioning of a node requires allocation from that
node and will override memory allocation policies.

Note that node local policy is the default for allocations but that can be
overridden by the application or at the command line level. Assumptions
that this is always the case come up frequently but if we do that we will
loose the ability to control memory locality for user space.

Re: [PATCH 1/6] mm: add kmalloc_array_node and kcalloc_node

[Christopher Lameter]

On Wed, 27 Sep 2017, Johannes Thumshirn wrote:

> +static inline void *kmalloc_array_node(size_t n, size_t size, gfp_t flags,
> +int node)
> +{
> + if (size != 0 && n > SIZE_MAX / size)
> + return NULL;
> + if (__builtin_constant_p(n) && __builtin_constant_p(size))
> + return kmalloc_node(n * size, flags, node);

Isnt the same check done by kmalloc_node already? The result of
multiplying two constants is a constant after all.

Re: [PATCH 1/6] mm: add kmalloc_array_node and kcalloc_node

[Christopher Lameter]

On Wed, 27 Sep 2017, Johannes Thumshirn wrote:

> +static inline void *kmalloc_array_node(size_t n, size_t size, gfp_t flags,
> +int node)
> +{
> + if (size != 0 && n > SIZE_MAX / size)
> + return NULL;
> + if (__builtin_constant_p(n) && __builtin_constant_p(size))
> + return kmalloc_node(n * size, flags, node);

Isnt the same check done by kmalloc_node already? The result of
multiplying two constants is a constant after all.

Re: [PATCH 1/6] mm: add kmalloc_array_node and kcalloc_node

[Michal Hocko]

On Wed 27-09-17 10:20:33, Johannes Thumshirn wrote:
> We have kmalloc_array() and kcalloc() wrappers on top of kmalloc() which
> ensure us overflow free multiplication for the size of a memory
> allocation but these implementations are not NUMA-aware.
> 
> Likewise we have kmalloc_node() which is a NUMA-aware version of
> kmalloc() but the implementation is not aware of any possible overflows in
> eventual size calculations.
> 
> Introduce a combination of the two above cases to have a NUMA-node aware
> version of kmalloc_array() and kcalloc().

Yes, this is helpful. I am just wondering why we cannot have
kmalloc_array to call kmalloc_array_node with the local node as a
parameter. Maybe some sort of an optimization?

> Signed-off-by: Johannes Thumshirn 

Anyway
Acked-by: Michal Hocko 

> ---
>  include/linux/slab.h | 16 
>  1 file changed, 16 insertions(+)
> 
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index 41473df6dfb0..aaf4723e41b3 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -635,6 +635,22 @@ extern void *__kmalloc_track_caller(size_t, gfp_t, 
> unsigned long);
>  #define kmalloc_track_caller(size, flags) \
>   __kmalloc_track_caller(size, flags, _RET_IP_)
>  
> +static inline void *kmalloc_array_node(size_t n, size_t size, gfp_t flags,
> +int node)
> +{
> + if (size != 0 && n > SIZE_MAX / size)
> + return NULL;
> + if (__builtin_constant_p(n) && __builtin_constant_p(size))
> + return kmalloc_node(n * size, flags, node);
> + return __kmalloc_node(n * size, flags, node);
> +}
> +
> +static inline void *kcalloc_node(size_t n, size_t size, gfp_t flags, int 
> node)
> +{
> + return kmalloc_array_node(n, size, flags | __GFP_ZERO, node);
> +}
> +
> +
>  #ifdef CONFIG_NUMA
>  extern void *__kmalloc_node_track_caller(size_t, gfp_t, int, unsigned long);
>  #define kmalloc_node_track_caller(size, flags, node) \
> -- 
> 2.13.5
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majord...@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: mailto:"d...@kvack.org;> em...@kvack.org 

-- 
Michal Hocko
SUSE Labs

Re: [PATCH 1/6] mm: add kmalloc_array_node and kcalloc_node

[Michal Hocko]

On Wed 27-09-17 10:20:33, Johannes Thumshirn wrote:
> We have kmalloc_array() and kcalloc() wrappers on top of kmalloc() which
> ensure us overflow free multiplication for the size of a memory
> allocation but these implementations are not NUMA-aware.
> 
> Likewise we have kmalloc_node() which is a NUMA-aware version of
> kmalloc() but the implementation is not aware of any possible overflows in
> eventual size calculations.
> 
> Introduce a combination of the two above cases to have a NUMA-node aware
> version of kmalloc_array() and kcalloc().

Yes, this is helpful. I am just wondering why we cannot have
kmalloc_array to call kmalloc_array_node with the local node as a
parameter. Maybe some sort of an optimization?

> Signed-off-by: Johannes Thumshirn 

Anyway
Acked-by: Michal Hocko 

> ---
>  include/linux/slab.h | 16 
>  1 file changed, 16 insertions(+)
> 
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index 41473df6dfb0..aaf4723e41b3 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -635,6 +635,22 @@ extern void *__kmalloc_track_caller(size_t, gfp_t, 
> unsigned long);
>  #define kmalloc_track_caller(size, flags) \
>   __kmalloc_track_caller(size, flags, _RET_IP_)
>  
> +static inline void *kmalloc_array_node(size_t n, size_t size, gfp_t flags,
> +int node)
> +{
> + if (size != 0 && n > SIZE_MAX / size)
> + return NULL;
> + if (__builtin_constant_p(n) && __builtin_constant_p(size))
> + return kmalloc_node(n * size, flags, node);
> + return __kmalloc_node(n * size, flags, node);
> +}
> +
> +static inline void *kcalloc_node(size_t n, size_t size, gfp_t flags, int 
> node)
> +{
> + return kmalloc_array_node(n, size, flags | __GFP_ZERO, node);
> +}
> +
> +
>  #ifdef CONFIG_NUMA
>  extern void *__kmalloc_node_track_caller(size_t, gfp_t, int, unsigned long);
>  #define kmalloc_node_track_caller(size, flags, node) \
> -- 
> 2.13.5
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majord...@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: mailto:"d...@kvack.org;> em...@kvack.org 

-- 
Michal Hocko
SUSE Labs

[PATCH 1/6] mm: add kmalloc_array_node and kcalloc_node

[Johannes Thumshirn]

We have kmalloc_array() and kcalloc() wrappers on top of kmalloc() which
ensure us overflow free multiplication for the size of a memory
allocation but these implementations are not NUMA-aware.

Likewise we have kmalloc_node() which is a NUMA-aware version of
kmalloc() but the implementation is not aware of any possible overflows in
eventual size calculations.

Introduce a combination of the two above cases to have a NUMA-node aware
version of kmalloc_array() and kcalloc().

Signed-off-by: Johannes Thumshirn 
---
 include/linux/slab.h | 16 
 1 file changed, 16 insertions(+)

diff --git a/include/linux/slab.h b/include/linux/slab.h
index 41473df6dfb0..aaf4723e41b3 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -635,6 +635,22 @@ extern void *__kmalloc_track_caller(size_t, gfp_t, 
unsigned long);
 #define kmalloc_track_caller(size, flags) \
__kmalloc_track_caller(size, flags, _RET_IP_)
 
+static inline void *kmalloc_array_node(size_t n, size_t size, gfp_t flags,
+  int node)
+{
+   if (size != 0 && n > SIZE_MAX / size)
+   return NULL;
+   if (__builtin_constant_p(n) && __builtin_constant_p(size))
+   return kmalloc_node(n * size, flags, node);
+   return __kmalloc_node(n * size, flags, node);
+}
+
+static inline void *kcalloc_node(size_t n, size_t size, gfp_t flags, int node)
+{
+   return kmalloc_array_node(n, size, flags | __GFP_ZERO, node);
+}
+
+
 #ifdef CONFIG_NUMA
 extern void *__kmalloc_node_track_caller(size_t, gfp_t, int, unsigned long);
 #define kmalloc_node_track_caller(size, flags, node) \
-- 
2.13.5

[PATCH 1/6] mm: add kmalloc_array_node and kcalloc_node

[Johannes Thumshirn]

We have kmalloc_array() and kcalloc() wrappers on top of kmalloc() which
ensure us overflow free multiplication for the size of a memory
allocation but these implementations are not NUMA-aware.

Likewise we have kmalloc_node() which is a NUMA-aware version of
kmalloc() but the implementation is not aware of any possible overflows in
eventual size calculations.

Introduce a combination of the two above cases to have a NUMA-node aware
version of kmalloc_array() and kcalloc().

Signed-off-by: Johannes Thumshirn 
---
 include/linux/slab.h | 16 
 1 file changed, 16 insertions(+)

diff --git a/include/linux/slab.h b/include/linux/slab.h
index 41473df6dfb0..aaf4723e41b3 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -635,6 +635,22 @@ extern void *__kmalloc_track_caller(size_t, gfp_t, 
unsigned long);
 #define kmalloc_track_caller(size, flags) \
__kmalloc_track_caller(size, flags, _RET_IP_)
 
+static inline void *kmalloc_array_node(size_t n, size_t size, gfp_t flags,
+  int node)
+{
+   if (size != 0 && n > SIZE_MAX / size)
+   return NULL;
+   if (__builtin_constant_p(n) && __builtin_constant_p(size))
+   return kmalloc_node(n * size, flags, node);
+   return __kmalloc_node(n * size, flags, node);
+}
+
+static inline void *kcalloc_node(size_t n, size_t size, gfp_t flags, int node)
+{
+   return kmalloc_array_node(n, size, flags | __GFP_ZERO, node);
+}
+
+
 #ifdef CONFIG_NUMA
 extern void *__kmalloc_node_track_caller(size_t, gfp_t, int, unsigned long);
 #define kmalloc_node_track_caller(size, flags, node) \
-- 
2.13.5