[dpdk-dev,v1,09/29] mem: add iovec-like allocation wrappers

Message ID 18771436831105304b9f7c133cc51e8c867f4273.1507730496.git.adrien.mazarguil@6wind.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail apply patch file failure

Commit Message

Adrien Mazarguil Oct. 11, 2017, 2:35 p.m. UTC
  These wrappers implement the ability to allocate room for several disparate
objects as a single contiguous allocation while complying with their
respective alignment constraints.

This is usually more efficient than allocating and freeing them
individually if they are not expected to be reallocated with rte_realloc().

A typical use case is when several objects that cannot be dissociated must
be allocated together, as shown in the following example:

 struct b {
    ...
    struct d *d;
 }

 struct a {
     ...
     struct b *b;
     struct c *c;
 }

 struct rte_malloc_vec vec[] = {
     { .size = sizeof(struct a), .addr = &ptr_a, },
     { .size = sizeof(struct b), .addr = &ptr_b, },
     { .size = sizeof(struct c), .addr = &ptr_c, },
     { .size = sizeof(struct d), .addr = &ptr_d, },
 };

 if (!rte_mallocv(NULL, vec, RTE_DIM(vec)))
     goto error;

 struct a *a = ptr_a;

 a->b = ptr_b;
 a->c = ptr_c;
 a->b->d = ptr_d;
 ...
 rte_free(a);

Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
---
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  9 ++
 lib/librte_eal/common/include/rte_malloc.h      | 85 ++++++++++++++++++
 lib/librte_eal/common/rte_malloc.c              | 92 ++++++++++++++++++++
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |  9 ++
 4 files changed, 195 insertions(+)
  

Comments

Ferruh Yigit Oct. 11, 2017, 9:58 p.m. UTC | #1
On 10/11/2017 3:35 PM, Adrien Mazarguil wrote:
> These wrappers implement the ability to allocate room for several disparate
> objects as a single contiguous allocation while complying with their
> respective alignment constraints.
> 
> This is usually more efficient than allocating and freeing them
> individually if they are not expected to be reallocated with rte_realloc().
> 
> A typical use case is when several objects that cannot be dissociated must
> be allocated together, as shown in the following example:
> 
>  struct b {
>     ...
>     struct d *d;
>  }
> 
>  struct a {
>      ...
>      struct b *b;
>      struct c *c;
>  }
> 
>  struct rte_malloc_vec vec[] = {
>      { .size = sizeof(struct a), .addr = &ptr_a, },
>      { .size = sizeof(struct b), .addr = &ptr_b, },
>      { .size = sizeof(struct c), .addr = &ptr_c, },
>      { .size = sizeof(struct d), .addr = &ptr_d, },
>  };
> 
>  if (!rte_mallocv(NULL, vec, RTE_DIM(vec)))
>      goto error;
> 
>  struct a *a = ptr_a;
> 
>  a->b = ptr_b;
>  a->c = ptr_c;
>  a->b->d = ptr_d;
>  ...
>  rte_free(a);
> 
> Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>

Hi Adrien,

Why there is an eal patch in the middle of the mlx4 patchset?

I believe this shouldn't go in via next-net tree, and should be reviewed
properly.

I am behaving more flexible for PMD patches about the process and
timing, because their scope is limited.
PMD patches can break at most the PMD itself and if the maintainer is
sending the patch, they should be knowing what they are doing, so vendor
gets the responsibility for their own driver. I am paying majority of
care to be sure it doesn't break others.

But ethdev and eal way beyond those flexibility, because their scope is
much larger.

Can you please extract this patch from the patchset?

Thanks,
ferruh
  
