Re: [dpdk-dev] [PATCH v4 4/8] eal: extract common code for memseg list initialization

[Burakov, Anatoly]

On 29-Apr-20 12:50 AM, Dmitry Kozlyuk wrote:
All supported OS create memory segment lists (MSL) and reserve VA space
for them in a nearly identical way. Move common code into EAL private
functions to reduce duplication.
Signed-off-by: Dmitry Kozlyuk <dmitry.kozl...@gmail.com>
---

Would it be possible to extract all similar code and make it use 
eal_memseg_list_init? For example, in FreeBSD eal_memory.c in nohuge 
mode (or in Linux legacy mem) the code remains untouched. I wonder if it 
can be ported to using this function.

  lib/librte_eal/common/eal_common_memory.c | 54 ++++++++++++++++++
  lib/librte_eal/common/eal_private.h       | 36 ++++++++++++
  lib/librte_eal/freebsd/eal_memory.c       | 57 +++----------------
  lib/librte_eal/linux/eal_memory.c         | 68 +++++------------------
  4 files changed, 113 insertions(+), 102 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_memory.c 
b/lib/librte_eal/common/eal_common_memory.c
index 1196a8037..56eff0acb 100644
--- a/lib/librte_eal/common/eal_common_memory.c
+++ b/lib/librte_eal/common/eal_common_memory.c
@@ -24,6 +24,7 @@
  #include "eal_private.h"
  #include "eal_internal_cfg.h"
  #include "eal_memcfg.h"
+#include "eal_options.h"
  #include "malloc_heap.h"
  
  /*
@@ -181,6 +182,59 @@ eal_get_virtual_area(void *requested_addr, size_t *size,
        return aligned_addr;
  }
  
+int
+eal_memseg_list_init(struct rte_memseg_list *msl, uint64_t page_sz,
+               int n_segs, int socket_id, int type_msl_idx, bool heap)
+{
+       char name[RTE_FBARRAY_NAME_LEN];
+
+       snprintf(name, sizeof(name), MEMSEG_LIST_FMT, page_sz >> 10, socket_id,
+                type_msl_idx);
+       if (rte_fbarray_init(&msl->memseg_arr, name, n_segs,
+                       sizeof(struct rte_memseg))) {
+               RTE_LOG(ERR, EAL, "Cannot allocate memseg list: %s\n",
+                       rte_strerror(rte_errno));
+               return -1;
+       }
+
+       msl->page_sz = page_sz;
+       msl->socket_id = socket_id;
+       msl->base_va = NULL;
+       msl->heap = heap;
+
+       RTE_LOG(DEBUG, EAL, "Memseg list allocated: 0x%zxkB at socket %i\n",
+                       (size_t)page_sz >> 10, socket_id);

The log message looks odd here. Maybe change it to indicate that the kB 
value is the page size, not the size of the memory?

+
+       return 0;
+}
+
+int
+eal_memseg_list_alloc(struct rte_memseg_list *msl, int reserve_flags)
+{
+       uint64_t page_sz;
+       size_t mem_sz;
+       void *addr;
+
+       page_sz = msl->page_sz;
+       mem_sz = page_sz * msl->memseg_arr.len;
+
+       addr = eal_get_virtual_area(
+               msl->base_va, &mem_sz, page_sz, 0, reserve_flags);
+       if (addr == NULL) {
+               if (rte_errno == EADDRNOTAVAIL)
+                       RTE_LOG(ERR, EAL, "Cannot reserve %llu bytes at [%p] - "
+                               "please use '--" OPT_BASE_VIRTADDR "' option\n",
+                               (unsigned long long)mem_sz, msl->base_va);
+               else
+                       RTE_LOG(ERR, EAL, "Cannot reserve memory\n");
+               return -1;
+       }
+       msl->base_va = addr;
+       msl->len = mem_sz;

Perhaps add a log message saying that space was allocated for this 
memseg list?

--
Thanks,
Anatoly