[RFC,01/11] mem: allow memseg lists to be marked as external
diff mbox series

Message ID a4acb67d720b790d81f468f8c64ec9ebcb23a08b.1530881548.git.anatoly.burakov@intel.com
State Superseded, archived
Delegated to: Thomas Monjalon
Headers show
Series
  • Support externally allocated memory in DPDK
Related show

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Burakov, Anatoly July 6, 2018, 1:17 p.m. UTC
When we allocate and use DPDK memory, we need to be able to
differentiate between DPDK hugepage segments and segments that
were made part of DPDK but are externally allocated. Add such
a property to memseg lists.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/librte_eal/common/eal_common_memory.c     | 51 ++++++++++++++++---
 .../common/include/rte_eal_memconfig.h        |  1 +
 lib/librte_eal/common/malloc_heap.c           |  2 +-
 3 files changed, 47 insertions(+), 7 deletions(-)

Comments

Alejandro Lucero July 10, 2018, 11:18 a.m. UTC | #1
On Fri, Jul 6, 2018 at 2:17 PM, Anatoly Burakov <anatoly.burakov@intel.com>
wrote:

> When we allocate and use DPDK memory, we need to be able to
> differentiate between DPDK hugepage segments and segments that
> were made part of DPDK but are externally allocated. Add such
> a property to memseg lists.
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>  lib/librte_eal/common/eal_common_memory.c     | 51 ++++++++++++++++---
>  .../common/include/rte_eal_memconfig.h        |  1 +
>  lib/librte_eal/common/malloc_heap.c           |  2 +-
>  3 files changed, 47 insertions(+), 7 deletions(-)
>
> diff --git a/lib/librte_eal/common/eal_common_memory.c
> b/lib/librte_eal/common/eal_common_memory.c
> index 4f0688f9d..835bbffb6 100644
> --- a/lib/librte_eal/common/eal_common_memory.c
> +++ b/lib/librte_eal/common/eal_common_memory.c
> @@ -24,6 +24,21 @@
>  #include "eal_private.h"
>  #include "eal_internal_cfg.h"
>
> +/* forward declarations for memseg walk functions. we support external
> segments,
> + * but for some functionality to work, we need to either skip or not skip
> + * external segments. for example, while we expect for virt2memseg to
> return a
> + * valid memseg even though it's an external memseg, for regular memseg
> walk we
> + * want to skip those because the expectation is that we will only walk
> the
> + * DPDK allocated memory.
> + */
>

I do not clear understand when external segments can be used along with
hugepages segments, but if this is a possibility, should not we support
memseg walk for just external segments and also to allow any walk type as
an API? If I'm right, it seems just memseg walk skipping external memsegs
can be invoked from other files.