Ferruh Yigit Oct. 11, 2017, 10 p.m. UTC | #2
On 10/11/2017 10:58 PM, Ferruh Yigit wrote:
> On 10/11/2017 3:35 PM, Adrien Mazarguil wrote:
>> These wrappers implement the ability to allocate room for several disparate
>> objects as a single contiguous allocation while complying with their
>> respective alignment constraints.
>>
>> This is usually more efficient than allocating and freeing them
>> individually if they are not expected to be reallocated with rte_realloc().
>>
>> A typical use case is when several objects that cannot be dissociated must
>> be allocated together, as shown in the following example:
>>
>>  struct b {
>>     ...
>>     struct d *d;
>>  }
>>
>>  struct a {
>>      ...
>>      struct b *b;
>>      struct c *c;
>>  }
>>
>>  struct rte_malloc_vec vec[] = {
>>      { .size = sizeof(struct a), .addr = &ptr_a, }
>>      { .size = sizeof(struct b), .addr = &ptr_b, },
>>      { .size = sizeof(struct c), .addr = &ptr_c, },
>>      { .size = sizeof(struct d), .addr = &ptr_d, },
>>  };
>>
>>  if (!rte_mallocv(NULL, vec, RTE_DIM(vec)))
>>      goto error;
>>
>>  struct a *a = ptr_a;
>>
>>  a->b = ptr_b;
>>  a->c = ptr_c;
>>  a->b->d = ptr_d;
>>  ...
>>  rte_free(a);
>>
>> Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
>> Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> 
> Hi Adrien,
> 
> Why there is an eal patch in the middle of the mlx4 patchset?
> 
> I believe this shouldn't go in via next-net tree, and should be reviewed
> properly.
> 
> I am behaving more flexible for PMD patches about the process and
> timing, because their scope is limited.
> PMD patches can break at most the PMD itself and if the maintainer is
> sending the patch, they should be knowing what they are doing, so vendor
> gets the responsibility for their own driver. I am paying majority of
> care to be sure it doesn't break others.
> 
> But ethdev and eal way beyond those flexibility, because their scope is
> much larger.
> 
> Can you please extract this patch from the patchset?

cc'ed Thomas.
  
Adrien Mazarguil Oct. 12, 2017, 11:07 a.m. UTC | #3
On Wed, Oct 11, 2017 at 10:58:45PM +0100, Ferruh Yigit wrote:
> On 10/11/2017 3:35 PM, Adrien Mazarguil wrote:
> > These wrappers implement the ability to allocate room for several disparate
> > objects as a single contiguous allocation while complying with their
> > respective alignment constraints.
> > 
> > This is usually more efficient than allocating and freeing them
> > individually if they are not expected to be reallocated with rte_realloc().
> > 
> > A typical use case is when several objects that cannot be dissociated must
> > be allocated together, as shown in the following example:
> > 
> >  struct b {
> >     ...
> >     struct d *d;
> >  }
> > 
> >  struct a {
> >      ...
> >      struct b *b;
> >      struct c *c;
> >  }
> > 
> >  struct rte_malloc_vec vec[] = {
> >      { .size = sizeof(struct a), .addr = &ptr_a, },
> >      { .size = sizeof(struct b), .addr = &ptr_b, },
> >      { .size = sizeof(struct c), .addr = &ptr_c, },
> >      { .size = sizeof(struct d), .addr = &ptr_d, },
> >  };
> > 
> >  if (!rte_mallocv(NULL, vec, RTE_DIM(vec)))
> >      goto error;
> > 
> >  struct a *a = ptr_a;
> > 
> >  a->b = ptr_b;
> >  a->c = ptr_c;
> >  a->b->d = ptr_d;
> >  ...
> >  rte_free(a);
> > 
> > Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> > Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> 
> Hi Adrien,
> 
> Why there is an eal patch in the middle of the mlx4 patchset?

Well, it was probably a mistake to leave it as is; it was more or less there
from the beginning and some other stuff I intended to send also relied on
it, which is why it got promoted to EAL. This series was actually supposed
to be sent much sooner but...

Anyway, I thought it could be useful to share it through EAL, and I secretly
hoped no one would notice.

> I believe this shouldn't go in via next-net tree, and should be reviewed
> properly.
> 
> I am behaving more flexible for PMD patches about the process and
> timing, because their scope is limited.
> PMD patches can break at most the PMD itself and if the maintainer is
> sending the patch, they should be knowing what they are doing, so vendor
> gets the responsibility for their own driver. I am paying majority of
> care to be sure it doesn't break others.
> 
> But ethdev and eal way beyond those flexibility, because their scope is
> much larger.
> 
> Can you please extract this patch from the patchset?

I'll submit v2 shortly to address this issue. I remain open to comments on
the usefulness of these functions, which I'll eventually re-send as a
separate patch.
  

