From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lazaros Koromilas Subject: [PATCH v2] ring: check for zero objects mc dequeue / mp enqueue Date: Thu, 17 Mar 2016 17:49:43 +0200 Message-ID: <1458229783-15547-1-git-send-email-l@nofutznetworks.com> To: dev@dpdk.org Return-path: Received: from mail-wm0-f42.google.com (mail-wm0-f42.google.com [74.125.82.42]) by dpdk.org (Postfix) with ESMTP id 8045EFFA for ; Thu, 17 Mar 2016 16:49:46 +0100 (CET) Received: by mail-wm0-f42.google.com with SMTP id l124so90196075wmf.1 for ; Thu, 17 Mar 2016 08:49:46 -0700 (PDT) Received: from lap-3.nofutz.com (ppp089210158019.access.hol.gr. [89.210.158.19]) by smtp.gmail.com with ESMTPSA id v82sm8693109wmv.6.2016.03.17.08.49.45 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 17 Mar 2016 08:49:45 -0700 (PDT) List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Issuing a zero objects dequeue with a single consumer has no effect. Doing so with multiple consumers, can get more than one thread to succeed the compare-and-set operation and observe starvation or even deadlock in the while loop that checks for preceding dequeues. The problematic piece of code when n = 0: cons_next = cons_head + n; success = rte_atomic32_cmpset(&r->cons.head, cons_head, cons_next); The same is possible on the enqueue path. Signed-off-by: Lazaros Koromilas --- lib/librte_ring/rte_ring.h | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lib/librte_ring/rte_ring.h b/lib/librte_ring/rte_ring.h index 943c97c..eb45e41 100644 --- a/lib/librte_ring/rte_ring.h +++ b/lib/librte_ring/rte_ring.h @@ -431,6 +431,11 @@ __rte_ring_mp_do_enqueue(struct rte_ring *r, void * const *obj_table, uint32_t mask = r->prod.mask; int ret; + /* Avoid the unnecessary cmpset operation below, which is also + * potentially harmful when n equals 0. */ + if (n == 0) + return 0; + /* move prod.head atomically */ do { /* Reset n to the initial burst count */ @@ -618,6 +623,11 @@ __rte_ring_mc_do_dequeue(struct rte_ring *r, void **obj_table, unsigned i, rep = 0; uint32_t mask = r->prod.mask; + /* Avoid the unnecessary cmpset operation below, which is also + * potentially harmful when n equals 0. */ + if (n == 0) + return 0; + /* move cons.head atomically */ do { /* Restore n as it may change every loop */ -- 1.9.1