> +static int
> +memseg_list_walk(rte_memseg_list_walk_t func, void *arg, bool
> skip_external);
> +static int
> +memseg_walk(rte_memseg_walk_t func, void *arg, bool skip_external);
> +static int
> +memseg_contig_walk(rte_memseg_contig_walk_t func, void *arg,
> +               bool skip_external);
> +
>  /*
>   * Try to mmap *size bytes in /dev/zero. If it is successful, return the
>   * pointer to the mmap'd area and keep *size unmodified. Else, retry
> @@ -621,9 +636,9 @@ rte_mem_iova2virt(rte_iova_t iova)
>          * as we know they are PA-contiguous as well
>          */
>         if (internal_config.legacy_mem)
> -               rte_memseg_contig_walk(find_virt_legacy, &vi);
> +               memseg_contig_walk(find_virt_legacy, &vi, false);
>         else
> -               rte_memseg_walk(find_virt, &vi);
> +               memseg_walk(find_virt, &vi, false);
>
>         return vi.virt;
>  }
> @@ -787,8 +802,8 @@ rte_mem_lock_page(const void *virt)
>         return mlock((void *)aligned, page_size);
>  }
>
> -int __rte_experimental
> -rte_memseg_contig_walk(rte_memseg_contig_walk_t func, void *arg)
> +static int
> +memseg_contig_walk(rte_memseg_contig_walk_t func, void *arg, bool
> skip_external)
>  {
>         struct rte_mem_config *mcfg = rte_eal_get_configuration()->
> mem_config;
>         int i, ms_idx, ret = 0;
> @@ -803,6 +818,8 @@ rte_memseg_contig_walk(rte_memseg_contig_walk_t func,
> void *arg)
>
>                 if (msl->memseg_arr.count == 0)
>                         continue;
> +               if (skip_external && msl->external)
> +                       continue;
>
>                 arr = &msl->memseg_arr;
>
> @@ -837,7 +854,13 @@ rte_memseg_contig_walk(rte_memseg_contig_walk_t
> func, void *arg)
>  }
>
>  int __rte_experimental
> -rte_memseg_walk(rte_memseg_walk_t func, void *arg)
> +rte_memseg_contig_walk(rte_memseg_contig_walk_t func, void *arg)
> +{
> +       return memseg_contig_walk(func, arg, true);
> +}
> +
> +static int
> +memseg_walk(rte_memseg_walk_t func, void *arg, bool skip_external)
>  {
>         struct rte_mem_config *mcfg = rte_eal_get_configuration()->
> mem_config;
>         int i, ms_idx, ret = 0;
> @@ -852,6 +875,8 @@ rte_memseg_walk(rte_memseg_walk_t func, void *arg)
>
>                 if (msl->memseg_arr.count == 0)
>                         continue;
> +               if (skip_external && msl->external)
> +                       continue;
>
>                 arr = &msl->memseg_arr;
>
> @@ -875,7 +900,13 @@ rte_memseg_walk(rte_memseg_walk_t func, void *arg)
>  }
>
>  int __rte_experimental
> -rte_memseg_list_walk(rte_memseg_list_walk_t func, void *arg)
> +rte_memseg_walk(rte_memseg_walk_t func, void *arg)
> +{
> +       return memseg_walk(func, arg, true);
> +}
> +
> +static int
> +memseg_list_walk(rte_memseg_list_walk_t func, void *arg, bool
> skip_external)
>  {
>         struct rte_mem_config *mcfg = rte_eal_get_configuration()->
> mem_config;
>         int i, ret = 0;
> @@ -888,6 +919,8 @@ rte_memseg_list_walk(rte_memseg_list_walk_t func,
> void *arg)
>
>                 if (msl->base_va == NULL)
>                         continue;
> +               if (skip_external && msl->external)
> +                       continue;
>
>                 ret = func(msl, arg);
>                 if (ret < 0) {
> @@ -904,6 +937,12 @@ rte_memseg_list_walk(rte_memseg_list_walk_t func,
> void *arg)
>         return ret;
>  }
>
> +int __rte_experimental
> +rte_memseg_list_walk(rte_memseg_list_walk_t func, void *arg)
> +{
> +       return memseg_list_walk(func, arg, true);
> +}
> +
>  /* init memory subsystem */
>  int
>  rte_eal_memory_init(void)
> diff --git a/lib/librte_eal/common/include/rte_eal_memconfig.h
> b/lib/librte_eal/common/include/rte_eal_memconfig.h
> index aff0688dd..4e8720ba6 100644
> --- a/lib/librte_eal/common/include/rte_eal_memconfig.h
> +++ b/lib/librte_eal/common/include/rte_eal_memconfig.h
> @@ -30,6 +30,7 @@ struct rte_memseg_list {
>                 uint64_t addr_64;
>                 /**< Makes sure addr is always 64-bits */
>         };
> +       bool external; /**< true if this list points to external memory */
>         int socket_id; /**< Socket ID for all memsegs in this list. */
>         uint64_t page_sz; /**< Page size for all memsegs in this list. */
>         volatile uint32_t version; /**< version number for multiprocess
> sync. */
> diff --git a/lib/librte_eal/common/malloc_heap.c b/lib/librte_eal/common/
> malloc_heap.c
> index d6cf3af81..8a1f54905 100644
> --- a/lib/librte_eal/common/malloc_heap.c
> +++ b/lib/librte_eal/common/malloc_heap.c
> @@ -631,7 +631,7 @@ malloc_heap_free(struct malloc_elem *elem)
>         ret = 0;
>
>         /* ...of which we can't avail if we are in legacy mode */
> -       if (internal_config.legacy_mem)
> +       if (internal_config.legacy_mem || msl->external)
>                 goto free_unlock;
>
>         /* check if we can free any memory back to the system */
> --
> 2.17.1
>
Burakov, Anatoly July 10, 2018, 11:31 a.m. UTC | #2
On 10-Jul-18 12:18 PM, Alejandro Lucero wrote:
> On Fri, Jul 6, 2018 at 2:17 PM, Anatoly Burakov <anatoly.burakov@intel.com>
> wrote:
> 
>> When we allocate and use DPDK memory, we need to be able to
>> differentiate between DPDK hugepage segments and segments that
>> were made part of DPDK but are externally allocated. Add such
>> a property to memseg lists.
>>
>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>> ---
>>   lib/librte_eal/common/eal_common_memory.c     | 51 ++++++++++++++++---
>>   .../common/include/rte_eal_memconfig.h        |  1 +
>>   lib/librte_eal/common/malloc_heap.c           |  2 +-
>>   3 files changed, 47 insertions(+), 7 deletions(-)
>>
>> diff --git a/lib/librte_eal/common/eal_common_memory.c
>> b/lib/librte_eal/common/eal_common_memory.c
>> index 4f0688f9d..835bbffb6 100644
>> --- a/lib/librte_eal/common/eal_common_memory.c
>> +++ b/lib/librte_eal/common/eal_common_memory.c
>> @@ -24,6 +24,21 @@
>>   #include "eal_private.h"
>>   #include "eal_internal_cfg.h"
>>
>> +/* forward declarations for memseg walk functions. we support external
>> segments,
>> + * but for some functionality to work, we need to either skip or not skip
>> + * external segments. for example, while we expect for virt2memseg to
>> return a
>> + * valid memseg even though it's an external memseg, for regular memseg
>> walk we
>> + * want to skip those because the expectation is that we will only walk
>> the
>> + * DPDK allocated memory.
>> + */
>>
> 
> I do not clear understand when external segments can be used along with
> hugepages segments, but if this is a possibility, should not we support
> memseg walk for just external segments and also to allow any walk type as
> an API? If I'm right, it seems just memseg walk skipping external memsegs
> can be invoked from other files.

Well, now that i think of it, every walk function will receive a memseg 
list along with memseg itself, so user code can check to skip external 
memsegs. So yes, you're correct. I'll do that in the next iteration. 
Thanks for your feedback!

Patch
diff mbox series

diff --git a/lib/librte_eal/common/eal_common_memory.c b/lib/librte_eal/common/eal_common_memory.c
index 4f0688f9d..835bbffb6 100644
--- a/lib/librte_eal/common/eal_common_memory.c
+++ b/lib/librte_eal/common/eal_common_memory.c
@@ -24,6 +24,21 @@ 
 #include "eal_private.h"
 #include "eal_internal_cfg.h"
 
+/* forward declarations for memseg walk functions. we support external segments,
+ * but for some functionality to work, we need to either skip or not skip
+ * external segments. for example, while we expect for virt2memseg to return a
+ * valid memseg even though it's an external memseg, for regular memseg walk we
+ * want to skip those because the expectation is that we will only walk the
+ * DPDK allocated memory.
+ */
+static int
+memseg_list_walk(rte_memseg_list_walk_t func, void *arg, bool skip_external);
+static int
+memseg_walk(rte_memseg_walk_t func, void *arg, bool skip_external);
+static int
+memseg_contig_walk(rte_memseg_contig_walk_t func, void *arg,
+		bool skip_external);
+
 /*
  * Try to mmap *size bytes in /dev/zero. If it is successful, return the
  * pointer to the mmap'd area and keep *size unmodified. Else, retry
@@ -621,9 +636,9 @@  rte_mem_iova2virt(rte_iova_t iova)
 	 * as we know they are PA-contiguous as well
 	 */
 	if (internal_config.legacy_mem)
-		rte_memseg_contig_walk(find_virt_legacy, &vi);
+		memseg_contig_walk(find_virt_legacy, &vi, false);
 	else
-		rte_memseg_walk(find_virt, &vi);
+		memseg_walk(find_virt, &vi, false);
 
 	return vi.virt;
 }