Patch

diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index de25582..1ab50a5 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -201,6 +201,15 @@  DPDK_17.08 {
 
 } DPDK_17.05;
 
+DPDK_17.11 {
+	global:
+
+	rte_mallocv;
+	rte_mallocv_socket;
+	rte_zmallocv;
+	rte_zmallocv_socket;
+} DPDK_17.08;
+
 EXPERIMENTAL {
 	global:
 
diff --git a/lib/librte_eal/common/include/rte_malloc.h b/lib/librte_eal/common/include/rte_malloc.h
index 3d37f79..545697c 100644
--- a/lib/librte_eal/common/include/rte_malloc.h
+++ b/lib/librte_eal/common/include/rte_malloc.h
@@ -60,6 +60,13 @@  struct rte_malloc_socket_stats {
 	size_t heap_allocsz_bytes; /**< Total allocated bytes on heap */
 };
 
+/** Object description used with rte_mallocv() and similar functions. */
+struct rte_malloc_vec {
+	size_t align; /**< Alignment constraint (power of 2), 0 if unknown. */
+	size_t size; /**< Object size. */
+	void **addr; /**< Storage for allocation address. */
+};
+
 /**
  * This function allocates memory from the huge-page area of memory. The memory
  * is not cleared. In NUMA systems, the memory allocated resides on the same
@@ -335,6 +342,84 @@  rte_malloc_set_limit(const char *type, size_t max);
 phys_addr_t
 rte_malloc_virt2phy(const void *addr);
 
+/**
+ * Allocate memory once for several disparate objects.
+ *
+ * This function adds iovec-like semantics (e.g. readv()) to rte_malloc().
+ * Memory is allocated once for several contiguous objects of nonuniform
+ * sizes and alignment constraints.
+ *
+ * Each entry of @p vec describes the size, alignment constraint and
+ * provides a buffer address where the resulting object pointer must be
+ * stored.
+ *
+ * The buffer of the first entry is guaranteed to point to the beginning of
+ * the allocated region and is safe to use with rte_free().
+ *
+ * NULL buffers are silently ignored.
+ *
+ * Providing a NULL buffer in the first entry prevents this function from
+ * allocating any memory but has otherwise no effect on its behavior. In
+ * this case, the contents of remaining non-NULL buffers are updated with
+ * addresses relative to zero (i.e. offsets that would have been used during
+ * the allocation).
+ *
+ * @param[in] type
+ *   A string identifying the type of allocated objects (useful for debug
+ *   purposes, such as identifying the cause of a memory leak). Can be NULL.
+ * @param[in, out] vec
+ *   Description of objects to allocate memory for.
+ * @param cnt
+ *   Number of entries in @p vec.
+ *
+ * @return
+ *   Size in bytes of the allocated region including any padding. In case of
+ *   error, rte_errno is set, 0 is returned and NULL is stored in the
+ *   non-NULL buffers pointed by @p vec.
+ *
+ * @see rte_malloc()
+ * @see struct rte_malloc_vec
+ */
+size_t
+rte_mallocv(const char *type, const struct rte_malloc_vec *vec,
+	    unsigned int cnt);
+
+/**
+ * Combines the semantics of rte_mallocv() with those of rte_zmalloc().
+ *
+ * @see rte_mallocv()
+ * @see rte_zmalloc()
+ */
+size_t
+rte_zmallocv(const char *type, const struct rte_malloc_vec *vec,
+	     unsigned int cnt);
+
+/**
+ * Socket-aware version of rte_mallocv().
+ *
+ * This function takes one additional parameter.
+ *
+ * @param socket
+ *   NUMA socket to allocate memory on. If SOCKET_ID_ANY is used, this
+ *   function will behave the same as rte_mallocv().
+ *
+ * @see rte_mallocv()
+ */
+size_t
+rte_mallocv_socket(const char *type, const struct rte_malloc_vec *vec,
+		   unsigned int cnt, int socket);
+
+/**
+ * Combines the semantics of rte_mallocv_socket() with those of
+ * rte_zmalloc_socket().
+ *
+ * @see rte_mallocv_socket()
+ * @see rte_zmalloc_socket()
+ */
+size_t
+rte_zmallocv_socket(const char *type, const struct rte_malloc_vec *vec,
+		    unsigned int cnt, int socket);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/librte_eal/common/rte_malloc.c b/lib/librte_eal/common/rte_malloc.c
index d65c05a..6c7a4f7 100644
--- a/lib/librte_eal/common/rte_malloc.c
+++ b/lib/librte_eal/common/rte_malloc.c
@@ -41,6 +41,7 @@ 
 #include <rte_memory.h>
 #include <rte_eal.h>
 #include <rte_eal_memconfig.h>
+#include <rte_errno.h>
 #include <rte_branch_prediction.h>
 #include <rte_debug.h>
 #include <rte_launch.h>
@@ -265,3 +266,94 @@  rte_malloc_virt2phy(const void *addr)
 			((uintptr_t)addr - (uintptr_t)elem->ms->addr);
 	return paddr;
 }
