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. > +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. > +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. > +/** > + * 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 :) > +}; > + > +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?
