[PATCH v1] event/dlb2: race condition during dynamic linking

[PATCH v1] event/dlb2: race condition during dynamic linking

[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.

Signed-off-by: Pravin Pathak <pravin.pathak.dev@gmail.com>
---
 drivers/event/dlb2/pf/base/dlb2_osdep.h | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/event/dlb2/pf/base/dlb2_osdep.h b/drivers/event/dlb2/pf/base/dlb2_osdep.h
index 382a49090a..1505563f83 100644
--- a/drivers/event/dlb2/pf/base/dlb2_osdep.h
+++ b/drivers/event/dlb2/pf/base/dlb2_osdep.h
@@ -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;
+	}
 }
 
 /**
-- 
2.53.0

RE: [PATCH v1] event/dlb2: race condition during dynamic linking

[Sarkar, Tirthendu]

On Wed, Oct 21, 2026 at 01:20:41PM -0400, Pravin Pathak wrote:
> dlb2 handles dynamic linking and linking asynchronously if queues
> being linked/unlinked have traffic. [...]

Fixes: 5433956d5185 ("event/dlb2: add eventdev probe")
Cc: stable@dpdk.org

Acked-by: Tirthendu Sarkar <tirthendu.sarkar@intel.com>
Tested-by: Tirthendu Sarkar <tirthendu.sarkar@intel.com>

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

Re: [PATCH v1] event/dlb2: race condition during dynamic linking

[Jerin Jacob]

On Fri, Oct 23, 2026 at 4:14 AM Sarkar, Tirthendu
<tirthendu.sarkar@intel.com> wrote:
>
> On Wed, Oct 21, 2026 at 01:20:41PM -0400, Pravin Pathak wrote:
> > dlb2 handles dynamic linking and linking asynchronously if queues
> > being linked/unlinked have traffic. [...]
>
> Fixes: 5433956d5185 ("event/dlb2: add eventdev probe")
> Cc: stable@dpdk.org
>
> Acked-by: Tirthendu Sarkar <tirthendu.sarkar@intel.com>
> Tested-by: Tirthendu Sarkar <tirthendu.sarkar@intel.com>
>
> 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.


# Fix the author to Pravin Pathak <pravin.pathak.dev@gmail.com> - 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 <pravin.pathak.dev@gmail.com>
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.

    Fixes: 5433956d5185 ("event/dlb2: add eventdev probe")
    Cc: stable@dpdk.org

    Signed-off-by: Pravin Pathak <pravin.pathak.dev@gmail.com>
    Acked-by: Tirthendu Sarkar <tirthendu.sarkar@intel.com>
    Acked-by: Bruce Richardson <bruce.richardson@intel.com>
    Tested-by: Tirthendu Sarkar <tirthendu.sarkar@intel.com>



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.


>
>
> Regards
> Tirthendu

Re: [PATCH v1] event/dlb2: race condition during dynamic linking

[Bruce Richardson]

On Wed, Oct 21, 2026 at 01:20:41PM -0400, Pravin Pathak 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 <pravin.pathak.dev@gmail.com>
> ---
>  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.

Acked-by: Bruce Richardson <bruce.richardson@intel.com>


> diff --git a/drivers/event/dlb2/pf/base/dlb2_osdep.h b/drivers/event/dlb2/pf/base/dlb2_osdep.h
> index 382a49090a..1505563f83 100644
> --- a/drivers/event/dlb2/pf/base/dlb2_osdep.h
> +++ b/drivers/event/dlb2/pf/base/dlb2_osdep.h
> @@ -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)".

>  		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;
> +	}

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.

>  }
>  
>  /**
> -- 
> 2.53.0
> 

Re: [PATCH v1] event/dlb2: race condition during dynamic linking

[Stephen Hemminger]

On Wed, 21 Oct 2026 13:20:41 -0400
Pravin Pathak <pravinpathakdev@gmail.com> 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 <pravin.pathak.dev@gmail.com>
> ---

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.