Hi,
On 5/3/2023 8:26 AM, Ophir Munk wrote:
In current DPDK the RTE_MAX_MEMZONE definition is unconditionally hard
coded as 2560. For applications requiring different values of this
parameter – it is more convenient to set the max value via an rte API -
rather than changing the dpdk source code per application. In many
organizations, the possibility to compile a private DPDK library for a
particular application does not exist at all. With this option there is
no need to recompile DPDK and it allows using an in-box packaged DPDK.
An example usage for updating the RTE_MAX_MEMZONE would be of an
application that uses the DPDK mempool library which is based on DPDK
memzone library. The application may need to create a number of
steering tables, each of which will require its own mempool allocation.
This commit is not about how to optimize the application usage of
mempool nor about how to improve the mempool implementation based on
memzone. It is about how to make the max memzone definition - run-time
customized.
This commit adds an API which must be called before rte_eal_init():
rte_memzone_max_set(int max). If not called, the default memzone
(RTE_MAX_MEMZONE) is used. There is also an API to query the effective
max memzone: rte_memzone_max_get().
Commit message could use a little rewording and shortening. Suggested:
---
Currently, RTE_MAX_MEMZONE constant is used to decide how many memzones
a DPDK application can have. This value could technically be changed by
manually editing `rte_config.h` before compilation, but if DPDK is
already compiled, that option is not useful. There are certain use cases
that would benefit from making this value configurable.
This commit addresses the issue by adding a new API to set maximum
number of memzones before EAL initialization (while using the old
constant as default value), as well as an API to get current maximum
number of memzones.
---
What do you think?
/* Array of memzone pointers */
-static const struct rte_memzone *ecore_mz_mapping[RTE_MAX_MEMZONE];
+static const struct rte_memzone **ecore_mz_mapping;
/* Counter to track current memzone allocated */
static uint16_t ecore_mz_count;
+int ecore_mz_mapping_alloc(void)
+{
+ ecore_mz_mapping = rte_zmalloc("ecore_mz_map",
+ rte_memzone_max_get() * sizeof(struct rte_memzone *), 0);
Doesn't this need to check if it's already allocated? Does it need any
special secondary process handling?
+
+ if (!ecore_mz_mapping)
+ return -ENOMEM;
+
+ return 0;
+}
+
+void ecore_mz_mapping_free(void)
+{
+ rte_free(ecore_mz_mapping);
Shouldn't this at least set the pointer to NULL to avoid double-free?
+#define RTE_DEFAULT_MAX_MEMZONE 2560
+
+static size_t memzone_max = RTE_DEFAULT_MAX_MEMZONE;
+
static inline const struct rte_memzone *
memzone_lookup_thread_unsafe(const char *name)
{
@@ -81,8 +85,9 @@ memzone_reserve_aligned_thread_unsafe(const char *name,
size_t len,
/* no more room in config */
if (arr->count >= arr->len) {
RTE_LOG(ERR, EAL,
- "%s(): Number of requested memzone segments exceeds
RTE_MAX_MEMZONE\n",
- __func__);
+ "%s(): Number of requested memzone segments exceeds max "
+ "memzone segments (%d >= %d)\n",
I think the "segments" terminology can be dropped, it is a holdover from
the times when memzones were not allocated by malloc. The message can
just say "Number of requested memzones exceeds maximum number of memzones".
+ __func__, arr->count, arr->len);
rte_errno = ENOSPC;
return NULL;
}
@@ -396,7 +401,7 @@ rte_eal_memzone_init(void)
if (rte_eal_process_type() == RTE_PROC_PRIMARY &&
rte_fbarray_init(&mcfg->memzones, "memzone",
- RTE_MAX_MEMZONE, sizeof(struct rte_memzone))) {
+ rte_memzone_max_get(), sizeof(struct rte_memzone))) {
RTE_LOG(ERR, EAL, "Cannot allocate memzone list\n");
ret = -1;
} else if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
@@ -430,3 +435,20 @@ void rte_memzone_walk(void (*func)(const struct
rte_memzone *, void *),
}
rte_rwlock_read_unlock(&mcfg->mlock);
}
+
+int
+rte_memzone_max_set(size_t max)
+{
+ /* Setting max memzone must occur befaore calling rte_eal_init() */
+ if (eal_get_internal_configuration()->init_complete > 0)
+ return -1;
+
+ memzone_max = max;
+ return 0;
+}
+
+size_t
+rte_memzone_max_get(void)
+{
+ return memzone_max;
+}
It seems that this is a local (static) value, which means it is not
shared between processes, and thus could potentially mismatch between
two different processes. While this _technically_ would not be a problem
because secondary process init will not actually use this value, but the
API will still return incorrect information.
I suggest updating/syncing this value somewhere in
`eal_mcfg_update_internal()/eal_mcfg_update_from_internal()`, and adding
this value to mem config. An alternative to that would be to just return
`mem_config->memzones.count` (instead of the value itself), and return
-1 (or zero?) if init hasn't yet completed.
--
Thanks,
Anatoly