@@ -787,8 +802,8 @@  rte_mem_lock_page(const void *virt)
 	return mlock((void *)aligned, page_size);
 }
 
-int __rte_experimental
-rte_memseg_contig_walk(rte_memseg_contig_walk_t func, void *arg)
+static int
+memseg_contig_walk(rte_memseg_contig_walk_t func, void *arg, bool skip_external)
 {
 	struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config;
 	int i, ms_idx, ret = 0;
@@ -803,6 +818,8 @@  rte_memseg_contig_walk(rte_memseg_contig_walk_t func, void *arg)
 
 		if (msl->memseg_arr.count == 0)
 			continue;
+		if (skip_external && msl->external)
+			continue;
 
 		arr = &msl->memseg_arr;
 
@@ -837,7 +854,13 @@  rte_memseg_contig_walk(rte_memseg_contig_walk_t func, void *arg)
 }
 
 int __rte_experimental
-rte_memseg_walk(rte_memseg_walk_t func, void *arg)
+rte_memseg_contig_walk(rte_memseg_contig_walk_t func, void *arg)
+{
+	return memseg_contig_walk(func, arg, true);
+}
+
+static int
+memseg_walk(rte_memseg_walk_t func, void *arg, bool skip_external)
 {
 	struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config;
 	int i, ms_idx, ret = 0;
@@ -852,6 +875,8 @@  rte_memseg_walk(rte_memseg_walk_t func, void *arg)
 
 		if (msl->memseg_arr.count == 0)
 			continue;
+		if (skip_external && msl->external)
+			continue;
 
 		arr = &msl->memseg_arr;
 
@@ -875,7 +900,13 @@  rte_memseg_walk(rte_memseg_walk_t func, void *arg)
 }
 
 int __rte_experimental
