From: "Wiles, Keith" <keith.wiles@intel.com>
To: Olivier Matz <olivier.matz@6wind.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v3 3/3] ring:add ring walk routine
Date: Thu, 27 Dec 2018 14:52:51 +0000 [thread overview]
Message-ID: <AE7192FA-2CD4-4BBF-9308-6475C397B353@intel.com> (raw)
In-Reply-To: <ED0EA2F8-CAD9-48FE-BC29-D8FAB31599B9@intel.com>
> On Dec 27, 2018, at 8:47 AM, Wiles, Keith <keith.wiles@intel.com> wrote:
>
>
>
>> On Dec 27, 2018, at 4:02 AM, Olivier Matz <olivier.matz@6wind.com> wrote:
>>
>> Hi,
>>
>> On Sun, Dec 16, 2018 at 11:27:21AM -0600, Keith Wiles wrote:
>>> Add a ring walk routine for debugging and DFS.
>>>
>>> Signed-off-by: Keith Wiles <keith.wiles@intel.com>
>>> ---
>>> V3
>>> Fix checkpatch warnings adding a commit message.
>>> Must be using a different checkpatch then on my Ubuntu 18.04 system
>>> V2
>>> Fix checkpatch warnings.
>>>
>>> lib/librte_ring/rte_ring.c | 20 ++++++++++++++++++++
>>> lib/librte_ring/rte_ring.h | 14 ++++++++++++++
>>> lib/librte_ring/rte_ring_version.map | 7 +++++++
>>> 3 files changed, 41 insertions(+)
>>>
>>> diff --git a/lib/librte_ring/rte_ring.c b/lib/librte_ring/rte_ring.c
>>> index d215acecc..fb5819e4b 100644
>>> --- a/lib/librte_ring/rte_ring.c
>>> +++ b/lib/librte_ring/rte_ring.c
>>> @@ -280,3 +280,23 @@ rte_ring_lookup(const char *name)
>>>
>>> return r;
>>> }
>>> +
>>> +void
>>> +rte_ring_walk(void (*func)(struct rte_ring *r, void *arg), void *arg)
>>> +{
>>> + const struct rte_tailq_entry *te;
>>> + struct rte_ring_list *ring_list;
>>> +
>>> + if (!func)
>>> + return;
>>> +
>>> + ring_list = RTE_TAILQ_CAST(rte_ring_tailq.head, rte_ring_list);
>>> +
>>> + rte_rwlock_read_lock(RTE_EAL_TAILQ_RWLOCK);
>>> +
>>> + TAILQ_FOREACH(te, ring_list, next) {
>>> + func((struct rte_ring *) te->data, arg);
>>> + }
>>> +
>>> + rte_rwlock_read_unlock(RTE_EAL_TAILQ_RWLOCK);
>>> +}
>>
>> In mempool, a FOREACH_SAFE() macro is using starting from this commit:
>> cae54ac47ced ("mempool: fix unsafe removal from list by callback")
>>
>> Maybe the same should be done for the ring.
>
> I am not removing or modifying the ring tailq list here and I have the lock already, why do I need to use _SAFE macro?
OK, I do see a possible case. If the function freed the node, but it would mean it would have to grab the lock and walk the list to free the te value and unlink it from the list. The function does not get the ’te’ pointer only the data. To free it they would have to grab the lock.
>
>>
>>
>>> diff --git a/lib/librte_ring/rte_ring.h b/lib/librte_ring/rte_ring.h
>>> index af5444a9f..b9391a655 100644
>>> --- a/lib/librte_ring/rte_ring.h
>>> +++ b/lib/librte_ring/rte_ring.h
>>> @@ -769,6 +769,20 @@ rte_ring_get_capacity(const struct rte_ring *r)
>>> */
>>> void rte_ring_list_dump(FILE *f);
>>>
>>> +/**
>>> + * Walk the list of ring entries and call the function provided
>>> + *
>>> + * @param func
>>> + * The function to call for each ring entry using the following prototype
>>> + * void (*func)(struct rte_ring *r, void *arg)
>>> + * @param arg
>>> + * argument for the call to function
>>> + * @return
>>> + * None.
>>> + */
>>
>> I don't think we need to duplicate the prototype in the comment. Please
>> add the dots at the end of the sentences, and remove @return.
>>
>>
>> Thanks,
>> Olivier
>
> Regards,
> Keith
Regards,
Keith
next prev parent reply other threads:[~2018-12-27 14:52 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-12-16 17:27 [dpdk-dev] [PATCH v2 1/3] dfs:add FUSE based filesystem for DPDK Keith Wiles
2018-12-16 17:27 ` [dpdk-dev] [PATCH v3 2/3] eal:add tailq walk routine Keith Wiles
2018-12-16 17:27 ` [dpdk-dev] [PATCH v3 3/3] ring:add ring " Keith Wiles
2018-12-27 10:02 ` Olivier Matz
2018-12-27 14:47 ` Wiles, Keith
2018-12-27 14:52 ` Wiles, Keith [this message]
2018-12-27 15:17 ` Olivier Matz
2018-12-27 15:40 ` Wiles, Keith
2018-12-27 17:24 ` [dpdk-dev] [PATCH v4] " Keith Wiles
2018-12-27 17:23 ` [dpdk-dev] [PATCH v4] eal:add tailq " Keith Wiles
2018-12-27 17:41 ` [dpdk-dev] [PATCH v5] ring:add ring " Keith Wiles
2018-12-27 17:44 ` [dpdk-dev] [PATCH v5] eal:add tailq " Keith Wiles
2018-12-27 18:16 ` [dpdk-dev] [PATCH v6] " Keith Wiles
2018-12-27 18:17 ` [dpdk-dev] [PATCH v5] ring:add ring " Keith Wiles
2018-12-27 18:19 ` [dpdk-dev] [PATCH v6] " Keith Wiles
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=AE7192FA-2CD4-4BBF-9308-6475C397B353@intel.com \
--to=keith.wiles@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).