All of lore.kernel.org
 help / color / mirror / Atom feed
* Slab infrastructure for bulk object allocation and freeing V2
@ 2015-03-30 14:31 ` Christoph Lameter
  0 siblings, 0 replies; 12+ messages in thread
From: Christoph Lameter @ 2015-03-30 14:31 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: linux-kernel, linux-mm, akpm, Pekka Enberg, iamjoonsoo, brouer

After all of the earlier discussions I thought it would be better to
first get agreement on the basic way to allow implementation of the
bulk alloc in the common slab code. So this is a revision of the initial
proposal and it just covers the first patch.



This patch adds the basic infrastructure for alloc / free operations
on pointer arrays. It includes a generic function in the common
slab code that is used in this infrastructure patch to
create the unoptimized functionality for slab bulk operations.

Allocators can then provide optimized allocation functions
for situations in which large numbers of objects are needed.
These optimization may avoid taking locks repeatedly and
bypass metadata creation if all objects in slab pages
can be used to provide the objects required.

Signed-off-by: Christoph Lameter <cl@linux.com>

Index: linux/include/linux/slab.h
===================================================================
--- linux.orig/include/linux/slab.h	2015-03-30 08:48:12.923927793 -0500
+++ linux/include/linux/slab.h	2015-03-30 08:48:12.923927793 -0500
@@ -289,6 +289,8 @@ static __always_inline int kmalloc_index
 void *__kmalloc(size_t size, gfp_t flags);
 void *kmem_cache_alloc(struct kmem_cache *, gfp_t flags);
 void kmem_cache_free(struct kmem_cache *, void *);
+void kmem_cache_free_array(struct kmem_cache *, size_t, void **);
+int kmem_cache_alloc_array(struct kmem_cache *, gfp_t, size_t, void **);

 #ifdef CONFIG_NUMA
 void *__kmalloc_node(size_t size, gfp_t flags, int node);
Index: linux/mm/slab_common.c
===================================================================
--- linux.orig/mm/slab_common.c	2015-03-30 08:48:12.923927793 -0500
+++ linux/mm/slab_common.c	2015-03-30 08:57:41.737572817 -0500
@@ -105,6 +105,29 @@ static inline int kmem_cache_sanity_chec
 }
 #endif

+int __kmem_cache_alloc_array(struct kmem_cache *s, gfp_t flags, size_t nr,
+								void **p)
+{
+	size_t i;
+
+	for (i = 0; i < nr; i++) {
+		void *x = p[i] = kmem_cache_alloc(s, flags);
+		if (!x)
+			return i;
+	}
+	return nr;
+}
+
+void __kmem_cache_free_array(struct kmem_cache *s, size_t nr, void **p)
+{
+	size_t i;
+
+	for (i = 0; i < nr; i++)
+		kmem_cache_free(s, p[i]);
+}
+
+#endif
+
 #ifdef CONFIG_MEMCG_KMEM
 void slab_init_memcg_params(struct kmem_cache *s)
 {
Index: linux/mm/slab.h
===================================================================
--- linux.orig/mm/slab.h	2015-03-30 08:48:12.923927793 -0500
+++ linux/mm/slab.h	2015-03-30 08:48:12.923927793 -0500
@@ -162,6 +162,10 @@ void slabinfo_show_stats(struct seq_file
 ssize_t slabinfo_write(struct file *file, const char __user *buffer,
 		       size_t count, loff_t *ppos);

+/* Generic implementations of array operations */
+void __kmem_cache_free_array(struct kmem_cache *, size_t, void **);
+int __kmem_cache_alloc_array(struct kmem_cache *, gfp_t, size_t, void **);
+
 #ifdef CONFIG_MEMCG_KMEM
 /*
  * Iterate over all memcg caches of the given root cache. The caller must hold
Index: linux/mm/slab.c
===================================================================
--- linux.orig/mm/slab.c	2015-03-30 08:48:12.923927793 -0500
+++ linux/mm/slab.c	2015-03-30 08:49:08.398137844 -0500
@@ -3401,6 +3401,17 @@ void *kmem_cache_alloc(struct kmem_cache
 }
 EXPORT_SYMBOL(kmem_cache_alloc);

+void kmem_cache_free_array(struct kmem_cache *s, size_t size, void **p) {
+	__kmem_cache_free_array(s, size, p);
+}
+EXPORT_SYMBOL(kmem_cache_free_array);
+
+int kmem_cache_alloc_array(struct kmem_cache *s, gfp_t flags, size_t size,
+								void **p) {
+	return kmem_cache_alloc_array(s, flags, size, p);
+}
+EXPORT_SYMBOL(kmem_cache_alloc_array);
+
 #ifdef CONFIG_TRACING
 void *
 kmem_cache_alloc_trace(struct kmem_cache *cachep, gfp_t flags, size_t size)
Index: linux/mm/slub.c
===================================================================
--- linux.orig/mm/slub.c	2015-03-30 08:48:12.923927793 -0500
+++ linux/mm/slub.c	2015-03-30 08:48:12.923927793 -0500
@@ -2752,6 +2752,18 @@ void kmem_cache_free(struct kmem_cache *
 }
 EXPORT_SYMBOL(kmem_cache_free);

+void kmem_cache_free_array(struct kmem_cache *s, size_t size, void **p) {
+	__kmem_cache_free_array(s, size, p);
+}
+EXPORT_SYMBOL(kmem_cache_free_array);
+
+int kmem_cache_alloc_array(struct kmem_cache *s, gfp_t flags, size_t size,
+								void **p) {
+	return kmem_cache_alloc_array(s, flags, size, p);
+}
+EXPORT_SYMBOL(kmem_cache_alloc_array);
+
+
 /*
  * Object placement in a slab is made very easy because we always start at
  * offset 0. If we tune the size of the object to the alignment then we can
Index: linux/mm/slob.c
===================================================================
--- linux.orig/mm/slob.c	2015-03-30 08:48:12.923927793 -0500
+++ linux/mm/slob.c	2015-03-30 08:50:16.995924460 -0500
@@ -612,6 +612,17 @@ void kmem_cache_free(struct kmem_cache *
 }
 EXPORT_SYMBOL(kmem_cache_free);

+void kmem_cache_free_array(struct kmem_cache *s, size_t size, void **p) {
+	__kmem_cache_free_array(s, size, p);
+}
+EXPORT_SYMBOL(kmem_cache_free_array);
+
+int kmem_cache_alloc_array(struct kmem_cache *s, gfp_t flags, size_t size,
+								void **p) {
+	return kmem_cache_alloc_array(s, flags, size, p);
+}
+EXPORT_SYMBOL(kmem_cache_alloc_array);
+
 int __kmem_cache_shutdown(struct kmem_cache *c)
 {
 	/* No way to check for remaining objects */

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

* Slab infrastructure for bulk object allocation and freeing V2
@ 2015-03-30 14:31 ` Christoph Lameter
  0 siblings, 0 replies; 12+ messages in thread
