From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67]) by dpdk.org (Postfix) with ESMTP id 810ED7CB0 for ; Thu, 27 Dec 2018 16:17:43 +0100 (CET) Received: from lfbn-1-5979-29.w90-110.abo.wanadoo.fr ([90.110.18.29] helo=droids-corp.org) by mail.droids-corp.org with esmtpsa (TLS1.0:RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1gcXRU-0001kO-Rv; Thu, 27 Dec 2018 16:19:26 +0100 Received: by droids-corp.org (sSMTP sendmail emulation); Thu, 27 Dec 2018 16:17:38 +0100 Date: Thu, 27 Dec 2018 16:17:38 +0100 From: Olivier Matz To: "Wiles, Keith" Cc: "dev@dpdk.org" Message-ID: <20181227151738.xo5akqr6kdzmvk7a@platinum> References: <20181216172721.91042-1-keith.wiles@intel.com> <20181216172721.91042-3-keith.wiles@intel.com> <20181227100237.6ich3lvy4ydqtkxq@platinum> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-dev] [PATCH v3 3/3] ring:add ring walk routine X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 27 Dec 2018 15:17:43 -0000 Hi, On Thu, Dec 27, 2018 at 02:52:51PM +0000, Wiles, Keith wrote: > > > > On Dec 27, 2018, at 8:47 AM, Wiles, Keith wrote: > > > > > > > >> On Dec 27, 2018, at 4:02 AM, Olivier Matz 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 > >>> --- > >>> 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. Unlinking the element from the list looks indeed laborious with only te->data, and I agree there is probably no use case for that. Looking at commit cae54ac47ced, it seems there was one, but I cannot find any usage of this feature in git history. So ok, let's keep your version. Thanks, Olivier