-rte_memseg_list_walk(rte_memseg_list_walk_t func, void *arg)
+rte_memseg_walk(rte_memseg_walk_t func, void *arg)
+{
+	return memseg_walk(func, arg, true);
+}
+
+static int
+memseg_list_walk(rte_memseg_list_walk_t func, void *arg, bool skip_external)
 {
 	struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config;
 	int i, ret = 0;
@@ -888,6 +919,8 @@  rte_memseg_list_walk(rte_memseg_list_walk_t func, void *arg)
 
 		if (msl->base_va == NULL)
 			continue;
+		if (skip_external && msl->external)
+			continue;
 
 		ret = func(msl, arg);
 		if (ret < 0) {
@@ -904,6 +937,12 @@  rte_memseg_list_walk(rte_memseg_list_walk_t func, void *arg)
 	return ret;
 }
 
+int __rte_experimental
+rte_memseg_list_walk(rte_memseg_list_walk_t func, void *arg)
+{
+	return memseg_list_walk(func, arg, true);
+}
+
 /* init memory subsystem */
 int
 rte_eal_memory_init(void)
diff --git a/lib/librte_eal/common/include/rte_eal_memconfig.h b/lib/librte_eal/common/include/rte_eal_memconfig.h
index aff0688dd..4e8720ba6 100644
--- a/lib/librte_eal/common/include/rte_eal_memconfig.h
+++ b/lib/librte_eal/common/include/rte_eal_memconfig.h
@@ -30,6 +30,7 @@  struct rte_memseg_list {
 		uint64_t addr_64;
 		/**< Makes sure addr is always 64-bits */
 	};
+	bool external; /**< true if this list points to external memory */
 	int socket_id; /**< Socket ID for all memsegs in this list. */
 	uint64_t page_sz; /**< Page size for all memsegs in this list. */
 	volatile uint32_t version; /**< version number for multiprocess sync. */
diff --git a/lib/librte_eal/common/malloc_heap.c b/lib/librte_eal/common/malloc_heap.c
index d6cf3af81..8a1f54905 100644
--- a/lib/librte_eal/common/malloc_heap.c
+++ b/lib/librte_eal/common/malloc_heap.c
@@ -631,7 +631,7 @@  malloc_heap_free(struct malloc_elem *elem)
 	ret = 0;
 
 	/* ...of which we can't avail if we are in legacy mode */
-	if (internal_config.legacy_mem)
+	if (internal_config.legacy_mem || msl->external)
 		goto free_unlock;
 
 	/* check if we can free any memory back to the system */