Re: [dpdk-dev] [PATCH v3 3/3] ring:add ring walk routine

[Olivier Matz <olivier.matz@6wind.com>]

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.


> 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