On Wed, 21 Oct 2026 13:20:41 -0400 Pravin Pathak <[email protected]> wrote:
> 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.
