On Wednesday 04 April 2018 04:52 AM, Anatoly Burakov wrote:
> It's there, so we might as well use it. Some operations will be
> sped up by that.
>
> Since we have to allocate an fbarray for memzones, we have to do
> it before we initialize memory subsystem, because that, in
> secondary processes, will (later) allocate more fbarrays than the
> primary process, which will result in inability to attach to
> memzone fbarray if we do it after the fact.
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>
> Notes:
> v3:
> - Moved earlier in patchset
> - Fixed compiled issues
> - Removed rte_panic() calls
>
[...]
> diff --git a/lib/librte_eal/common/eal_common_memzone.c b/lib/librte_eal/common/eal_common_memzone.c
> index 529b36f..aed9331 100644
> --- a/lib/librte_eal/common/eal_common_memzone.c
> +++ b/lib/librte_eal/common/eal_common_memzone.c
> @@ -28,42 +28,31 @@
> static inline const struct rte_memzone *
> memzone_lookup_thread_unsafe(const char *name)
> {
> - const struct rte_mem_config *mcfg;
> + struct rte_mem_config *mcfg;
> + struct rte_fbarray *arr;
> const struct rte_memzone *mz;
> - unsigned i = 0;
> + int i = 0;
>
> /* get pointer to global configuration */
> mcfg = rte_eal_get_configuration()->mem_config;
> + arr = &mcfg->memzones;
>
> /*
> * the algorithm is not optimal (linear), but there are few
> * zones and this function should be called at init only
> */
> - for (i = 0; i < RTE_MAX_MEMZONE; i++) {
> - mz = &mcfg->memzone[i];
> - if (mz->addr != NULL && !strncmp(name, mz->name, RTE_MEMZONE_NAMESIZE))
> - return &mcfg->memzone[i];
> + i = rte_fbarray_find_next_used(arr, 0);
> + while (i >= 0) {
> + mz = rte_fbarray_get(arr, i++);
^^^^^^^^
As discussed offline, this needs to be changed.
Double increment of 'i' leading to skips over lookup.
> + if (mz->addr != NULL &&
> + !strncmp(name, mz->name, RTE_MEMZONE_NAMESIZE))
> + return mz;
> + i = rte_fbarray_find_next_used(arr, i + 1);
> }
>
> return NULL;
> }
>
[..]
-
Shreyansh
@@ -264,11 +264,15 @@ static const struct eth_dev_ops ena_dev_ops = {
static inline int ena_cpu_to_node(int cpu)
{
struct rte_config *config = rte_eal_get_configuration();
+ struct rte_fbarray *arr = &config->mem_config->memzones;
+ const struct rte_memzone *mz;
- if (likely(cpu < RTE_MAX_MEMZONE))
- return config->mem_config->memzone[cpu].socket_id;
+ if (unlikely(cpu >= RTE_MAX_MEMZONE))
+ return NUMA_NO_NODE;
- return NUMA_NO_NODE;
+ mz = rte_fbarray_get(arr, cpu);
+
+ return mz->socket_id;
}
static inline void ena_rx_mbuf_prepare(struct rte_mbuf *mbuf,
@@ -599,14 +599,24 @@ rte_eal_init(int argc, char **argv)
}
}
+ /* in secondary processes, memory init may allocate additional fbarrays
+ * not present in primary processes, so to avoid any potential issues,
+ * initialize memzones first.
+ */
+ if (rte_eal_memzone_init() < 0) {
+ rte_eal_init_alert("Cannot init memzone\n");
+ rte_errno = ENODEV;
+ return -1;
+ }
+
if (rte_eal_memory_init() < 0) {
rte_eal_init_alert("Cannot init memory\n");
rte_errno = ENOMEM;
return -1;
}
- if (rte_eal_memzone_init() < 0) {
- rte_eal_init_alert("Cannot init memzone\n");
+ if (rte_eal_malloc_heap_init() < 0) {
+ rte_eal_init_alert("Cannot init malloc heap\n");
rte_errno = ENODEV;
return -1;
}
@@ -28,42 +28,31 @@
static inline const struct rte_memzone *
memzone_lookup_thread_unsafe(const char *name)
{
- const struct rte_mem_config *mcfg;
+ struct rte_mem_config *mcfg;
+ struct rte_fbarray *arr;
const struct rte_memzone *mz;
- unsigned i = 0;
+ int i = 0;
/* get pointer to global configuration */
mcfg = rte_eal_get_configuration()->mem_config;
+ arr = &mcfg->memzones;
/*
* the algorithm is not optimal (linear), but there are few
* zones and this function should be called at init only
*/
- for (i = 0; i < RTE_MAX_MEMZONE; i++) {
- mz = &mcfg->memzone[i];
- if (mz->addr != NULL && !strncmp(name, mz->name, RTE_MEMZONE_NAMESIZE))
- return &mcfg->memzone[i];
+ i = rte_fbarray_find_next_used(arr, 0);
+ while (i >= 0) {
+ mz = rte_fbarray_get(arr, i++);
+ if (mz->addr != NULL &&
+ !strncmp(name, mz->name, RTE_MEMZONE_NAMESIZE))
+ return mz;
+ i = rte_fbarray_find_next_used(arr, i + 1);
}
return NULL;
}
-static inline struct rte_memzone *
-get_next_free_memzone(void)
-{
- struct rte_mem_config *mcfg;
- unsigned i = 0;
-
- /* get pointer to global configuration */
- mcfg = rte_eal_get_configuration()->mem_config;
-
- for (i = 0; i < RTE_MAX_MEMZONE; i++) {
- if (mcfg->memzone[i].addr == NULL)
- return &mcfg->memzone[i];
- }
-
- return NULL;
-}
/* This function will return the greatest free block if a heap has been
* specified. If no heap has been specified, it will return the heap and
@@ -103,14 +92,16 @@ memzone_reserve_aligned_thread_unsafe(const char *name, size_t len,
{
struct rte_memzone *mz;
struct rte_mem_config *mcfg;
+ struct rte_fbarray *arr;
size_t requested_len;
- int socket, i;
+ int socket, i, mz_idx;
/* get pointer to global configuration */
mcfg = rte_eal_get_configuration()->mem_config;
+ arr = &mcfg->memzones;
/* no more room in config */
- if (mcfg->memzone_cnt >= RTE_MAX_MEMZONE) {
+ if (arr->count >= arr->len) {
RTE_LOG(ERR, EAL, "%s(): No more room in config\n", __func__);
rte_errno = ENOSPC;
return NULL;
@@ -219,7 +210,14 @@ memzone_reserve_aligned_thread_unsafe(const char *name, size_t len,
struct malloc_elem *elem = malloc_elem_from_data(mz_addr);
/* fill the zone in config */
- mz = get_next_free_memzone();
+ mz_idx = rte_fbarray_find_next_free(arr, 0);
+
+ if (mz_idx < 0) {
+ mz = NULL;
+ } else {
+ rte_fbarray_set_used(arr, mz_idx);
+ mz = rte_fbarray_get(arr, mz_idx);
+ }
if (mz == NULL) {
RTE_LOG(ERR, EAL, "%s(): Cannot find free memzone but there is room "
@@ -229,7 +227,6 @@ memzone_reserve_aligned_thread_unsafe(const char *name, size_t len,
return NULL;
}
- mcfg->memzone_cnt++;
snprintf(mz->name, sizeof(mz->name), "%s", name);
mz->iova = rte_malloc_virt2iova(mz_addr);
mz->addr = mz_addr;
@@ -342,34 +339,38 @@ int
rte_memzone_free(const struct rte_memzone *mz)
{
struct rte_mem_config *mcfg;
+ struct rte_fbarray *arr;
+ struct rte_memzone *found_mz;
int ret = 0;
- void *addr;
+ void *addr = NULL;
unsigned idx;
if (mz == NULL)
return -EINVAL;
mcfg = rte_eal_get_configuration()->mem_config;
+ arr = &mcfg->memzones;
rte_rwlock_write_lock(&mcfg->mlock);
- idx = ((uintptr_t)mz - (uintptr_t)mcfg->memzone);
- idx = idx / sizeof(struct rte_memzone);
+ idx = rte_fbarray_find_idx(arr, mz);
+ found_mz = rte_fbarray_get(arr, idx);
- addr = mcfg->memzone[idx].addr;
- if (addr == NULL)
+ if (found_mz == NULL) {
+ ret = -EINVAL;
+ } else if (found_mz->addr == NULL) {
+ RTE_LOG(ERR, EAL, "Memzone is not allocated\n");
ret = -EINVAL;
- else if (mcfg->memzone_cnt == 0) {
- rte_panic("%s(): memzone address not NULL but memzone_cnt is 0!\n",
- __func__);
} else {
- memset(&mcfg->memzone[idx], 0, sizeof(mcfg->memzone[idx]));
- mcfg->memzone_cnt--;
+ addr = found_mz->addr;
+ memset(found_mz, 0, sizeof(*found_mz));
+ rte_fbarray_set_free(arr, idx);
}
rte_rwlock_write_unlock(&mcfg->mlock);
- rte_free(addr);
+ if (addr != NULL)
+ rte_free(addr);
return ret;
}
@@ -405,7 +406,7 @@ dump_memzone(const struct rte_memzone *mz, void *arg)
size_t page_sz;
FILE *f = arg;
- mz_idx = mz - mcfg->memzone;
+ mz_idx = rte_fbarray_find_idx(&mcfg->memzones, mz);
fprintf(f, "Zone %u: name:<%s>, len:0x%zx, virt:%p, "
"socket_id:%"PRId32", flags:%"PRIx32"\n",
@@ -462,19 +463,23 @@ rte_eal_memzone_init(void)
/* get pointer to global configuration */
mcfg = rte_eal_get_configuration()->mem_config;
- /* secondary processes don't need to initialise anything */
- if (rte_eal_process_type() == RTE_PROC_SECONDARY)
- return 0;
-
rte_rwlock_write_lock(&mcfg->mlock);
- /* delete all zones */
- mcfg->memzone_cnt = 0;
- memset(mcfg->memzone, 0, sizeof(mcfg->memzone));
+ if (rte_eal_process_type() == RTE_PROC_PRIMARY &&
+ rte_fbarray_init(&mcfg->memzones, "memzone",
+ RTE_MAX_MEMZONE, sizeof(struct rte_memzone))) {
+ RTE_LOG(ERR, EAL, "Cannot allocate memzone list\n");
+ return -1;
+ } else if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
+ rte_fbarray_attach(&mcfg->memzones)) {
+ RTE_LOG(ERR, EAL, "Cannot attach to memzone list\n");
+ rte_rwlock_write_unlock(&mcfg->mlock);
+ return -1;
+ }
rte_rwlock_write_unlock(&mcfg->mlock);
- return rte_eal_malloc_heap_init();
+ return 0;
}
/* Walk all reserved memory zones */
@@ -482,14 +487,18 @@ void rte_memzone_walk(void (*func)(const struct rte_memzone *, void *),
void *arg)
{
struct rte_mem_config *mcfg;
- unsigned i;
+ struct rte_fbarray *arr;
+ int i;
mcfg = rte_eal_get_configuration()->mem_config;
+ arr = &mcfg->memzones;
rte_rwlock_read_lock(&mcfg->mlock);
- for (i=0; i<RTE_MAX_MEMZONE; i++) {
- if (mcfg->memzone[i].addr != NULL)
- (*func)(&mcfg->memzone[i], arg);
+ i = rte_fbarray_find_next_used(arr, 0);
+ while (i >= 0) {
+ struct rte_memzone *mz = rte_fbarray_get(arr, i);
+ (*func)(mz, arg);
+ i = rte_fbarray_find_next_used(arr, i + 1);
}
rte_rwlock_read_unlock(&mcfg->mlock);
}
@@ -58,10 +58,8 @@ struct rte_mem_config {
rte_rwlock_t qlock; /**< used for tailq operation for thread safe. */
rte_rwlock_t mplock; /**< only used by mempool LIB for thread-safe. */
- uint32_t memzone_cnt; /**< Number of allocated memzones */
-
/* memory segments and zones */
- struct rte_memzone memzone[RTE_MAX_MEMZONE]; /**< Memzone descriptors. */
+ struct rte_fbarray memzones; /**< Memzone descriptors. */
struct rte_memseg_list memsegs[RTE_MAX_MEMSEG_LISTS];
/**< list of dynamic arrays holding memsegs */
@@ -278,6 +278,10 @@ rte_eal_malloc_heap_init(void)
if (mcfg == NULL)
return -1;
+ /* secondary process does not need to initialize anything */
+ if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+ return 0;
+
/* add all IOVA-contiguous areas to the heap */
return rte_memseg_contig_walk(malloc_add_seg, NULL);
}
@@ -858,6 +858,15 @@ rte_eal_init(int argc, char **argv)
return -1;
}
#endif
+ /* in secondary processes, memory init may allocate additional fbarrays
+ * not present in primary processes, so to avoid any potential issues,
+ * initialize memzones first.
+ */
+ if (rte_eal_memzone_init() < 0) {
+ rte_eal_init_alert("Cannot init memzone\n");
+ rte_errno = ENODEV;
+ return -1;
+ }
if (rte_eal_memory_init() < 0) {
rte_eal_init_alert("Cannot init memory\n");
@@ -868,8 +877,8 @@ rte_eal_init(int argc, char **argv)
/* the directories are locked during eal_hugepage_info_init */
eal_hugedirs_unlock();
- if (rte_eal_memzone_init() < 0) {
- rte_eal_init_alert("Cannot init memzone\n");
+ if (rte_eal_malloc_heap_init() < 0) {
+ rte_eal_init_alert("Cannot init malloc heap\n");
rte_errno = ENODEV;
return -1;
}
@@ -909,7 +909,7 @@ test_memzone_basic(void)
const struct rte_memzone *mz;
int memzone_cnt_after, memzone_cnt_expected;
int memzone_cnt_before =
- rte_eal_get_configuration()->mem_config->memzone_cnt;
+ rte_eal_get_configuration()->mem_config->memzones.count;
memzone1 = rte_memzone_reserve(TEST_MEMZONE_NAME("testzone1"), 100,
SOCKET_ID_ANY, 0);
@@ -933,7 +933,7 @@ test_memzone_basic(void)
(memzone3 != NULL) + (memzone4 != NULL);
memzone_cnt_after =
- rte_eal_get_configuration()->mem_config->memzone_cnt;
+ rte_eal_get_configuration()->mem_config->memzones.count;
if (memzone_cnt_after != memzone_cnt_expected)
return -1;
@@ -1012,7 +1012,7 @@ test_memzone_basic(void)
}
memzone_cnt_after =
- rte_eal_get_configuration()->mem_config->memzone_cnt;
+ rte_eal_get_configuration()->mem_config->memzones.count;
if (memzone_cnt_after != memzone_cnt_before)
return -1;
@@ -1033,7 +1033,8 @@ static int
test_memzone(void)
{
/* take note of how many memzones were allocated before running */
- int memzone_cnt = rte_eal_get_configuration()->mem_config->memzone_cnt;
+ int memzone_cnt =
+ rte_eal_get_configuration()->mem_config->memzones.count;
printf("test basic memzone API\n");
if (test_memzone_basic() < 0)