From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.lysator.liu.se (mail.lysator.liu.se [130.236.254.3]) by dpdk.org (Postfix) with ESMTP id 897011B61A for ; Tue, 3 Apr 2018 10:14:50 +0200 (CEST) Received: from mail.lysator.liu.se (localhost [127.0.0.1]) by mail.lysator.liu.se (Postfix) with ESMTP id D890540036 for ; Tue, 3 Apr 2018 10:14:49 +0200 (CEST) Received: by mail.lysator.liu.se (Postfix, from userid 1004) id C59AB40033; Tue, 3 Apr 2018 10:14:49 +0200 (CEST) X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on bernadotte.lysator.liu.se X-Spam-Level: X-Spam-Status: No, score=-1.0 required=5.0 tests=ALL_TRUSTED,AWL autolearn=disabled version=3.4.1 X-Spam-Score: -1.0 Received: from [192.168.1.59] (m176-71-126-62.cust.tele2.se [176.71.126.62]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.lysator.liu.se (Postfix) with ESMTPSA id D13E240032; Tue, 3 Apr 2018 10:14:46 +0200 (CEST) To: "Van Haaren, Harry" , "dev@dpdk.org" References: <1522228611-4838-1-git-send-email-hofors@lysator.liu.se> From: =?UTF-8?Q?Mattias_R=c3=b6nnblom?= Message-ID: <6976944b-b835-925a-15dd-833f583104a5@lysator.liu.se> Date: Tue, 3 Apr 2018 10:14:46 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-Virus-Scanned: ClamAV using ClamSMTP Subject: Re: [dpdk-dev] [PATCH] eventdev: fix incorrect MP/MC tail updates in rte_event_ring 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: Tue, 03 Apr 2018 08:14:50 -0000 On 2018-03-29 14:38, Van Haaren, Harry wrote: >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Mattias Rönnblom >> Sent: Wednesday, March 28, 2018 10:17 AM >> To: dev@dpdk.org >> Cc: Mattias Rönnblom >> Subject: [dpdk-dev] [PATCH] eventdev: fix incorrect MP/MC tail updates in >> rte_event_ring >> >> rte_event_ring enqueue and dequeue tail updates were hardcoded for a >> SC/SP configuration. >> >> Signed-off-by: Mattias Rönnblom >> --- > > Hi Mattias, > > > Thanks for your patch, a few notes; > > 1) > You can CC the maintainer of a section - the event rings falls under > the eventdev library, and Jerin Jacob is the maintainer. > > > 2) > In DPDK we note patches that are "Fixes", > so we can track what commit they fix. Fixes patches are > also often candidates for backporting. > > Details on getting the fixes line here: > https://dpdk.org/doc/guides/contributing/patches.html#commit-messages-body > > For this patch, the following is the fixline: > > Fixes: dc39e2f359b5 ("eventdev: add ring structure for events") > Cc: bruce.richardson@intel.com > > Thanks for your help. I'll resubmit and try to get the administrative details correct. >> lib/librte_eventdev/rte_event_ring.h | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/lib/librte_eventdev/rte_event_ring.h >> b/lib/librte_eventdev/rte_event_ring.h >> index 29d4228..07b4559 100644 >> --- a/lib/librte_eventdev/rte_event_ring.h >> +++ b/lib/librte_eventdev/rte_event_ring.h >> @@ -99,7 +99,7 @@ rte_event_ring_enqueue_burst(struct rte_event_ring *r, >> >> ENQUEUE_PTRS(&r->r, &r[1], prod_head, events, n, struct rte_event); >> >> - update_tail(&r->r.prod, prod_head, prod_next, 1, 1); >> + update_tail(&r->r.prod, prod_head, prod_next, 1, r->r.prod.single); >> end: >> if (free_space != NULL) >> *free_space = free_entries - n; >> @@ -140,7 +140,7 @@ rte_event_ring_dequeue_burst(struct rte_event_ring *r, >> >> DEQUEUE_PTRS(&r->r, &r[1], cons_head, events, n, struct rte_event); >> >> - update_tail(&r->r.cons, cons_head, cons_next, 1, 0); >> + update_tail(&r->r.cons, cons_head, cons_next, 1, r->r.cons.single); > > > The signature of update_tail() is as follows: > > static __rte_always_inline void > update_tail(struct rte_ring_headtail *ht, uint32_t old_val, uint32_t new_val, > uint32_t single, uint32_t enqueue) > > It seems to be that the 2nd last parameter sets the single/multi producer? > Yes, you are right. I ported this fix from my 17.08 tree, and update_tail() has changed since then - something I failed to notice. I reproduced the issue on 17.08, and even on a machine with a strong memory model, the event ring doesn't work correctly in a MP/SC configuration. With the fix, it at least passes my tests on 17.08. I didn't try MP/MC, and I also didn't try anything beyond building on HEAD. Regards, Mattias