event/dlb2: race condition during dynamic linking
PATCH 1/1 event/dlb2: race condition during dynamic linking
dlb2 handles dynamic linking and linking asynchronously if queues being linked/unlinked have traffic. It starts a thread if not already started to handle the process. This change avoids the race condition in thread creation where sometimes the thread starts immediately, descheduling the calling thread and finishes the job and exits, and then caller sets worker launched true when the thread is not running.
--- drivers/event/dlb2/pf/base/dlb2_osdep.h | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
drivers/event/dlb2/pf/base/dlb2_osdep.h +5 -3
@@ -208,14 +208,16 @@ static inline void os_schedule_work(struct dlb2_hw *hw) dlb2_dev = container_of(hw, struct dlb2_dev, hw); + dlb2_dev->worker_launched = true; + ret =rte_thread_create_internal_control (&complete_queue_map_unmap_thread,"dlb-qunmap" , dlb2_complete_queue_map_unmap, dlb2_dev); -if (ret) +if (ret) {DLB2_ERR (dlb2_dev,"Could not create queue complete map/unmap thread, err=%d\n" , ret); -else - dlb2_dev->worker_launched = true; + dlb2_dev->worker_launched = false; + } }/**
Signature · 1 line
2.53.0
Quoted 2 lines · Pravin Pathak
dlb2 handles dynamic linking and linking asynchronously if queues being linked/unlinked have traffic. [...]
Thanks Pravin. Confirmed on DLB 2.5: without this patch, runtime unlink/relink under traffic stalls on all ports once the worker exits before worker_launched is set.
Jerin, please add the Fixes/Cc tags when applying.
Regards
Tirthendu
Quoted 8 lines · Tirthendu
Quoted 2 lines · Pravin Pathak
dlb2 handles dynamic linking and linking asynchronously if queues being linked/unlinked have traffic. [...]
Fixes 5433956d5185 ("event/dlb2: add eventdev probe")
Quoted 8 lines
Acked-by Tirthendu Sarkar <[email protected]>Tested-by Tirthendu Sarkar <[email protected]>Thanks Pravin. Confirmed on DLB 2.5: without this patch, runtime unlink/relink under traffic stalls on all ports once the worker exits before
worker_launchedis set.Jerin, please add the Fixes/Cc tags when applying.
# Fix the author to Pravin Pathak <[email protected]> - Use .mailmap email
# Changed if (ret) to if (ret != 0)
# Updated as
commit 2631abb3e2cbad9beb3d5710fd9fd482b18b6f25 (HEAD -> for-main, origin/for-main, origin/HEAD)
Author: Pravin Pathak <[email protected]> Date: Wed Oct 21 13:20:41 2026 -0400
event/dlb2: fix race condition during dynamic linking
dlb2 handles dynamic linking and linking asynchronously if queues being linked/unlinked have traffic. It starts a thread if not already started to handle the process. This change avoids the race condition in thread creation where sometimes the thread starts immediately, descheduling the calling thread and finishes the job and exits, and then caller sets worker launched true when the thread is not running.
Applied to dpdk-next-eventdev/for-main. Thanks
@Thomas Monjalon Please pull this patch for event-dev tree, if you are already pulled the tree.
Quoted 4 lines
Regards
Tirthendu
Quoted 7 lines · Pravin Pathak
dlb2 handles dynamic linking and linking asynchronously if queues being linked/unlinked have traffic. It starts a thread if not already started to handle the process. This change avoids the race condition in thread creation where sometimes the thread starts immediately, descheduling the calling thread and finishes the job and exits, and then caller sets worker launched true when the thread is not running.
Quoted 5 lines
Signed-off-by Pravin Pathak <[email protected]>--- drivers/event/dlb2/pf/base/dlb2_osdep.h | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
Fix makes sense to me. Note some comment inline below.
Quoted 5 lines
drivers/event/dlb2/pf/base/dlb2_osdep.h +0
-0
drivers/event/dlb2/pf/base/dlb2_osdep.h static inline void os_schedule_work(struct dlb2_hw *hw) · line 215
Quoted 9 lines
drivers/event/dlb2/pf/base/dlb2_osdep.h +3
-1@@ -208,14 +208,16 @@ static inline void os_schedule_work(struct dlb2_hw *hw) dlb2_dev = container_of(hw, struct dlb2_dev, hw); + dlb2_dev->worker_launched = true; + ret =rte_thread_create_internal_control (&complete_queue_map_unmap_thread,"dlb-qunmap" , dlb2_complete_queue_map_unmap, dlb2_dev); -if (ret) +if (ret) {
DPDK coding style recommends using explicit comparisons for clarity, so this should really be "if (ret != 0)" or "if (ret < 0)".
drivers/event/dlb2/pf/base/dlb2_osdep.h static inline void os_schedule_work(struct dlb2_hw *hw) · line 220
Quoted 7 lines
drivers/event/dlb2/pf/base/dlb2_osdep.h +2
-2DLB2_ERR (dlb2_dev,"Could not create queue complete map/unmap thread, err=%d\n" , ret); - else - dlb2_dev->worker_launched = true; + dlb2_dev->worker_launched = false; + }
I notice that worker_launched is neither volatile nor atomic, so is technically not suitable for communicating between threads - though in practice it's presumably working fine here. Maybe look to make it atomic in a future patch.
Quoted 6 lines
drivers/event/dlb2/pf/base/dlb2_osdep.h +0
-0}/** Signature · 1 line
2.53.0
Quoted 9 lines · On Wed, 21 Oct 2026 13:20:41 -0400 Pravin Pathak <[email protected]>…
dlb2 handles dynamic linking and linking asynchronously if queues being linked/unlinked have traffic. It starts a thread if not already started to handle the process. This change avoids the race condition in thread creation where sometimes the thread starts immediately, descheduling the calling thread and finishes the job and exits, and then caller sets worker launched true when the thread is not running.
Signed-off-by Pravin Pathak <[email protected]>---
Ran a more detailed AI review on this.
Subject: Re: [PATCH] event/dlb2: fix race condition during dynamic linking
The fix is correct and worth having in stable. Some notes on what it does and does not cover, since two related problems remain in this code.
What the patch fixes
--------------------
os_schedule_work() set worker_launched = true after rte_thread_create_internal_control() returned, but the worker thread (dlb2_complete_queue_map_unmap()) clears that flag as its last act before exiting. If the new thread ran to completion before the creator was scheduled again, the creator stamped true over the worker's false.
The flag is then stuck true for the life of the device:
- os_worker_active() always reports an active worker
- all three call sites in dlb2_resource.c (2336, 2370, 6037) are guarded by if (!os_worker_active(hw)), so no worker is ever launched again
- deferred map/unmap work is stranded, leaving ports in DLB2_QUEUE_MAP_IN_PROG / UNMAP_IN_PROG indefinitely
Setting the flag before thread creation closes that window: creation is a synchronization point, so the worker is guaranteed to observe true and its own clear is the last write. Resetting to false on the create-failure path is right.
Remaining issues
----------------
Both of these predate the patch, so they are follow-up material rather than objections, but they are in the same few lines and should probably be cleaned up while the context is fresh.
1. Missed wakeup. resource_mutex is only ever taken inside the worker loop:
drivers/event/dlb2/pf/dlb2_main.h:46 rte_spinlock_t resource_mutex
drivers/event/dlb2/pf/base/dlb2_osdep.h:172,178,190 worker only
drivers/event/dlb2/pf/dlb2_main.c:34 rte_spinlock_init()
No caller holds it across the check-and-schedule pair, so for one control thread against the worker:
- worker's final iteration scans, finds nothing pending
- control thread marks a new unmap in progress
- control thread reads worker_launched as true, skips os_schedule_work()
- worker sets worker_launched = false and exits
The work is stranded until the next link/unlink happens to launch a new worker. This needs no MT-unsafe API use to hit: eventdev link/unlink is not MT-safe by contract, and a single conforming control thread racing the worker is enough.
The kernel driver this base code was ported from holds resource_mutex around the check and the schedule; the DPDK port dropped that. Before this patch the resulting failure was permanent, now it is transient.
2. worker_launched is a plain bool with mixed access. The worker writes it under the spinlock, callers read and write it with no synchronization, which is a C11 data race. The practical impact is small since the worst case is a duplicate worker and two workers looping under the same mutex are safe, but it should be RTE_ATOMIC(bool) with release/acquire.