|WARNING| [v2] examples/vhost: fix hang on exit

dpdklab at iol.unh.edu dpdklab at iol.unh.edu
Thu Oct 29 19:30:04 CET 2026


Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/172244

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-10-29

# DPDK Patch Review

## Errors

None identified.

## Warnings

**1. Missing `rte_mb()` after atomic store may cause delayed worker notification**

Line: `rte_atomic_store_explicit(&stop_workers, true, rte_memory_order_relaxed);`

The atomic store uses `relaxed` ordering, which provides no ordering guarantees.
After unregistering the vhost drivers, the main thread sets `stop_workers = true`
and immediately calls `rte_eal_mp_wait_lcore()`, expecting workers to exit.
However, with `relaxed` ordering, the store may not be visible to other cores immediately,
causing workers to continue spinning on the old value.

The fix is to use `release` ordering on the store:
```c
rte_atomic_store_explicit(&stop_workers, true, rte_memory_order_release);
```

And correspondingly, the load in the worker loop should use `acquire` ordering:
```c
while (!rte_atomic_load_explicit(&stop_workers, rte_memory_order_acquire)) {
```

This ensures the store is visible to workers before `mp_wait_lcore()` begins waiting.

**Context**: The worker loop currently uses `relaxed`:
```c
while (!rte_atomic_load_explicit(&stop_workers, rte_memory_order_relaxed)) {
```

While this may work on x86 due to total-store-order, it is not portable
and could cause indefinite hangs on architectures with weaker memory models (ARM, POWER).

**2. Potential worker exit before unregister**

Lines: unregister_drivers() call followed by atomic store

The patch fixes the original deadlock where `destroy_device()` waited for workers
that had already exited. However, the new sequence introduces a different race:

1. Main thread calls `sigwait(&set, &sig)` and unblocks
2. Main thread unregisters drivers - `destroy_device()` may be called
3. Main thread sets `stop_workers = true`
4. If a device destroy happens between steps 2-3, and that destroy callback
   spins waiting for a worker ack, the worker hasn't been told to stop yet

The safer ordering is to set `stop_workers` **before** unregistering drivers:
```c
/* Tell worker threads to prepare for shutdown */
rte_atomic_store_explicit(&stop_workers, true, rte_memory_order_release);

/* Give workers a chance to observe the flag */
rte_delay_ms(100);

/* Unregister vhost driver - destroy callbacks may now safely complete */
unregister_drivers(nb_sockets);

/* Wait for workers to fully exit */
rte_eal_mp_wait_lcore();
```

This ensures workers are already in their exit path when `destroy_device()` runs,
avoiding the hang. The delay is optional but increases robustness on loaded systems.

## Info

**Signal handling improvement acknowledged**

The conversion from `sig_atomic_t quit` with a signal handler
to `sigwait()` with blocked signals is the correct modern pattern.
`sigwait()` avoids async-signal-safety issues and allows the main thread
to cleanly orchestrate shutdown.

Review-Result: WARNING


More information about the test-report mailing list