Overall, this is ok, but why can't it be the default?
Lots of little things should be cleaned up
> +struct rte_mempool_common_stack
> +{
> + /* Spinlock to protect access */
> + rte_spinlock_t sl;
> +
> + uint32_t size;
> + uint32_t len;
> + void *objs[];
> +
> +#ifdef RTE_LIBRTE_MEMPOOL_DEBUG
> +#endif
Useless remove it
> +};
> +
> +static void *
> +common_stack_alloc(struct rte_mempool *mp)
> +{
> + struct rte_mempool_common_stack *s;
> + char stack_name[RTE_RING_NAMESIZE];
> + unsigned n = mp->size;
> + int size = sizeof(*s) + (n+16)*sizeof(void*);
> +
> + /* Allocate our local memory structure */
> + snprintf(stack_name, sizeof(stack_name), "%s-common-stack", mp->name);
> + s = rte_zmalloc_socket(stack_name,
The name for zmalloc is ignored in current code, why bother making it unique.
> + size,
> + RTE_CACHE_LINE_SIZE,
> + mp->socket_id);
> + if (s == NULL) {
> + RTE_LOG(ERR, MEMPOOL, "Cannot allocate stack!\n");
> + return NULL;
> + }
> +
> + /* And the spinlock we use to protect access */
> + rte_spinlock_init(&s->sl);
> +
> + s->size = n;
> + mp->pool = (void *) s;
Since pool is void *, no need for a cast here
> + rte_mempool_set_handler(mp, "stack");
> +
> + return (void *) s;
> +}
> +
> +static int common_stack_put(void *p, void * const *obj_table,
> + unsigned n)
> +{
> + struct rte_mempool_common_stack * s = (struct rte_mempool_common_stack
> *)p;
> + void **cache_objs;
> + unsigned index;
> +
> + /* Acquire lock */
Useless obvious comment, about as good a classic /* Add one to i */
>
> +static int common_stack_get(void *p, void **obj_table,
> + unsigned n)
> +{
> + struct rte_mempool_common_stack * s = (struct rte_mempool_common_stack
> *)p;
Unnecessary cast, in C void * can be assigned to any type.
> + void **cache_objs;
> + unsigned index, len;
> +
> + /* Acquire lock */
Yet another useless comment.
> + rte_spinlock_lock(&s->sl);
> +
> + if(unlikely(n > s->len)) {
> + rte_spinlock_unlock(&s->sl);
> + return -ENOENT;
> + }
> +
> + cache_objs = s->objs;
> +
> + for (index = 0, len = s->len - 1; index < n; ++index, len--,
> obj_table++)
> + *obj_table = cache_objs[len];
> +
> + s->len -= n;
> + rte_spinlock_unlock(&s->sl);
> + return n;
> +}
> +
> +static unsigned common_stack_get_count(void *p)
> +{
> + struct rte_mempool_common_stack * s = (struct rte_mempool_common_stack
> *)p;
Another useless cast.
> + return s->len;
> +}
> +
> +static void
> +common_stack_free(void *p)
> +{
> + rte_free((struct rte_mempool_common_stack *)p);
Yet another useless cast
> +}
> +
> +static struct rte_mempool_handler handler_stack = {
For security, any data structure with function pointers should be const.
> + .name = "stack",
> + .alloc = common_stack_alloc,
> + .free = common_stack_free,
> + .put = common_stack_put,
> + .get = common_stack_get,
> + .get_count = common_stack_get_count
> +};
> +
> +MEMPOOL_REGISTER_HANDLER(handler_stack);
