From: Stephen Hemminger <stephen@networkplumber.org>
To: David Hunt <david.hunt@intel.com>
Cc: olivier.matz@6wind.com, dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH 1/2] mempool: add stack (fifo) mempool handler
Date: Thu, 5 May 2016 14:28:35 -0700 [thread overview]
Message-ID: <20160505142835.7df35079@xeon-e3> (raw)
In-Reply-To: <1462472982-49782-2-git-send-email-david.hunt@intel.com>
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);
next prev parent reply other threads:[~2016-05-05 21:28 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-05-05 18:29 [dpdk-dev] [PATCH 0/2] " David Hunt
2016-05-05 18:29 ` [dpdk-dev] [PATCH 1/2] " David Hunt
2016-05-05 21:28 ` Stephen Hemminger [this message]
2016-05-19 15:21 ` Hunt, David
2016-05-05 18:29 ` [dpdk-dev] [PATCH 2/2] test: add autotest for external mempool stack handler David Hunt
2016-05-06 8:34 ` [dpdk-dev] [PATCH 0/2] mempool: add stack (fifo) mempool handler Tan, Jianfeng
2016-05-06 23:02 ` Hunt, David
2016-05-19 14:48 ` [dpdk-dev] v2 mempool: add stack (lifo) " David Hunt
2016-05-19 14:48 ` [dpdk-dev] [PATCH v2 1/3] " David Hunt
2016-05-23 12:55 ` Olivier Matz
2016-06-15 10:10 ` Hunt, David
2016-06-17 14:18 ` Hunt, David
2016-06-20 8:17 ` Olivier Matz
2016-06-20 12:59 ` Hunt, David
2016-06-29 14:31 ` Olivier MATZ
2016-05-19 14:48 ` [dpdk-dev] [PATCH v2 2/3] mempool: make declaration of handler structs const David Hunt
2016-05-23 12:55 ` Olivier Matz
2016-05-24 14:01 ` Hunt, David
2016-05-19 14:48 ` [dpdk-dev] [PATCH v2 3/3] test: add autotest for external mempool stack handler David Hunt
2016-05-19 15:16 ` [dpdk-dev] v2 mempool: add stack (lifo) mempool handler Hunt, David
2016-06-20 13:08 ` [dpdk-dev] mempool: add stack " David Hunt
2016-06-20 13:08 ` [dpdk-dev] [PATCH v3 1/2] mempool: add stack (lifo) " David Hunt
2016-06-20 13:25 ` Jerin Jacob
2016-06-20 13:54 ` Thomas Monjalon
2016-06-20 13:58 ` Ananyev, Konstantin
2016-06-20 14:22 ` Jerin Jacob
2016-06-20 17:56 ` Ananyev, Konstantin
2016-06-21 3:35 ` Jerin Jacob
2016-06-21 9:28 ` Ananyev, Konstantin
2016-06-21 9:44 ` Olivier Matz
2016-06-21 3:42 ` Jerin Jacob
2016-06-20 13:08 ` [dpdk-dev] [PATCH v3 2/2] test: add autotest for external mempool stack handler David Hunt
2016-06-30 7:41 ` [dpdk-dev] [PATCH v4 0/2] mempool: add stack mempool handler David Hunt
2016-06-30 7:41 ` [dpdk-dev] [PATCH v4 1/2] mempool: add stack (lifo) " David Hunt
2016-06-30 7:41 ` [dpdk-dev] [PATCH v4 2/2] test: migrate custom handler test to stack handler David Hunt
2016-06-30 9:45 ` Thomas Monjalon
2016-06-30 17:36 ` Hunt, David
2016-06-30 17:46 ` Thomas Monjalon
2016-06-30 17:49 ` Hunt, David
2016-06-30 18:05 ` [dpdk-dev] [PATCH v5 0/2] mempool: add stack mempool handler David Hunt
2016-06-30 18:05 ` [dpdk-dev] [PATCH v5 1/2] mempool: add stack (lifo) " David Hunt
2016-06-30 18:05 ` [dpdk-dev] [PATCH v5 2/2] test: migrate custom handler test to stack handler David Hunt
2016-07-01 7:32 ` Olivier MATZ
2016-07-01 7:46 ` [dpdk-dev] [PATCH v6 0/2] mempool: add stack mempool handler David Hunt
2016-07-01 7:46 ` [dpdk-dev] [PATCH v6 1/2] mempool: add stack (lifo) " David Hunt
2016-07-01 7:46 ` [dpdk-dev] [PATCH v6 2/2] test: migrate custom handler test to stack handler David Hunt
2016-07-01 8:18 ` [dpdk-dev] [PATCH v6 0/2] mempool: add stack mempool handler Olivier MATZ
2016-07-01 10:41 ` Thomas Monjalon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160505142835.7df35079@xeon-e3 \
--to=stephen@networkplumber.org \
--cc=david.hunt@intel.com \
--cc=dev@dpdk.org \
--cc=olivier.matz@6wind.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).