From: Olivier MATZ <olivier.matz@6wind.com>
To: "Hunt, David" <david.hunt@intel.com>, dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v2 1/3] mempool: add stack (lifo) mempool handler
Date: Wed, 29 Jun 2016 16:31:28 +0200 [thread overview]
Message-ID: <5773DBC0.5050709@6wind.com> (raw)
In-Reply-To: <5767E8AB.1040109@intel.com>
Hi Dave,
On 06/20/2016 02:59 PM, Hunt, David wrote:
> Hi Olivier,
>
> On 20/6/2016 9:17 AM, Olivier Matz wrote:
>> Hi David,
>>
>> On 06/17/2016 04:18 PM, Hunt, David wrote:
>>>> After reading it, I realize that it's nearly exactly the same code than
>>>> in "app/test: test external mempool handler".
>>>> http://patchwork.dpdk.org/dev/patchwork/patch/12896/
>>>>
>>>> We should drop one of them. If this stack handler is really useful for
>>>> a performance use-case, it could go in librte_mempool. At the first
>>>> read, the code looks like a demo example : it uses a simple spinlock
>>>> for
>>>> concurrent accesses to the common pool. Maybe the mempool cache hides
>>>> this cost, in this case we could also consider removing the use of the
>>>> rte_ring.
>>> While I agree that the code is similar, the handler in the test is a
>>> ring based handler,
>>> where as this patch adds an array based handler.
>> Not sure I'm getting what you are saying. Do you mean stack instead
>> of array?
>
> Yes, apologies, stack.
>
>> Actually, both are stacks when talking about bulks of objects. If we
>> consider each objects one by one, that's true the order will differ.
>> But as discussed in [1], the cache code already reverses the order of
>> objects when doing a mempool_get(). I'd say the reversing in cache code
>> is not really needed (only the order of object bulks should remain the
>> same). A rte_memcpy() looks to be faster, but it would require to do
>> some real-life tests to validate or unvalidate this theory.
>>
>> So to conclude, I still think both code in app/test and lib/mempool are
>> quite similar, and only one of them should be kept.
>>
>> [1] http://www.dpdk.org/ml/archives/dev/2016-May/039873.html
>
> OK, so we will probably remove the test app portion in the future is if
> is not needed,
> and if we apply the stack handler proposed in this patch set.
Back on this thread. Maybe I misunderstood what you were saying
here (because I see this comment is not addressed in v3).
I don't think we should add similar code at two different places
and then remove it later in another patchset. I feel it's better to
have only one instance of the stack handler, either in test, or
in librte_mempool.
If you plan to do a v4, I think this is something that could go in
16.07-rc2.
Regards,
Olivier
next prev parent reply other threads:[~2016-06-29 14:31 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-05-05 18:29 [dpdk-dev] [PATCH 0/2] mempool: add stack (fifo) " David Hunt
2016-05-05 18:29 ` [dpdk-dev] [PATCH 1/2] " David Hunt
2016-05-05 21:28 ` Stephen Hemminger
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 [this message]
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=5773DBC0.5050709@6wind.com \
--to=olivier.matz@6wind.com \
--cc=david.hunt@intel.com \
--cc=dev@dpdk.org \
/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).