From: Christoph Lameter @ 2015-03-30 14:31 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: linux-kernel, linux-mm, akpm, Pekka Enberg, iamjoonsoo

After all of the earlier discussions I thought it would be better to
first get agreement on the basic way to allow implementation of the
bulk alloc in the common slab code. So this is a revision of the initial
proposal and it just covers the first patch.



This patch adds the basic infrastructure for alloc / free operations
on pointer arrays. It includes a generic function in the common
slab code that is used in this infrastructure patch to
create the unoptimized functionality for slab bulk operations.

Allocators can then provide optimized allocation functions
for situations in which large numbers of objects are needed.
These optimization may avoid taking locks repeatedly and
bypass metadata creation if all objects in slab pages
can be used to provide the objects required.

Signed-off-by: Christoph Lameter <cl@linux.com>

Index: linux/include/linux/slab.h
===================================================================
--- linux.orig/include/linux/slab.h	2015-03-30 08:48:12.923927793 -0500
+++ linux/include/linux/slab.h	2015-03-30 08:48:12.923927793 -0500
@@ -289,6 +289,8 @@ static __always_inline int kmalloc_index
 void *__kmalloc(size_t size, gfp_t flags);
 void *kmem_cache_alloc(struct kmem_cache *, gfp_t flags);
 void kmem_cache_free(struct kmem_cache *, void *);
+void kmem_cache_free_array(struct kmem_cache *, size_t, void **);
+int kmem_cache_alloc_array(struct kmem_cache *, gfp_t, size_t, void **);

 #ifdef CONFIG_NUMA
 void *__kmalloc_node(size_t size, gfp_t flags, int node);
