[dpdk-dev] [PATCH] eventdev: fix incorrect MP/MC tail updates in rte_event_ring
Mattias Rönnblom
hofors at lysator.liu.se
Tue Apr 3 10:14:46 CEST 2018
On 2018-03-29 14:38, Van Haaren, Harry wrote:
>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Mattias Rönnblom
>> Sent: Wednesday, March 28, 2018 10:17 AM
>> To: dev at dpdk.org
>> Cc: Mattias Rönnblom <hofors at lysator.liu.se>
>> 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 <hofors at lysator.liu.se>
>> ---
>
> 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 at 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
More information about the dev
mailing list