Hi Dmitry,
On 2022/9/20 19:30, Dmitry Kozlyuk wrote:
2022-09-20 03:46 (UTC+0000), Chengwen Feng:
The memarea library is an allocator of variable-size object. It is a
collection of allocated objects that can be efficiently alloc or free
all at once, the main features are as follows:
a) it facilitate alloc and free of memory with low overhead.
Yet, the overhead is 64B per element, just like rte_malloc.
b) it provides refcnt feature which could be useful in some scenes.
Are you sure refcnt should be in this library?
I've expressed my concerns here:
https://inbox.dpdk.org/dev/caeyuuwbpc-9dcqkj0lzi6rkcuwyeyeghlrbmbubtux4ljg+...@mail.gmail.com
There are more unanswered questions in that mail,
it would be good to clarify them before reviewing these patches
in order to understand all the intentions.
Sorry to forgot reply it.
We have the following scene which used refcnt:
nic-rx -> decoder -> process
|
-> recording
as above show, the process and recording module both use the decoder's
output, the are just reader.
so in this case, the refcnt is useful.
+static int
+memarea_check_param(const struct rte_memarea_param *init)
+{
+ size_t len;
+
+ len = strnlen(init->name, RTE_MEMAREA_NAMESIZE);
+ if (len == 0 || len >= RTE_MEMAREA_NAMESIZE) {
+ RTE_LOG(ERR, MEMAREA, "memarea name invalid!\n");
+ return -EINVAL;
+ }
Please check init->name == NULL first.
No need checking because name is an array.
Maybe I should check init == NULL here.
+struct rte_memarea *
+rte_memarea_create(const struct rte_memarea_param *init)
+{
[...]
+ RTE_LOG(ERR, MEMAREA, "malloc memarea management obj fail!\n");
In all error messages, it would be useful to provide details:
the name of the area, what size was requested, etc.
will fix in v2.
+/**
+ * Memarea memory source.
+ */
+enum rte_memarea_source {
+ /** Memory source comes from system API (e.g. malloc). */
+ RTE_MEMAREA_SOURCE_SYSTEM_API,
+ /** Memory source comes from user-provided address. */
+ RTE_MEMAREA_SOURCE_USER_ADDR,
+ /** Memory source comes from user-provided memarea. */
+ RTE_MEMAREA_SOURCE_USER_MEMAREA,
+
+ RTE_MEMAREA_SOURCE_BUTT
DPDK enumerations must not include an item to hold the element count,
because it is harmful for ABI (e.g. developers create arrays of this size
and when a new item is added in a new DPDK version, the array overflows).
If it's supposed to mean "the end of item list",
the proper word would be "last" or "max" BTW :)
will fix in v2
+};
+
+struct rte_memarea {
+ void *private_data; /**< private management data pointer. */
+};
Jerin and Stephen suggested to make the structure opaque,
i.e. only declare the struct and define it privately.
It would reduce ABI and simplify allocation.
Any justification to expose it?
do you mean the rte_memarea just void * ? it just (void
*)(memarea_private *)priv ?
It's another popular type to impl ABI compatiable.
It's more simpler, will fix in v2