Index: linux/mm/slab_common.c
===================================================================
--- linux.orig/mm/slab_common.c	2015-03-30 08:48:12.923927793 -0500
+++ linux/mm/slab_common.c	2015-03-30 08:57:41.737572817 -0500
@@ -105,6 +105,29 @@ static inline int kmem_cache_sanity_chec
 }
 #endif

+int __kmem_cache_alloc_array(struct kmem_cache *s, gfp_t flags, size_t nr,
+								void **p)
+{
+	size_t i;
+
+	for (i = 0; i < nr; i++) {
+		void *x = p[i] = kmem_cache_alloc(s, flags);
+		if (!x)
+			return i;
+	}
+	return nr;
+}
+
+void __kmem_cache_free_array(struct kmem_cache *s, size_t nr, void **p)
+{
+	size_t i;
+
+	for (i = 0; i < nr; i++)
+		kmem_cache_free(s, p[i]);
+}
+
+#endif
+
 #ifdef CONFIG_MEMCG_KMEM
 void slab_init_memcg_params(struct kmem_cache *s)
 {
Index: linux/mm/slab.h
===================================================================
--- linux.orig/mm/slab.h	2015-03-30 08:48:12.923927793 -0500
+++ linux/mm/slab.h	2015-03-30 08:48:12.923927793 -0500
@@ -162,6 +162,10 @@ void slabinfo_show_stats(struct seq_file
 ssize_t slabinfo_write(struct file *file, const char __user *buffer,
 		       size_t count, loff_t *ppos);

+/* Generic implementations of array operations */
+void __kmem_cache_free_array(struct kmem_cache *, size_t, void **);
+int __kmem_cache_alloc_array(struct kmem_cache *, gfp_t, size_t, void **);
+
 #ifdef CONFIG_MEMCG_KMEM
 /*
  * Iterate over all memcg caches of the given root cache. The caller must hold
Index: linux/mm/slab.c
===================================================================
--- linux.orig/mm/slab.c	2015-03-30 08:48:12.923927793 -0500
+++ linux/mm/slab.c	2015-03-30 08:49:08.398137844 -0500
@@ -3401,6 +3401,17 @@ void *kmem_cache_alloc(struct kmem_cache
 }
 EXPORT_SYMBOL(kmem_cache_alloc);

+void kmem_cache_free_array(struct kmem_cache *s, size_t size, void **p) {
+	__kmem_cache_free_array(s, size, p);
+}
+EXPORT_SYMBOL(kmem_cache_free_array);
+
+int kmem_cache_alloc_array(struct kmem_cache *s, gfp_t flags, size_t size,
+								void **p) {
+	return kmem_cache_alloc_array(s, flags, size, p);
+}
+EXPORT_SYMBOL(kmem_cache_alloc_array);
+
 #ifdef CONFIG_TRACING
 void *
 kmem_cache_alloc_trace(struct kmem_cache *cachep, gfp_t flags, size_t size)
Index: linux/mm/slub.c
===================================================================
--- linux.orig/mm/slub.c	2015-03-30 08:48:12.923927793 -0500
+++ linux/mm/slub.c	2015-03-30 08:48:12.923927793 -0500
@@ -2752,6 +2752,18 @@ void kmem_cache_free(struct kmem_cache *
 }
 EXPORT_SYMBOL(kmem_cache_free);

+void kmem_cache_free_array(struct kmem_cache *s, size_t size, void **p) {
+	__kmem_cache_free_array(s, size, p);
+}
+EXPORT_SYMBOL(kmem_cache_free_array);
+
+int kmem_cache_alloc_array(struct kmem_cache *s, gfp_t flags, size_t size,
+								void **p) {
+	return kmem_cache_alloc_array(s, flags, size, p);
+}
+EXPORT_SYMBOL(kmem_cache_alloc_array);
+
+
 /*
  * Object placement in a slab is made very easy because we always start at
  * offset 0. If we tune the size of the object to the alignment then we can
Index: linux/mm/slob.c
===================================================================
--- linux.orig/mm/slob.c	2015-03-30 08:48:12.923927793 -0500
+++ linux/mm/slob.c	2015-03-30 08:50:16.995924460 -0500
@@ -612,6 +612,17 @@ void kmem_cache_free(struct kmem_cache *
 }
 EXPORT_SYMBOL(kmem_cache_free);

+void kmem_cache_free_array(struct kmem_cache *s, size_t size, void **p) {
+	__kmem_cache_free_array(s, size, p);
+}
+EXPORT_SYMBOL(kmem_cache_free_array);
+
+int kmem_cache_alloc_array(struct kmem_cache *s, gfp_t flags, size_t size,
+								void **p) {
+	return kmem_cache_alloc_array(s, flags, size, p);
+}
+EXPORT_SYMBOL(kmem_cache_alloc_array);
+
 int __kmem_cache_shutdown(struct kmem_cache *c)
 {
 	/* No way to check for remaining objects */

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

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

* Re: Slab infrastructure for bulk object allocation and freeing V2
  2015-03-30 14:31 ` Christoph Lameter
@ 2015-03-31  0:17   ` Jesper Dangaard Brouer
  -1 siblings, 0 replies; 12+ messages in thread
From: Jesper Dangaard Brouer @ 2015-03-31  0:17 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: linux-kernel, linux-mm, akpm, Pekka Enberg, iamjoonsoo, brouer

On Mon, 30 Mar 2015 09:31:19 -0500 (CDT)
Christoph Lameter <cl@linux.com> wrote:

> After all of the earlier discussions I thought it would be better to
> first get agreement on the basic way to allow implementation of the
> bulk alloc in the common slab code. So this is a revision of the initial
> proposal and it just covers the first patch.

I agree, it would be good to get the basic API in.
 
> This patch adds the basic infrastructure for alloc / free operations
> on pointer arrays. It includes a generic function in the common
> slab code that is used in this infrastructure patch to
> create the unoptimized functionality for slab bulk operations.
> 
> Allocators can then provide optimized allocation functions
> for situations in which large numbers of objects are needed.
> These optimization may avoid taking locks repeatedly and
> bypass metadata creation if all objects in slab pages
> can be used to provide the objects required.

I'll volunteer to performance benchmark the different allocators
optimized functions in this area. (I'll have time after April 13th).

 
> Signed-off-by: Christoph Lameter <cl@linux.com>

Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
 
-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Sr. Network Kernel Developer at Red Hat
  Author of http://www.iptv-analyzer.org
  LinkedIn: http://www.linkedin.com/in/brouer

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

* Re: Slab infrastructure for bulk object allocation and freeing V2
@ 2015-03-31  0:17   ` Jesper Dangaard Brouer
  0 siblings, 0 replies; 12+ messages in thread
From: Jesper Dangaard Brouer @ 2015-03-31  0:17 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: linux-kernel, linux-mm, akpm, Pekka Enberg, iamjoonsoo, brouer

On Mon, 30 Mar 2015 09:31:19 -0500 (CDT)
Christoph Lameter <cl@linux.com> wrote:

> After all of the earlier discussions I thought it would be better to
> first get agreement on the basic way to allow implementation of the
> bulk alloc in the common slab code. So this is a revision of the initial
> proposal and it just covers the first patch.

I agree, it would be good to get the basic API in.
 
> This patch adds the basic infrastructure for alloc / free operations
> on pointer arrays. It includes a generic function in the common
> slab code that is used in this infrastructure patch to
> create the unoptimized functionality for slab bulk operations.
> 
> Allocators can then provide optimized allocation functions
> for situations in which large numbers of objects are needed.
> These optimization may avoid taking locks repeatedly and
> bypass metadata creation if all objects in slab pages
> can be used to provide the objects required.

I'll volunteer to performance benchmark the different allocators
optimized functions in this area. (I'll have time after April 13th).

 
> Signed-off-by: Christoph Lameter <cl@linux.com>

Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
 
-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Sr. Network Kernel Developer at Red Hat
  Author of http://www.iptv-analyzer.org
  LinkedIn: http://www.linkedin.com/in/brouer

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

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

* Re: Slab infrastructure for bulk object allocation and freeing V2
  2015-03-30 14:31 ` Christoph Lameter
@ 2015-03-31 21:20   ` Andrew Morton
  -1 siblings, 0 replies; 12+ messages in thread
From: Andrew Morton @ 2015-03-31 21:20 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: Jesper Dangaard Brouer, linux-kernel, linux-mm, akpm,
	Pekka Enberg, iamjoonsoo

On Mon, 30 Mar 2015 09:31:19 -0500 (CDT) Christoph Lameter <cl@linux.com> wrote:

> After all of the earlier discussions I thought it would be better to
> first get agreement on the basic way to allow implementation of the
> bulk alloc in the common slab code. So this is a revision of the initial
> proposal and it just covers the first patch.
> 
> 
> 
> This patch adds the basic infrastructure for alloc / free operations
> on pointer arrays. It includes a generic function in the common
> slab code that is used in this infrastructure patch to
> create the unoptimized functionality for slab bulk operations.
> 
> Allocators can then provide optimized allocation functions
> for situations in which large numbers of objects are needed.
> These optimization may avoid taking locks repeatedly and
> bypass metadata creation if all objects in slab pages
> can be used to provide the objects required.

This patch doesn't really do anything.  I guess nailing down the
interface helps a bit.


> @@ -289,6 +289,8 @@ static __always_inline int kmalloc_index
>  void *__kmalloc(size_t size, gfp_t flags);
>  void *kmem_cache_alloc(struct kmem_cache *, gfp_t flags);
>  void kmem_cache_free(struct kmem_cache *, void *);
> +void kmem_cache_free_array(struct kmem_cache *, size_t, void **);
> +int kmem_cache_alloc_array(struct kmem_cache *, gfp_t, size_t, void **);
> 
>  #ifdef CONFIG_NUMA
>  void *__kmalloc_node(size_t size, gfp_t flags, int node);
> Index: linux/mm/slab_common.c
> ===================================================================
> --- linux.orig/mm/slab_common.c	2015-03-30 08:48:12.923927793 -0500
> +++ linux/mm/slab_common.c	2015-03-30 08:57:41.737572817 -0500
> @@ -105,6 +105,29 @@ static inline int kmem_cache_sanity_chec
>  }
>  #endif
> 
> +int __kmem_cache_alloc_array(struct kmem_cache *s, gfp_t flags, size_t nr,
> +								void **p)
> +{
> +	size_t i;
> +
> +	for (i = 0; i < nr; i++) {
> +		void *x = p[i] = kmem_cache_alloc(s, flags);
> +		if (!x)
> +			return i;
> +	}
> +	return nr;
> +}

Some documentation would be nice.  It's a major new interface, exported
to modules.  And it isn't completely obvious, because the return
semantics are weird.

What's the reason for returning a partial result when ENOMEM?  Some
callers will throw away the partial result and simply fail out.  If a
caller attempts to go ahead and use the partial result then great, but
you can bet that nobody will actually runtime test this situation, so
the interface is an invitation for us to release partially-tested code
into the wild.


Instead of the above, did you consider doing

int __weak kmem_cache_alloc_array(struct kmem_cache *s, gfp_t flags, size_t nr,

?

This way we save a level of function call and all that wrapper code in
the allocators simply disappears.

> --- linux.orig/mm/slab.c	2015-03-30 08:48:12.923927793 -0500
> +++ linux/mm/slab.c	2015-03-30 08:49:08.398137844 -0500
> @@ -3401,6 +3401,17 @@ void *kmem_cache_alloc(struct kmem_cache
>  }
>  EXPORT_SYMBOL(kmem_cache_alloc);
> 
> +void kmem_cache_free_array(struct kmem_cache *s, size_t size, void **p) {
> +	__kmem_cache_free_array(s, size, p);
> +}

Coding style is weird.



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

* Re: Slab infrastructure for bulk object allocation and freeing V2
@ 2015-03-31 21:20   ` Andrew Morton
  0 siblings, 0 replies; 12+ messages in thread
From: Andrew Morton @ 2015-03-31 21:20 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: Jesper Dangaard Brouer, linux-kernel, linux-mm, akpm,
	Pekka Enberg, iamjoonsoo

On Mon, 30 Mar 2015 09:31:19 -0500 (CDT) Christoph Lameter <cl@linux.com> wrote:

> After all of the earlier discussions I thought it would be better to
> first get agreement on the basic way to allow implementation of the
> bulk alloc in the common slab code. So this is a revision of the initial
> proposal and it just covers the first patch.
> 
> 
> 
> This patch adds the basic infrastructure for alloc / free operations
> on pointer arrays. It includes a generic function in the common
> slab code that is used in this infrastructure patch to
> create the unoptimized functionality for slab bulk operations.
> 
> Allocators can then provide optimized allocation functions
> for situations in which large numbers of objects are needed.
> These optimization may avoid taking locks repeatedly and
> bypass metadata creation if all objects in slab pages
> can be used to provide the objects required.

This patch doesn't really do anything.  I guess nailing down the
interface helps a bit.


> @@ -289,6 +289,8 @@ static __always_inline int kmalloc_index
>  void *__kmalloc(size_t size, gfp_t flags);
>  void *kmem_cache_alloc(struct kmem_cache *, gfp_t flags);
>  void kmem_cache_free(struct kmem_cache *, void *);
> +void kmem_cache_free_array(struct kmem_cache *, size_t, void **);
> +int kmem_cache_alloc_array(struct kmem_cache *, gfp_t, size_t, void **);
> 
>  #ifdef CONFIG_NUMA
>  void *__kmalloc_node(size_t size, gfp_t flags, int node);
> Index: linux/mm/slab_common.c
> ===================================================================
> --- linux.orig/mm/slab_common.c	2015-03-30 08:48:12.923927793 -0500
> +++ linux/mm/slab_common.c	2015-03-30 08:57:41.737572817 -0500
> @@ -105,6 +105,29 @@ static inline int kmem_cache_sanity_chec
>  }
>  #endif
> 
> +int __kmem_cache_alloc_array(struct kmem_cache *s, gfp_t flags, size_t nr,
> +								void **p)
> +{
> +	size_t i;
> +
> +	for (i = 0; i < nr; i++) {
> +		void *x = p[i] = kmem_cache_alloc(s, flags);
> +		if (!x)
> +			return i;
> +	}
> +	return nr;
> +}

Some documentation would be nice.  It's a major new interface, exported
to modules.  And it isn't completely obvious, because the return
semantics are weird.

What's the reason for returning a partial result when ENOMEM?  Some
callers will throw away the partial result and simply fail out.  If a
caller attempts to go ahead and use the partial result then great, but
you can bet that nobody will actually runtime test this situation, so
the interface is an invitation for us to release partially-tested code
into the wild.


Instead of the above, did you consider doing

int __weak kmem_cache_alloc_array(struct kmem_cache *s, gfp_t flags, size_t nr,

?

This way we save a level of function call and all that wrapper code in
the allocators simply disappears.

> --- linux.orig/mm/slab.c	2015-03-30 08:48:12.923927793 -0500
> +++ linux/mm/slab.c	2015-03-30 08:49:08.398137844 -0500
> @@ -3401,6 +3401,17 @@ void *kmem_cache_alloc(struct kmem_cache
>  }
>  EXPORT_SYMBOL(kmem_cache_alloc);
> 
> +void kmem_cache_free_array(struct kmem_cache *s, size_t size, void **p) {
> +	__kmem_cache_free_array(s, size, p);
> +}

Coding style is weird.


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

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

* Re: Slab infrastructure for bulk object allocation and freeing V2
  2015-03-31 21:20   ` Andrew Morton
@ 2015-04-02 14:25     ` Christoph Lameter
  -1 siblings, 0 replies; 12+ messages in thread
From: Christoph Lameter @ 2015-04-02 14:25 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Jesper Dangaard Brouer, linux-kernel, linux-mm, akpm,
	Pekka Enberg, iamjoonsoo

On Tue, 31 Mar 2015, Andrew Morton wrote:

> This patch doesn't really do anything.  I guess nailing down the
> interface helps a bit.

Right.

> to modules.  And it isn't completely obvious, because the return
> semantics are weird.

Ok.

> What's the reason for returning a partial result when ENOMEM?  Some
> callers will throw away the partial result and simply fail out.  If a
> caller attempts to go ahead and use the partial result then great, but
> you can bet that nobody will actually runtime test this situation, so
> the interface is an invitation for us to release partially-tested code
> into the wild.

Just rely on the fact that small allocations never fail? The caller get
all the requested objects if the function returns?

> Instead of the above, did you consider doing
>
> int __weak kmem_cache_alloc_array(struct kmem_cache *s, gfp_t flags, size_t nr,
>
> ?
>
> This way we save a level of function call and all that wrapper code in
> the allocators simply disappears.

I think we will need the auxiliary function in the common code later
because that allows the allocations to only do the allocations that
can be optimized and for the rest just fall back to the generic
implementations. There may be situations in which the optimizations wont
work. For SLUB this may be the case f.e. if debug options are enabled.

> > --- linux.orig/mm/slab.c	2015-03-30 08:48:12.923927793 -0500
> > +++ linux/mm/slab.c	2015-03-30 08:49:08.398137844 -0500
> > @@ -3401,6 +3401,17 @@ void *kmem_cache_alloc(struct kmem_cache
> >  }
> >  EXPORT_SYMBOL(kmem_cache_alloc);
> >
> > +void kmem_cache_free_array(struct kmem_cache *s, size_t size, void **p) {
> > +	__kmem_cache_free_array(s, size, p);
> > +}
>
> Coding style is weird.

Ok. Will fix.


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

* Re: Slab infrastructure for bulk object allocation and freeing V2
@ 2015-04-02 14:25     ` Christoph Lameter
  0 siblings, 0 replies; 12+ messages in thread
From: Christoph Lameter @ 2015-04-02 14:25 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Jesper Dangaard Brouer, linux-kernel, linux-mm, akpm,
	Pekka Enberg, iamjoonsoo

On Tue, 31 Mar 2015, Andrew Morton wrote:

> This patch doesn't really do anything.  I guess nailing down the
> interface helps a bit.

Right.

> to modules.  And it isn't completely obvious, because the return
> semantics are weird.

Ok.

> What's the reason for returning a partial result when ENOMEM?  Some
> callers will throw away the partial result and simply fail out.  If a
> caller attempts to go ahead and use the partial result then great, but
> you can bet that nobody will actually runtime test this situation, so
> the interface is an invitation for us to release partially-tested code
> into the wild.

Just rely on the fact that small allocations never fail? The caller get
all the requested objects if the function returns?

> Instead of the above, did you consider doing
>
> int __weak kmem_cache_alloc_array(struct kmem_cache *s, gfp_t flags, size_t nr,
>
> ?
>
> This way we save a level of function call and all that wrapper code in
> the allocators simply disappears.

I think we will need the auxiliary function in the common code later
because that allows the allocations to only do the allocations that
can be optimized and for the rest just fall back to the generic
implementations. There may be situations in which the optimizations wont
work. For SLUB this may be the case f.e. if debug options are enabled.

> > --- linux.orig/mm/slab.c	2015-03-30 08:48:12.923927793 -0500
> > +++ linux/mm/slab.c	2015-03-30 08:49:08.398137844 -0500
> > @@ -3401,6 +3401,17 @@ void *kmem_cache_alloc(struct kmem_cache
> >  }
> >  EXPORT_SYMBOL(kmem_cache_alloc);
> >
> > +void kmem_cache_free_array(struct kmem_cache *s, size_t size, void **p) {
> > +	__kmem_cache_free_array(s, size, p);
> > +}
>
> Coding style is weird.

Ok. Will fix.

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

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

* Re: Slab infrastructure for bulk object allocation and freeing V2
  2015-04-02 14:25     ` Christoph Lameter
@ 2015-04-02 20:42       ` Andrew Morton
  -1 siblings, 0 replies; 12+ messages in thread
From: Andrew Morton @ 2015-04-02 20:42 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: Jesper Dangaard Brouer, linux-kernel, linux-mm, akpm,
	Pekka Enberg, iamjoonsoo

On Thu, 2 Apr 2015 09:25:37 -0500 (CDT) Christoph Lameter <cl@linux.com> wrote:

> > What's the reason for returning a partial result when ENOMEM?  Some
> > callers will throw away the partial result and simply fail out.  If a
> > caller attempts to go ahead and use the partial result then great, but
> > you can bet that nobody will actually runtime test this situation, so
> > the interface is an invitation for us to release partially-tested code
> > into the wild.
> 
> Just rely on the fact that small allocations never fail? The caller get
> all the requested objects if the function returns?

I'd suggest the latter: either the callee successfully allocates all
the requested objects or it fails.

> > Instead of the above, did you consider doing
> >
> > int __weak kmem_cache_alloc_array(struct kmem_cache *s, gfp_t flags, size_t nr,
> >
> > ?
> >
> > This way we save a level of function call and all that wrapper code in
> > the allocators simply disappears.
> 
> I think we will need the auxiliary function in the common code later
> because that allows the allocations to only do the allocations that
> can be optimized and for the rest just fall back to the generic
> implementations. There may be situations in which the optimizations wont
> work. For SLUB this may be the case f.e. if debug options are enabled.

hm, OK.  The per-allocator wrappers could be made static inline in .h
if that makes sense.


With the current code, gcc should be able to convert the call into a
tailcall.

<checks>

nope.

kmem_cache_free_array:
	pushq	%rbp	#
	movq	%rsp, %rbp	#,
	call	__kmem_cache_free_array	#
	leave
	ret

stupid gcc.

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

* Re: Slab infrastructure for bulk object allocation and freeing V2
@ 2015-04-02 20:42       ` Andrew Morton
  0 siblings, 0 replies; 12+ messages in thread
From: Andrew Morton @ 2015-04-02 20:42 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: Jesper Dangaard Brouer, linux-kernel, linux-mm, akpm,
	Pekka Enberg, iamjoonsoo

On Thu, 2 Apr 2015 09:25:37 -0500 (CDT) Christoph Lameter <cl@linux.com> wrote:

> > What's the reason for returning a partial result when ENOMEM?  Some
> > callers will throw away the partial result and simply fail out.  If a
> > caller attempts to go ahead and use the partial result then great, but
> > you can bet that nobody will actually runtime test this situation, so
> > the interface is an invitation for us to release partially-tested code
> > into the wild.
> 
> Just rely on the fact that small allocations never fail? The caller get
> all the requested objects if the function returns?

I'd suggest the latter: either the callee successfully allocates all
the requested objects or it fails.

> > Instead of the above, did you consider doing
> >
> > int __weak kmem_cache_alloc_array(struct kmem_cache *s, gfp_t flags, size_t nr,
> >
> > ?
> >
> > This way we save a level of function call and all that wrapper code in
> > the allocators simply disappears.
> 
> I think we will need the auxiliary function in the common code later
> because that allows the allocations to only do the allocations that
> can be optimized and for the rest just fall back to the generic
> implementations. There may be situations in which the optimizations wont
> work. For SLUB this may be the case f.e. if debug options are enabled.

hm, OK.  The per-allocator wrappers could be made static inline in .h
if that makes sense.


With the current code, gcc should be able to convert the call into a
tailcall.

<checks>

nope.

kmem_cache_free_array:
	pushq	%rbp	#
	movq	%rsp, %rbp	#,
	call	__kmem_cache_free_array	#
	leave
	ret

stupid gcc.

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

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

* Re: Slab infrastructure for bulk object allocation and freeing V2
  2015-04-02 20:42       ` Andrew Morton
@ 2015-04-06 18:27         ` Christoph Lameter
  -1 siblings, 0 replies; 12+ messages in thread
From: Christoph Lameter @ 2015-04-06 18:27 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Jesper Dangaard Brouer, linux-kernel, linux-mm, akpm,
	Pekka Enberg, iamjoonsoo

On Thu, 2 Apr 2015, Andrew Morton wrote:

> hm, OK.  The per-allocator wrappers could be made static inline in .h
> if that makes sense.

The allocators will add code to the "per-allocator wrappers". Inlining
that would be bad. Basicalkly the "wrapper" is the skeleon to which
optimizations can be added while keeping the call to the generic
implementaiton if the allocator has to punt for some reason.


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

* Re: Slab infrastructure for bulk object allocation and freeing V2
@ 2015-04-06 18:27         ` Christoph Lameter
  0 siblings, 0 replies; 12+ messages in thread
From: Christoph Lameter @ 2015-04-06 18:27 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Jesper Dangaard Brouer, linux-kernel, linux-mm, akpm,
	Pekka Enberg, iamjoonsoo

On Thu, 2 Apr 2015, Andrew Morton wrote:

> hm, OK.  The per-allocator wrappers could be made static inline in .h
> if that makes sense.

The allocators will add code to the "per-allocator wrappers". Inlining
that would be bad. Basicalkly the "wrapper" is the skeleon to which
optimizations can be added while keeping the call to the generic
implementaiton if the allocator has to punt for some reason.

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

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

end of thread, other threads:[~2015-04-06 18:27 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-30 14:31 Slab infrastructure for bulk object allocation and freeing V2 Christoph Lameter
2015-03-30 14:31 ` Christoph Lameter
2015-03-31  0:17 ` Jesper Dangaard Brouer
2015-03-31  0:17   ` Jesper Dangaard Brouer
2015-03-31 21:20 ` Andrew Morton
2015-03-31 21:20   ` Andrew Morton
2015-04-02 14:25   ` Christoph Lameter
2015-04-02 14:25     ` Christoph Lameter
2015-04-02 20:42     ` Andrew Morton
2015-04-02 20:42       ` Andrew Morton
2015-04-06 18:27       ` Christoph Lameter
2015-04-06 18:27         ` Christoph Lameter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.