Re: [PATCH] event/opdl: fix atomic queue race condition issue
From: Van Haaren, Harry <hidden>
Date: 2018-03-26 12:29:08
From: Ma, Liang J
Sent: Tuesday, March 13, 2018 11:34 AM
To: jerin.jacob@caviumnetworks.com
Cc: dev@dpdk.org; Van Haaren, Harry <redacted>; Jain, Deepak
K [off-list ref]; Geary, John [off-list ref]; Mccarthy,
Peter [off-list ref]
Subject: [PATCH] event/opdl: fix atomic queue race condition issue
If application link one atomic queue to multiple ports,
and each worker core update flow_id, there will have a
chance to hit race condition issue and lead to double processing
same event. This fix solve the problem and eliminate
the race condition issue.
Fixes: 4236ce9bf5bf ("event/opdl: add OPDL ring infrastructure library")General notes - Spaces around & % << >> and other bitwise manipulations (https://dpdk.org/doc/guides/contributing/coding_style.html#operators) - I've noted a few below, but there are more - Usually checkpatch flags these - I'm curious why it didn't in this case It would be nice if we didn't have to rely on __atomic_load_n() and friends, however I don't see a better alternative. Given other DPDK components are also using __atomic_* functions, no objection here. <snip>
quoted hunk ↗ jump to hunk
@@ -520,7 +528,17 @@ opdl_stage_claim_singlethread(struct opdl_stage *s, void*entries, for (j = 0; j < num_entries; j++) { ev = (struct rte_event *)get_slot(t, s->head+j); - if ((ev->flow_id%s->nb_instance) == s->instance_id) {
Spaces around the %
+ + event = __atomic_load_n(&(ev->event), + __ATOMIC_ACQUIRE); + + opa_id = OPDL_OPA_MASK&(event>>OPDL_OPA_OFFSET);
Spaces &
+ flow_id = OPDL_FLOWID_MASK&event;
Spaces &
+
+ if (opa_id >= s->queue_id)
+ continue;
+
+ if ((flow_id%s->nb_instance) == s->instance_id) {Spaces % <snip rest of patch> Will re-review v2. Cheers, -Harry