+
+/*
+ * Internal helper to allocate memory once for several disparate objects.
+ *
+ * The most restrictive alignment constraint for standard objects is assumed
+ * to be sizeof(double) and is used as a default value.
+ *
+ * C11 code would include stdalign.h and use alignof(max_align_t) however
+ * we'll stick with C99 for the time being.
+ */
+static inline size_t
+rte_mallocv_inline(const char *type, const struct rte_malloc_vec *vec,
+		   unsigned int cnt, int zero, int socket)
+{
+	unsigned int i;
+	size_t size;
+	size_t least;
+	uint8_t *data = NULL;
+	int fill = !vec[0].addr;
+
+fill:
+	size = 0;
+	least = 0;
+	for (i = 0; i < cnt; ++i) {
+		size_t align = (uintptr_t)vec[i].align;
+
+		if (!align) {
+			align = sizeof(double);
+		} else if (!rte_is_power_of_2(align)) {
+			rte_errno = EINVAL;
+			goto error;
+		}
+		if (least < align)
+			least = align;
+		align = RTE_ALIGN_CEIL(size, align);
+		size = align + vec[i].size;
+		if (fill && vec[i].addr)
+			*vec[i].addr = data + align;
+	}
+	if (fill)
+		return size;
+	if (!zero)
+		data = rte_malloc_socket(type, size, least, socket);
+	else
+		data = rte_zmalloc_socket(type, size, least, socket);
+	if (data) {
+		fill = 1;
+		goto fill;
+	}
+	rte_errno = ENOMEM;
+error:
+	for (i = 0; i != cnt; ++i)
+		if (vec[i].addr)
+			*vec[i].addr = NULL;
+	return 0;
+}
+
+/* Allocate memory once for several disparate objects. */
+size_t
+rte_mallocv(const char *type, const struct rte_malloc_vec *vec,
+	    unsigned int cnt)
+{
+	return rte_mallocv_inline(type, vec, cnt, 0, SOCKET_ID_ANY);
+}
+
+/* Combines the semantics of rte_mallocv() with those of rte_zmalloc(). */
+size_t
+rte_zmallocv(const char *type, const struct rte_malloc_vec *vec,
+	     unsigned int cnt)
+{
+	return rte_mallocv_inline(type, vec, cnt, 1, SOCKET_ID_ANY);
+}
+
+/* Socket-aware version of rte_mallocv(). */
+size_t
+rte_mallocv_socket(const char *type, const struct rte_malloc_vec *vec,
+		   unsigned int cnt, int socket)
+{
+	return rte_mallocv_inline(type, vec, cnt, 0, socket);
+}
+
+/*
+ * Combines the semantics of rte_mallocv_socket() with those of
+ * rte_zmalloc_socket().
+ */
+size_t
+rte_zmallocv_socket(const char *type, const struct rte_malloc_vec *vec,
+		    unsigned int cnt, int socket)
+{
+	return rte_mallocv_inline(type, vec, cnt, 1, socket);
+}
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index 146156e..d620da3 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -205,6 +205,15 @@  DPDK_17.08 {
 
 } DPDK_17.05;
 
+DPDK_17.11 {
+	global:
+
+	rte_mallocv;
+	rte_mallocv_socket;
+	rte_zmallocv;
+	rte_zmallocv_socket;
+} DPDK_17.08;
+
 EXPERIMENTAL {
 	global: