From mboxrd@z Thu Jan 1 00:00:00 1970 From: Olivier MATZ Subject: Re: rte_ring features in use (or not) Date: Wed, 25 Jan 2017 14:20:52 +0100 Message-ID: <20170125142052.7989e0ec@glumotte.dev.6wind.com> References: <20170125121456.GA24344@bricha3-MOBL3.ger.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: dev@dpdk.org To: Bruce Richardson Return-path: Received: from mail-wm0-f45.google.com (mail-wm0-f45.google.com [74.125.82.45]) by dpdk.org (Postfix) with ESMTP id ECE58DE5 for ; Wed, 25 Jan 2017 14:20:59 +0100 (CET) Received: by mail-wm0-f45.google.com with SMTP id c85so26032172wmi.1 for ; Wed, 25 Jan 2017 05:20:59 -0800 (PST) In-Reply-To: <20170125121456.GA24344@bricha3-MOBL3.ger.corp.intel.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Wed, 25 Jan 2017 12:14:56 +0000, Bruce Richardson wrote: > Hi all, > > while looking at the rte_ring code, I'm wondering if we can simplify > that a bit by removing some of the code it in that may not be used. > Specifically: > > * Does anyone use the NIC stats functionality for debugging? I've > certainly never seen it used, and it's presence makes the rest less > readable. Can it be dropped? What do you call NIC stats? The stats that are enabled with RTE_LIBRTE_RING_DEBUG? If yes, I was recently thinking almost the same about mempool stats. The need to enable stats at compilation makes them less usable. On the other hand, I feel the mempool/ring stats may be useful, for instance to check if mbufs are used from mempool cache, and not from common pool. For mempool, my conclusion was: - Enabling stats (debug) changes the ABI, because it adds a field in the structure, this is bad - enabling stats is not the same than enabling debug, we should have 2 different ifdefs - if statistics don't cost a lot, they should be enabled by default, because it's a good debug tool (ex: have a stats for each access to common pool) For the ring, in my opinion, the stats could be fully removed. > * RTE_RING_PAUSE_REP_COUNT is set to be disabled at build time, and > so does anyone actually use this? Can it be dropped? This option looks like a hack to use the ring in conditions where it should no be used (preemptable threads). And having a compile-time option for this kind of stuff is not in vogue ;) > * Who uses the watermarks feature as is? I know we have a sample app > that uses it, but there are better ways I think to achieve the same > goal while simplifying the ring implementation. Rather than have a > set watermark on enqueue, have both enqueue and dequeue functions > return the number of free or used slots available in the ring (in > case of enqueue, how many free there are, in case of dequeue, how > many items are available). Easier to implement and far more useful to > the app. +1