diff --git a/doc/design.md b/doc/design.md index 8055ce2eb..f60a05d26 100644 --- a/doc/design.md +++ b/doc/design.md @@ -342,7 +342,27 @@ Sample application code can be find in [tx_st20_pipeline_sample.c](../app/sample By default, the `st20p_tx_get_frame` and `st20p_rx_get_frame` functions operate in non-blocking mode, which means the function call will immediately return `NULL` if no frame is available. To switch to blocking mode, where the call will wait until a frame is ready for application use or one second timeout occurs, you must enable the `ST20P_TX_FLAG_BLOCK_GET` or `ST20P_RX_FLAG_BLOCK_GET` flag respectively during the session creation stage, and application can use `st20p_tx_wake_block`/`st20p_rx_wake_block` to wake up the waiting directly. -#### 6.3.1. Ancillary (ST40) pipeline +#### 6.3.1. Threading model and lock-free assumptions + +Each pipeline framebuffer carries a single `_Atomic` status field, and every stage transition (for example `FREE`→`IN_USER`, `READY`→`CONVERTED`, `IN_TRANSMITTING`→`FREE`) is performed with a C11 atomic load/store or compare-exchange rather than a mutex. This lock-free protocol is correct only under the following assumptions, which the get/put API contract implicitly relies on: + +- **One application thread per session and per direction.** For a given session, `st*p_tx_get_frame`/`st*p_tx_put_frame` + (and, on the receive side, `st*p_rx_get_frame`/`st*p_rx_put_frame`) must be called from the same thread, and each call + must be synchronous — it must return before the next get/put is issued. The atomic status field prevents state + corruption and double-claim between the application thread and the data-plane thread, but it does **not** make these + calls re-entrant or safe to invoke concurrently from multiple application threads. The producer/consumer cursor hints + (`framebuff_producer_idx`/`framebuff_consumer_idx`) and the get→process→put handoff assume this single-threaded, + sequential usage. +- **Synchronous lower-layer callbacks.** The transport-layer callbacks driven from the data-plane tasklet — for example + `frame_ready`, `packet_convert`, `frame_done`, and `query_ext_frame` — are assumed to run synchronously and to + completion within their single owning tasklet thread. Under this assumption each pipeline stage has exactly one + producer thread and one consumer thread, so the atomic status field (with acquire/release ordering) is the only + cross-thread synchronization required. If any such callback were deferred to another thread or re-entered + concurrently, the acquire/release ordering would no longer be sufficient and the lock-free protocol would break. + +In short: the atomic status field synchronizes exactly one data-plane (tasklet) thread against one application thread. Multi-threaded or asynchronous use of the get/put APIs, or asynchronous lower-layer callbacks, are outside the model and are not supported. + +#### 6.3.2. Ancillary (ST40) pipeline The same pipeline abstraction is now available for ancillary-only flows through `st40_pipeline_api.h`. It keeps the zero-copy RFC 8331 payload path but hides scheduler registration, frame queues, and pacing details behind the familiar get/put contract so applications only need to implement: diff --git a/lib/src/st2110/pipeline/st20_pipeline_rx.c b/lib/src/st2110/pipeline/st20_pipeline_rx.c index 571a26610..27113456a 100644 --- a/lib/src/st2110/pipeline/st20_pipeline_rx.c +++ b/lib/src/st2110/pipeline/st20_pipeline_rx.c @@ -53,7 +53,7 @@ static struct st20p_rx_frame* rx_st20p_next_available( /* check ready frame from idx_start */ while (1) { framebuff = &ctx->framebuffs[idx]; - if (desired == framebuff->stat) { + if (desired == atomic_load_explicit(&framebuff->stat, memory_order_acquire)) { /* find one desired */ return framebuff; } @@ -68,6 +68,27 @@ static struct st20p_rx_frame* rx_st20p_next_available( return NULL; } +/* Scan from idx_start for a framebuff in state `desired` and atomically claim + * it by transitioning it to `claimed`. A concurrent thread can win the race on + * the scanned candidate between the scan and the claim; on a lost race this + * keeps scanning instead of giving up, so the caller only sees NULL once every + * slot has genuinely been checked and found unavailable. */ +static struct st20p_rx_frame* rx_st20p_claim_available( + struct st20p_rx_ctx* ctx, uint16_t idx_start, enum st20p_rx_frame_status desired, + enum st20p_rx_frame_status claimed) { + struct st20p_rx_frame* framebuff; + + while ((framebuff = rx_st20p_next_available(ctx, idx_start, desired))) { + uint32_t expected = desired; + if (atomic_compare_exchange_strong_explicit(&framebuff->stat, &expected, claimed, + memory_order_acq_rel, + memory_order_relaxed)) + return framebuff; + } + + return NULL; +} + static int rx_st20p_packet_convert(void* priv, void* frame, struct st20_rx_uframe_pg_meta* meta) { struct st20p_rx_ctx* ctx = priv; @@ -76,13 +97,13 @@ static int rx_st20p_packet_convert(void* priv, void* frame, struct st20_rfc4175_422_10_pg2_be* src = meta->payload; MTL_MAY_UNUSED(frame); - mt_pthread_mutex_lock(&ctx->lock); if (meta->row_number == 0 && meta->row_offset == 0) { /* first packet of frame */ framebuff = rx_st20p_next_available(ctx, ctx->framebuff_producer_idx, ST20P_RX_FRAME_FREE); if (framebuff) { - framebuff->stat = ST20P_RX_FRAME_IN_CONVERTING; + atomic_store_explicit(&framebuff->stat, ST20P_RX_FRAME_IN_CONVERTING, + memory_order_release); framebuff->dst.timestamp = meta->timestamp; } } else { @@ -95,7 +116,6 @@ static int rx_st20p_packet_convert(void* priv, void* frame, if (framebuff && framebuff->dst.timestamp != meta->timestamp) { /* should never happen */ err_once("%s(%d), wrong frame timestamp\n", __func__, ctx->idx); - mt_pthread_mutex_unlock(&ctx->lock); return -EIO; } } @@ -104,11 +124,9 @@ static int rx_st20p_packet_convert(void* priv, void* frame, rte_atomic32_inc(&ctx->stat_busy); /* relaxed atomic: written from tasklet, read from any thread via * st20p_rx_get_session_stats(); no ordering vs other state required. */ - __atomic_fetch_add(&ctx->stat_frames_dropped, 1, __ATOMIC_RELAXED); - mt_pthread_mutex_unlock(&ctx->lock); + atomic_fetch_add_explicit(&ctx->stat_frames_dropped, 1, memory_order_relaxed); return -EBUSY; } - mt_pthread_mutex_unlock(&ctx->lock); if (ctx->ops.output_fmt == ST_FRAME_FMT_YUV422PLANAR10LE) { uint8_t* y = (uint8_t*)framebuff->dst.addr[0] + framebuff->dst.linesize[0] * meta->row_number + meta->row_offset * 2; @@ -155,7 +173,6 @@ static int rx_st20p_frame_ready(void* priv, void* frame, if (!ctx->ready) return -EBUSY; /* not ready */ - mt_pthread_mutex_lock(&ctx->lock); if (ctx->ops.flags & ST20P_RX_FLAG_PKT_CONVERT) { framebuff = rx_st20p_next_available(ctx, ctx->framebuff_producer_idx, ST20P_RX_FRAME_IN_CONVERTING); @@ -165,7 +182,6 @@ static int rx_st20p_frame_ready(void* priv, void* frame, rx_st20p_next_available(ctx, framebuff->idx, ST20P_RX_FRAME_IN_CONVERTING); if (framebuff && framebuff->dst.timestamp != meta->timestamp) { /* should never happen */ - mt_pthread_mutex_unlock(&ctx->lock); err_once("%s(%d), wrong frame timestamp\n", __func__, ctx->idx); return 0; /* surpress the error */ } @@ -178,8 +194,7 @@ static int rx_st20p_frame_ready(void* priv, void* frame, /* not any free frame */ if (!framebuff) { rte_atomic32_inc(&ctx->stat_busy); - __atomic_fetch_add(&ctx->stat_frames_dropped, 1, __ATOMIC_RELAXED); - mt_pthread_mutex_unlock(&ctx->lock); + atomic_fetch_add_explicit(&ctx->stat_frames_dropped, 1, memory_order_relaxed); return -EBUSY; } @@ -194,7 +209,6 @@ static int rx_st20p_frame_ready(void* priv, void* frame, if (ret < 0) { err("%s(%d), query_ext_frame for frame %u fail %d\n", __func__, ctx->idx, framebuff->idx, ret); - mt_pthread_mutex_unlock(&ctx->lock); return ret; } @@ -211,7 +225,6 @@ static int rx_st20p_frame_ready(void* priv, void* frame, if (ret < 0) { err("%s(%d), ext_frame check frame %u fail %d\n", __func__, ctx->idx, framebuff->idx, ret); - mt_pthread_mutex_unlock(&ctx->lock); return ret; } } @@ -257,18 +270,17 @@ static int rx_st20p_frame_ready(void* priv, void* frame, /* ask app to consume src frame directly */ if (ctx->derive || (ctx->ops.flags & ST20P_RX_FLAG_PKT_CONVERT)) { if (ctx->derive) framebuff->dst = framebuff->src; - framebuff->stat = ST20P_RX_FRAME_CONVERTED; + atomic_store_explicit(&framebuff->stat, ST20P_RX_FRAME_CONVERTED, + memory_order_release); /* point to next */ ctx->framebuff_producer_idx = rx_st20p_next_idx(ctx, framebuff->idx); - mt_pthread_mutex_unlock(&ctx->lock); rx_st20p_notify_frame_available(ctx); return 0; } - framebuff->stat = ST20P_RX_FRAME_READY; + atomic_store_explicit(&framebuff->stat, ST20P_RX_FRAME_READY, memory_order_release); /* point to next */ ctx->framebuff_producer_idx = rx_st20p_next_idx(ctx, framebuff->idx); - mt_pthread_mutex_unlock(&ctx->lock); dbg("%s(%d), frame %u succ\n", __func__, ctx->idx, framebuff->idx); @@ -291,14 +303,12 @@ static int rx_st20p_query_ext_frame(void* priv, struct st20_ext_frame* ext_frame if (!ctx->ready) return -EBUSY; /* not ready */ - mt_pthread_mutex_lock(&ctx->lock); framebuff = rx_st20p_next_available(ctx, ctx->framebuff_producer_idx, ST20P_RX_FRAME_FREE); /* not any free frame */ if (!framebuff) { rte_atomic32_inc(&ctx->stat_busy); - __atomic_fetch_add(&ctx->stat_frames_dropped, 1, __ATOMIC_RELAXED); - mt_pthread_mutex_unlock(&ctx->lock); + atomic_fetch_add_explicit(&ctx->stat_frames_dropped, 1, memory_order_relaxed); return -EBUSY; } @@ -306,7 +316,6 @@ static int rx_st20p_query_ext_frame(void* priv, struct st20_ext_frame* ext_frame memset(&ext_st, 0, sizeof(ext_st)); ret = ctx->ops.query_ext_frame(ctx->ops.priv, &ext_st, meta); if (ret < 0) { - mt_pthread_mutex_unlock(&ctx->lock); return -EBUSY; } /* only 1 plane for no converter mode */ @@ -315,7 +324,6 @@ static int rx_st20p_query_ext_frame(void* priv, struct st20_ext_frame* ext_frame ext_frame->buf_len = ext_st.size; ext_frame->opaque = ext_st.opaque; framebuff->src.opaque = ext_st.opaque; - mt_pthread_mutex_unlock(&ctx->lock); return 0; } @@ -397,19 +405,19 @@ static struct st20_convert_frame_meta* rx_st20p_convert_get_frame(void* priv) { if (!ctx->ready) goto out; /* not ready */ - mt_pthread_mutex_lock(&ctx->lock); + /* Claim READY -> IN_CONVERTING. rx_st20p_claim_available() retries across the + * ring on a lost CAS race, so it only returns NULL once every slot has + * actually been checked -- unlike a scan-then-single-CAS-attempt, which could + * give up even while other READY frames remain. */ framebuff = - rx_st20p_next_available(ctx, ctx->framebuff_convert_idx, ST20P_RX_FRAME_READY); + rx_st20p_claim_available(ctx, ctx->framebuff_convert_idx, ST20P_RX_FRAME_READY, + ST20P_RX_FRAME_IN_CONVERTING); /* not any ready frame */ if (!framebuff) { - mt_pthread_mutex_unlock(&ctx->lock); goto out; } - - framebuff->stat = ST20P_RX_FRAME_IN_CONVERTING; /* point to next */ ctx->framebuff_convert_idx = rx_st20p_next_idx(ctx, framebuff->idx); - mt_pthread_mutex_unlock(&ctx->lock); dbg("%s(%d), frame %u succ\n", __func__, idx, framebuff->idx); ret_frame = &framebuff->convert_frame; @@ -428,9 +436,10 @@ static int rx_st20p_convert_put_frame(void* priv, struct st20_convert_frame_meta MT_HANDLE_GUARD(ctx, MT_ST20_HANDLE_PIPELINE_RX, -EIO); - if (ST20P_RX_FRAME_IN_CONVERTING != framebuff->stat) { + if (ST20P_RX_FRAME_IN_CONVERTING != + atomic_load_explicit(&framebuff->stat, memory_order_acquire)) { err("%s(%d), frame %u not in converting %d\n", __func__, idx, convert_idx, - framebuff->stat); + (int)atomic_load_explicit(&framebuff->stat, memory_order_relaxed)); ret = -EIO; goto out; } @@ -439,10 +448,11 @@ static int rx_st20p_convert_put_frame(void* priv, struct st20_convert_frame_meta if (result < 0) { /* free the frame */ st20_rx_put_framebuff(ctx->transport, framebuff->src.addr[0]); - framebuff->stat = ST20P_RX_FRAME_FREE; + atomic_store_explicit(&framebuff->stat, ST20P_RX_FRAME_FREE, memory_order_release); rte_atomic32_inc(&ctx->stat_convert_fail); } else { - framebuff->stat = ST20P_RX_FRAME_CONVERTED; + atomic_store_explicit(&framebuff->stat, ST20P_RX_FRAME_CONVERTED, + memory_order_release); rx_st20p_notify_frame_available(ctx); } @@ -665,7 +675,7 @@ static int rx_st20p_init_dst_fbs(struct mtl_main_impl* impl, struct st20p_rx_ctx ctx->framebuffs = frames; for (uint16_t i = 0; i < ctx->framebuff_cnt; i++) { - frames[i].stat = ST20P_RX_FRAME_FREE; + atomic_store_explicit(&frames[i].stat, ST20P_RX_FRAME_FREE, memory_order_relaxed); frames[i].idx = i; frames[i].dst.fmt = ops->output_fmt; frames[i].dst.interlaced = ops->interlaced; @@ -777,12 +787,10 @@ static int rx_st20p_stat(void* priv) { if (!ctx->ready) return -EBUSY; /* not ready */ - mt_pthread_mutex_lock(&ctx->lock); producer_idx = ctx->framebuff_producer_idx; consumer_idx = ctx->framebuff_consumer_idx; producer_stat = framebuff[producer_idx].stat; consumer_stat = framebuff[consumer_idx].stat; - mt_pthread_mutex_unlock(&ctx->lock); notice("RX_st20p(%d,%s), p(%d:%s) c(%d:%s)\n", ctx->idx, ctx->ops_name, producer_idx, rx_st20p_stat_name(producer_stat), consumer_idx, @@ -842,67 +850,61 @@ struct st_frame* st20p_rx_get_frame(st20p_rx_handle handle) { ctx->stat_get_frame_try++; - mt_pthread_mutex_lock(&ctx->lock); - if (ctx->internal_converter) { /* convert internal */ - framebuff = - rx_st20p_next_available(ctx, ctx->framebuff_consumer_idx, ST20P_RX_FRAME_READY); + /* Claim READY->IN_USER before converting so no other consumer can take this + * slot. rx_st20p_claim_available() retries across the ring on a lost CAS + * race, so it only returns NULL once every slot has actually been checked. */ + framebuff = rx_st20p_claim_available(ctx, ctx->framebuff_consumer_idx, + ST20P_RX_FRAME_READY, ST20P_RX_FRAME_IN_USER); if (!framebuff && ctx->block_get) { /* wait here */ - mt_pthread_mutex_unlock(&ctx->lock); mt_pthread_mutex_lock(&ctx->block_wake_mutex); while (!ctx->block_wake_pending && - !__atomic_load_n(&ctx->lc_destroying, __ATOMIC_ACQUIRE)) { + !atomic_load_explicit(&ctx->lc_destroying, memory_order_acquire)) { int _ret = mt_pthread_cond_timedwait_ns( &ctx->block_wake_cond, &ctx->block_wake_mutex, ctx->block_timeout_ns); if (_ret) break; } ctx->block_wake_pending = false; mt_pthread_mutex_unlock(&ctx->block_wake_mutex); - if (__atomic_load_n(&ctx->lc_destroying, __ATOMIC_ACQUIRE)) goto out; + if (atomic_load_explicit(&ctx->lc_destroying, memory_order_acquire)) goto out; /* get again */ - mt_pthread_mutex_lock(&ctx->lock); - framebuff = - rx_st20p_next_available(ctx, ctx->framebuff_consumer_idx, ST20P_RX_FRAME_READY); + framebuff = rx_st20p_claim_available(ctx, ctx->framebuff_consumer_idx, + ST20P_RX_FRAME_READY, ST20P_RX_FRAME_IN_USER); } /* not any ready frame */ if (!framebuff) { - mt_pthread_mutex_unlock(&ctx->lock); goto out; } ctx->internal_converter->convert_func(&framebuff->src, &framebuff->dst); } else { - framebuff = rx_st20p_next_available(ctx, ctx->framebuff_consumer_idx, - ST20P_RX_FRAME_CONVERTED); + framebuff = + rx_st20p_claim_available(ctx, ctx->framebuff_consumer_idx, + ST20P_RX_FRAME_CONVERTED, ST20P_RX_FRAME_IN_USER); if (!framebuff && ctx->block_get) { /* wait here */ - mt_pthread_mutex_unlock(&ctx->lock); mt_pthread_mutex_lock(&ctx->block_wake_mutex); while (!ctx->block_wake_pending && - !__atomic_load_n(&ctx->lc_destroying, __ATOMIC_ACQUIRE)) { + !atomic_load_explicit(&ctx->lc_destroying, memory_order_acquire)) { int _ret = mt_pthread_cond_timedwait_ns( &ctx->block_wake_cond, &ctx->block_wake_mutex, ctx->block_timeout_ns); if (_ret) break; } ctx->block_wake_pending = false; mt_pthread_mutex_unlock(&ctx->block_wake_mutex); - if (__atomic_load_n(&ctx->lc_destroying, __ATOMIC_ACQUIRE)) goto out; + if (atomic_load_explicit(&ctx->lc_destroying, memory_order_acquire)) goto out; /* get again */ - mt_pthread_mutex_lock(&ctx->lock); - framebuff = rx_st20p_next_available(ctx, ctx->framebuff_consumer_idx, - ST20P_RX_FRAME_CONVERTED); + framebuff = + rx_st20p_claim_available(ctx, ctx->framebuff_consumer_idx, + ST20P_RX_FRAME_CONVERTED, ST20P_RX_FRAME_IN_USER); } /* not any converted frame */ if (!framebuff) { - mt_pthread_mutex_unlock(&ctx->lock); goto out; } } - framebuff->stat = ST20P_RX_FRAME_IN_USER; - /* point to next */ + /* point to next (best-effort hint; the CAS above is the real guard) */ ctx->framebuff_consumer_idx = rx_st20p_next_idx(ctx, framebuff->idx); - mt_pthread_mutex_unlock(&ctx->lock); - dbg("%s(%d), frame %u succ\n", __func__, idx, framebuff->idx); frame = &framebuff->dst; if (framebuff->user_meta_data_size) { @@ -913,9 +915,9 @@ struct st_frame* st20p_rx_get_frame(st20p_rx_handle handle) { frame->user_meta_size = 0; } ctx->stat_get_frame_succ++; - __atomic_fetch_add(&ctx->stat_frames_received, 1, __ATOMIC_RELAXED); + atomic_fetch_add_explicit(&ctx->stat_frames_received, 1, memory_order_relaxed); if (frame->status == ST_FRAME_STATUS_CORRUPTED) - __atomic_fetch_add(&ctx->stat_frames_corrupted, 1, __ATOMIC_RELAXED); + atomic_fetch_add_explicit(&ctx->stat_frames_corrupted, 1, memory_order_relaxed); MT_USDT_ST20P_RX_FRAME_GET(idx, framebuff->idx, frame->addr[0]); /* check if dump USDT enabled */ if (MT_USDT_ST20P_RX_FRAME_DUMP_ENABLED()) { @@ -941,16 +943,17 @@ int st20p_rx_put_frame(st20p_rx_handle handle, struct st_frame* frame) { MT_HANDLE_GUARD(ctx, MT_ST20_HANDLE_PIPELINE_RX, -EIO); - if (ST20P_RX_FRAME_IN_USER != framebuff->stat) { + if (ST20P_RX_FRAME_IN_USER != + atomic_load_explicit(&framebuff->stat, memory_order_acquire)) { err("%s(%d), frame %u not in user %d\n", __func__, idx, consumer_idx, - framebuff->stat); + (int)atomic_load_explicit(&framebuff->stat, memory_order_relaxed)); ret = -EIO; goto out; } /* free the frame */ st20_rx_put_framebuff(ctx->transport, framebuff->src.addr[0]); - framebuff->stat = ST20P_RX_FRAME_FREE; + atomic_store_explicit(&framebuff->stat, ST20P_RX_FRAME_FREE, memory_order_release); ctx->stat_put_frame++; MT_USDT_ST20P_RX_FRAME_PUT(idx, framebuff->idx, frame->addr[0]); @@ -971,16 +974,17 @@ int st20p_rx_put_frame_abort(st20p_rx_handle handle, struct st_frame* frame) { MT_HANDLE_GUARD(ctx, MT_ST20_HANDLE_PIPELINE_RX, -EIO); - if (ST20P_RX_FRAME_IN_USER != framebuff->stat) { + if (ST20P_RX_FRAME_IN_USER != + atomic_load_explicit(&framebuff->stat, memory_order_acquire)) { err("%s(%d), frame %u not in user %d\n", __func__, idx, consumer_idx, - framebuff->stat); + (int)atomic_load_explicit(&framebuff->stat, memory_order_relaxed)); ret = -EIO; goto out; } /* free the frame without processing */ st20_rx_put_framebuff(ctx->transport, framebuff->src.addr[0]); - framebuff->stat = ST20P_RX_FRAME_FREE; + atomic_store_explicit(&framebuff->stat, ST20P_RX_FRAME_FREE, memory_order_release); dbg("%s(%d), frame %u aborted\n", __func__, idx, consumer_idx); ret = 0; out: @@ -1054,7 +1058,6 @@ st20p_rx_handle st20p_rx_create(mtl_handle mt, struct st20p_rx_ops* ops) { ctx->dst_size = dst_size; rte_atomic32_set(&ctx->stat_convert_fail, 0); rte_atomic32_set(&ctx->stat_busy, 0); - mt_pthread_mutex_init(&ctx->lock, NULL); mt_pthread_mutex_init(&ctx->block_wake_mutex, NULL); mt_pthread_cond_wait_init(&ctx->block_wake_cond); @@ -1145,7 +1148,6 @@ int st20p_rx_free(st20p_rx_handle handle) { } rx_st20p_uinit_dst_fbs(ctx); - mt_pthread_mutex_destroy(&ctx->lock); mt_pthread_mutex_destroy(&ctx->block_wake_mutex); mt_pthread_cond_destroy(&ctx->block_wake_cond); notice("%s(%d), succ\n", __func__, ctx->idx); @@ -1235,11 +1237,11 @@ int st20p_rx_get_session_stats(st20p_rx_handle handle, struct st20_rx_user_stats if (ret < 0) goto out; /* Overlay pipeline-tracked frame-level counters; transport never sets these. */ stats->common.stat_frames_received = - __atomic_load_n(&ctx->stat_frames_received, __ATOMIC_RELAXED); + atomic_load_explicit(&ctx->stat_frames_received, memory_order_relaxed); stats->common.stat_frames_dropped = - __atomic_load_n(&ctx->stat_frames_dropped, __ATOMIC_RELAXED); + atomic_load_explicit(&ctx->stat_frames_dropped, memory_order_relaxed); stats->common.stat_frames_corrupted = - __atomic_load_n(&ctx->stat_frames_corrupted, __ATOMIC_RELAXED); + atomic_load_explicit(&ctx->stat_frames_corrupted, memory_order_relaxed); ret = 0; out: MT_HANDLE_RELEASE(ctx); @@ -1257,9 +1259,9 @@ int st20p_rx_reset_session_stats(st20p_rx_handle handle) { MT_HANDLE_GUARD(ctx, MT_ST20_HANDLE_PIPELINE_RX, 0); - __atomic_store_n(&ctx->stat_frames_received, 0, __ATOMIC_RELAXED); - __atomic_store_n(&ctx->stat_frames_dropped, 0, __ATOMIC_RELAXED); - __atomic_store_n(&ctx->stat_frames_corrupted, 0, __ATOMIC_RELAXED); + atomic_store_explicit(&ctx->stat_frames_received, 0, memory_order_relaxed); + atomic_store_explicit(&ctx->stat_frames_dropped, 0, memory_order_relaxed); + atomic_store_explicit(&ctx->stat_frames_corrupted, 0, memory_order_relaxed); ret = st20_rx_reset_session_stats(ctx->transport); MT_HANDLE_RELEASE(ctx); return ret; diff --git a/lib/src/st2110/pipeline/st20_pipeline_rx.h b/lib/src/st2110/pipeline/st20_pipeline_rx.h index 10da24635..245171b85 100644 --- a/lib/src/st2110/pipeline/st20_pipeline_rx.h +++ b/lib/src/st2110/pipeline/st20_pipeline_rx.h @@ -18,7 +18,7 @@ enum st20p_rx_frame_status { }; struct st20p_rx_frame { - enum st20p_rx_frame_status stat; + _Atomic uint32_t stat; struct st_frame src; /* before converting */ struct st_frame dst; /* converted */ struct st20_convert_frame_meta convert_frame; @@ -54,7 +54,6 @@ struct st20p_rx_ctx { uint16_t framebuff_convert_idx; uint16_t framebuff_consumer_idx; struct st20p_rx_frame* framebuffs; - pthread_mutex_t lock; int usdt_frame_cnt; /* for ST20P_RX_FLAG_BLOCK_GET */ diff --git a/lib/src/st2110/pipeline/st20_pipeline_tx.c b/lib/src/st2110/pipeline/st20_pipeline_tx.c index f40f29b61..379707f9e 100644 --- a/lib/src/st2110/pipeline/st20_pipeline_tx.c +++ b/lib/src/st2110/pipeline/st20_pipeline_tx.c @@ -29,6 +29,7 @@ static inline struct st_frame* tx_st20p_user_frame(struct st20p_tx_ctx* ctx, static void tx_st20p_block_wake(struct st20p_tx_ctx* ctx) { /* notify block */ mt_pthread_mutex_lock(&ctx->block_wake_mutex); + ctx->block_wake_pending = true; mt_pthread_cond_signal(&ctx->block_wake_cond); mt_pthread_mutex_unlock(&ctx->block_wake_mutex); } @@ -51,7 +52,7 @@ static struct st20p_tx_frame* tx_st20p_next_available( /* check ready frame from start */ for (int idx = 0; idx < ctx->framebuff_cnt; idx++) { framebuff = &ctx->framebuffs[idx]; - if (desired == framebuff->stat) { + if (desired == atomic_load_explicit(&framebuff->stat, memory_order_acquire)) { return framebuff; } } @@ -66,7 +67,7 @@ static struct st20p_tx_frame* tx_st20p_newest_available( for (uint16_t idx = 0; idx < ctx->framebuff_cnt; idx++) { framebuff = &ctx->framebuffs[idx]; - if ((desired == framebuff->stat && + if ((desired == atomic_load_explicit(&framebuff->stat, memory_order_acquire) && (!framebuff_newest || !mt_seq32_greater(framebuff->seq_number, framebuff_newest->seq_number)))) { framebuff_newest = framebuff; @@ -76,28 +77,46 @@ static struct st20p_tx_frame* tx_st20p_newest_available( return framebuff_newest; } +/* Scan for a framebuff in state `desired` and atomically claim it by + * transitioning it to `claimed`. A concurrent thread can win the race on the + * scanned candidate between the scan and the claim; on a lost race this keeps + * scanning instead of giving up, so the caller only sees NULL once every slot + * has genuinely been checked and found unavailable. */ +static struct st20p_tx_frame* tx_st20p_claim_available( + struct st20p_tx_ctx* ctx, enum st20p_tx_frame_status desired, + enum st20p_tx_frame_status claimed) { + struct st20p_tx_frame* framebuff; + + while ((framebuff = tx_st20p_next_available(ctx, desired))) { + uint32_t expected = desired; + if (atomic_compare_exchange_strong_explicit(&framebuff->stat, &expected, claimed, + memory_order_acq_rel, + memory_order_relaxed)) + return framebuff; + } + + return NULL; +} + /* Check if a CONVERTED frame is late. * * If late, the sequence is: - * 1. Park as DROPPED under lock — invisible to both next_frame (CONVERTED) and + * 1. Park as DROPPED — invisible to both next_frame (CONVERTED) and * get_frame (FREE), so no other thread can grab the slot while we clean up. - * 2. Unlock before all callbacks so app callbacks cannot deadlock on ctx->lock. - * 3. Fire notify_frame_done — app sees the frame and can safely release any + * 2. Fire notify_frame_done — app sees the frame and can safely release any * external buffer it owns (EXT_FRAME mode). - * 4. Fire notify_frame_late + USDT probe. - * 5. Re-acquire lock, clear ext-buffer pointers on the framebuff so the next - * get_frame call receives a clean slot with no stale external pointers. - * 6. If EXT_FRAME_USER_DONE: advance to IN_USER — app must call + * 3. Fire notify_frame_late + USDT probe. + * 4. Clear ext-buffer pointers on the framebuff so the next get_frame call + * receives a clean slot with no stale external pointers. + * 5. If EXT_FRAME_USER_DONE: advance to IN_USER — app must call * st20p_tx_notify_ext_frame_free to release the slot to FREE. - * Otherwise: advance to FREE, unlock, then notify_frame_available — + * Otherwise: advance to FREE, then notify_frame_available — * the slot is immediately visible to get_frame callers. - * 7. Re-acquire for the caller's do-while loop and return true. - * - * Must be called with ctx->lock held. Returns with ctx->lock held. */ + * 6. Return true so the caller's do-while loop retries. */ static bool tx_st20p_if_frame_late(struct st20p_tx_ctx* ctx, struct st20p_tx_frame* framebuff) { struct st_frame* frame = tx_st20p_user_frame(ctx, framebuff); - uint32_t rtp_ts; /* captured under lock for use in USDT after unlock */ + uint32_t rtp_ts; /* captured for use in USDT after state transition */ bool user_done = ctx->ops.flags & ST20P_TX_FLAG_EXT_FRAME_MANUAL_RELEASE; /* Park in IN_USER only when user_done flag is set and derive path */ bool need_in_user = user_done && !framebuff->frame_done_cb_called; @@ -127,11 +146,9 @@ static bool tx_st20p_if_frame_late(struct st20p_tx_ctx* ctx, ctx->stat_drop_frame++; /* relaxed atomic: written from both tasklet (late drop) and user thread * (put_frame_abort), and read from any thread via st20p_tx_get_session_stats(). */ - __atomic_fetch_add(&ctx->stat_frames_dropped, 1, __ATOMIC_RELAXED); + atomic_fetch_add_explicit(&ctx->stat_frames_dropped, 1, memory_order_relaxed); rtp_ts = frame->rtp_timestamp; - framebuff->stat = ST20P_TX_FRAME_DROPPED; - - mt_pthread_mutex_unlock(&ctx->lock); + atomic_store_explicit(&framebuff->stat, ST20P_TX_FRAME_DROPPED, memory_order_release); dbg("%s(%d), frame %u drop late by %" PRIu64 "ns (> period %" PRIu64 "ns), cur %" PRIu64 " frame %" PRIu64 "\n", @@ -147,18 +164,16 @@ static bool tx_st20p_if_frame_late(struct st20p_tx_ctx* ctx, if (ctx->ops.notify_frame_late) ctx->ops.notify_frame_late(ctx->ops.priv, 0); MT_USDT_ST20P_TX_FRAME_DROP(ctx->idx, framebuff->idx, rtp_ts); - mt_pthread_mutex_lock(&ctx->lock); - /* ext_frame derive: park in IN_USER until app calls * st20p_tx_notify_ext_frame_free */ - framebuff->stat = need_in_user ? ST20P_TX_FRAME_IN_USER : ST20P_TX_FRAME_FREE; - mt_pthread_mutex_unlock(&ctx->lock); + atomic_store_explicit(&framebuff->stat, + need_in_user ? ST20P_TX_FRAME_IN_USER : ST20P_TX_FRAME_FREE, + memory_order_release); if (!need_in_user) { tx_st20p_notify_frame_available(ctx); } - mt_pthread_mutex_lock(&ctx->lock); return true; /* frame was dropped, caller should retry */ } @@ -171,20 +186,34 @@ static int tx_st20p_next_frame(void* priv, uint16_t* next_frame_idx, if (!ctx->ready) return -EBUSY; /* not ready */ - mt_pthread_mutex_lock(&ctx->lock); - do { + while (1) { framebuff = tx_st20p_newest_available(ctx, ST20P_TX_FRAME_CONVERTED); if (!framebuff) break; /* no converted frame available */ - if (drop_cnt >= ST_TX_DROP_MAX_BATCH) { - info("%s(%d), max drop batch %d reached, stopping\n", __func__, ctx->idx, drop_cnt); - framebuff = NULL; - break; + + if (tx_st20p_if_frame_late(ctx, framebuff)) { + /* frame missed its TX window and was dropped; cap the batch then rescan */ + if (++drop_cnt >= ST_TX_DROP_MAX_BATCH) { + info("%s(%d), max drop batch %d reached, stopping\n", __func__, ctx->idx, + drop_cnt); + framebuff = NULL; + break; + } + continue; } - drop_cnt++; - } while (tx_st20p_if_frame_late(ctx, framebuff)); + + /* Claim CONVERTED -> IN_TRANSMITTING atomically. newest_available() is a + * lock-free scan, so between it and here the slot may have been taken by a + * late-drop or (defensively) another consumer; on a lost race the CAS fails + * and we rescan for another CONVERTED frame. Mirrors the FREE->IN_USER and + * READY->IN_CONVERTING claims. */ + uint32_t expected = ST20P_TX_FRAME_CONVERTED; + if (atomic_compare_exchange_strong_explicit( + &framebuff->stat, &expected, ST20P_TX_FRAME_IN_TRANSMITTING, + memory_order_acq_rel, memory_order_relaxed)) + break; /* claimed */ + } if (!framebuff) { - mt_pthread_mutex_unlock(&ctx->lock); /* When drop-when-late is active, ensure the app knows about free slots so it * can refill the pipeline promptly after drops freed frames. */ if (ctx->ops.flags & ST20P_TX_FLAG_DROP_WHEN_LATE) { @@ -194,7 +223,6 @@ static int tx_st20p_next_frame(void* priv, uint16_t* next_frame_idx, return -EBUSY; } - framebuff->stat = ST20P_TX_FRAME_IN_TRANSMITTING; *next_frame_idx = framebuff->idx; frame = tx_st20p_user_frame(ctx, framebuff); @@ -208,7 +236,6 @@ static int tx_st20p_next_frame(void* priv, uint16_t* next_frame_idx, meta->user_meta_size = framebuff->user_meta_data_size; } - mt_pthread_mutex_unlock(&ctx->lock); dbg("%s(%d), frame %u succ, frame_idx: %u\n", __func__, ctx->idx, framebuff->idx, framebuff->idx); MT_USDT_ST20P_TX_FRAME_NEXT(ctx->idx, framebuff->idx); @@ -236,17 +263,18 @@ static int tx_st20p_frame_done(void* priv, uint16_t frame_idx, /* Transition state BEFORE the callback so the app can call * st20p_tx_notify_ext_frame_free from within notify_frame_done. */ - mt_pthread_mutex_lock(&ctx->lock); - if (ST20P_TX_FRAME_IN_TRANSMITTING == framebuff->stat) { + if (ST20P_TX_FRAME_IN_TRANSMITTING == + atomic_load_explicit(&framebuff->stat, memory_order_acquire)) { ret = 0; - framebuff->stat = need_in_user ? ST20P_TX_FRAME_IN_USER : ST20P_TX_FRAME_FREE; + atomic_store_explicit(&framebuff->stat, + need_in_user ? ST20P_TX_FRAME_IN_USER : ST20P_TX_FRAME_FREE, + memory_order_release); dbg("%s(%d), frame_idx: %u\n", __func__, ctx->idx, frame_idx); } else { ret = -EIO; - err("%s(%d), err status %d for frame %u\n", __func__, ctx->idx, framebuff->stat, - frame_idx); + err("%s(%d), err status %d for frame %u\n", __func__, ctx->idx, + (int)atomic_load_explicit(&framebuff->stat, memory_order_relaxed), frame_idx); } - mt_pthread_mutex_unlock(&ctx->lock); if (ctx->ops.notify_frame_done && !framebuff->frame_done_cb_called) { /* notify app which frame done */ @@ -254,7 +282,8 @@ static int tx_st20p_frame_done(void* priv, uint16_t frame_idx, ctx->ops.notify_frame_done(ctx->ops.priv, frame); framebuff->frame_done_cb_called = true; } - if (ret == 0) __atomic_fetch_add(&ctx->stat_frames_sent, 1, __ATOMIC_RELAXED); + if (ret == 0) + atomic_fetch_add_explicit(&ctx->stat_frames_sent, 1, memory_order_relaxed); if (!need_in_user) { /* notify app can get frame */ @@ -287,17 +316,23 @@ static struct st20_convert_frame_meta* tx_st20p_convert_get_frame(void* priv) { if (!ctx->ready) goto out; /* not ready */ - mt_pthread_mutex_lock(&ctx->lock); - framebuff = tx_st20p_newest_available(ctx, ST20P_TX_FRAME_READY); - /* not any free frame */ + /* Claim the oldest READY -> IN_CONVERTING. newest_available() picks by + * seq_number to preserve conversion order; a concurrent converter thread may + * have already taken the picked slot between the scan and the CAS, so on a + * lost race rescan rather than giving up -- otherwise a losing thread would + * report "no frame" even though other READY frames remain. */ + while ((framebuff = tx_st20p_newest_available(ctx, ST20P_TX_FRAME_READY))) { + uint32_t expected_stat = ST20P_TX_FRAME_READY; + if (atomic_compare_exchange_strong_explicit( + &framebuff->stat, &expected_stat, ST20P_TX_FRAME_IN_CONVERTING, + memory_order_acq_rel, memory_order_relaxed)) + break; + } + /* not any ready frame */ if (!framebuff) { - mt_pthread_mutex_unlock(&ctx->lock); goto out; } - framebuff->stat = ST20P_TX_FRAME_IN_CONVERTING; - mt_pthread_mutex_unlock(&ctx->lock); - dbg("%s(%d), frame %u succ, frame_idx: %u\n", __func__, idx, framebuff->idx, framebuff->idx); ret_frame = &framebuff->convert_frame; @@ -317,11 +352,10 @@ static int tx_st20p_convert_put_frame(void* priv, struct st20_convert_frame_meta MT_HANDLE_GUARD(ctx, MT_ST20_HANDLE_PIPELINE_TX, -EIO); - mt_pthread_mutex_lock(&ctx->lock); - if (ST20P_TX_FRAME_IN_CONVERTING != framebuff->stat) { - mt_pthread_mutex_unlock(&ctx->lock); + if (ST20P_TX_FRAME_IN_CONVERTING != + atomic_load_explicit(&framebuff->stat, memory_order_acquire)) { err("%s(%d), frame %u not in converting %d\n", __func__, idx, convert_idx, - framebuff->stat); + (int)atomic_load_explicit(&framebuff->stat, memory_order_relaxed)); ret = -EIO; goto out; } @@ -330,15 +364,14 @@ static int tx_st20p_convert_put_frame(void* priv, struct st20_convert_frame_meta dbg("%s(%d), frame %u result %d data_size %" PRIu64 ", frame_idx: %u\n", __func__, idx, convert_idx, result, data_size, convert_idx); - framebuff->stat = ST20P_TX_FRAME_FREE; - mt_pthread_mutex_unlock(&ctx->lock); + atomic_store_explicit(&framebuff->stat, ST20P_TX_FRAME_FREE, memory_order_release); /* notify app can get frame */ tx_st20p_notify_frame_available(ctx); rte_atomic32_inc(&ctx->stat_convert_fail); } else { - framebuff->stat = ST20P_TX_FRAME_CONVERTED; - mt_pthread_mutex_unlock(&ctx->lock); + atomic_store_explicit(&framebuff->stat, ST20P_TX_FRAME_CONVERTED, + memory_order_release); } if (ctx->ops.notify_frame_done && !framebuff->frame_done_cb_called) { @@ -520,7 +553,7 @@ static int tx_st20p_init_src_fbs(struct mtl_main_impl* impl, struct st20p_tx_ctx ctx->framebuffs = frames; for (uint16_t i = 0; i < ctx->framebuff_cnt; i++) { - frames[i].stat = ST20P_TX_FRAME_FREE; + atomic_store_explicit(&frames[i].stat, ST20P_TX_FRAME_FREE, memory_order_relaxed); frames[i].idx = i; frames[i].src.fmt = ops->input_fmt; frames[i].src.interlaced = ops->interlaced; @@ -620,7 +653,10 @@ static int tx_st20p_stat(void* priv) { if (!ctx->ready) return -EBUSY; /* not ready */ for (uint16_t j = 0; j < ctx->framebuff_cnt; j++) { - enum st20p_tx_frame_status stat = framebuff[j].stat; + /* relaxed: concurrent with data-plane stat writes; a torn snapshot only + * skews the diagnostic counters, never the lock-free protocol. */ + enum st20p_tx_frame_status stat = + atomic_load_explicit(&framebuff[j].stat, memory_order_relaxed); if (stat < ST20P_TX_FRAME_STATUS_MAX) { status_counts[stat]++; } @@ -685,10 +721,13 @@ static void tx_st20p_framebuffs_flush(struct st20p_tx_ctx* ctx) { int retry = 0; while (1) { - if (framebuff->stat == ST20P_TX_FRAME_FREE) break; - if (framebuff->stat == ST20P_TX_FRAME_DROPPED) - break; /* dropped, effectively free */ - if (framebuff->stat == ST20P_TX_FRAME_IN_TRANSMITTING) { + /* acquire: the transport tasklet may still be draining IN_TRANSMITTING + * frames concurrently with this teardown scan. */ + enum st20p_tx_frame_status stat = + atomic_load_explicit(&framebuff->stat, memory_order_acquire); + if (stat == ST20P_TX_FRAME_FREE) break; + if (stat == ST20P_TX_FRAME_DROPPED) break; /* dropped, effectively free */ + if (stat == ST20P_TX_FRAME_IN_TRANSMITTING) { /* make sure transport to finish the transmit */ /* WA to use sleep here, todo: add a transport API to query the stat */ mt_sleep_ms(50); @@ -696,11 +735,11 @@ static void tx_st20p_framebuffs_flush(struct st20p_tx_ctx* ctx) { } dbg("%s(%d), frame %u are still in %s, retry %d, frame_idx: %u\n", __func__, - ctx->idx, i, tx_st20p_stat_name(framebuff->stat), retry, i); + ctx->idx, i, tx_st20p_stat_name(stat), retry, i); retry++; if (retry > 100) { info("%s(%d), frame %u are still in %s, retry %d\n", __func__, ctx->idx, i, - tx_st20p_stat_name(framebuff->stat), retry); + tx_st20p_stat_name(stat), retry); break; } mt_sleep_ms(10); @@ -722,30 +761,34 @@ struct st_frame* st20p_tx_get_frame(st20p_tx_handle handle) { ctx->stat_get_frame_try++; - mt_pthread_mutex_lock(&ctx->lock); - framebuff = tx_st20p_next_available(ctx, ST20P_TX_FRAME_FREE); + /* Claim FREE->IN_USER. tx_st20p_claim_available() retries across the ring + * on a lost CAS race, so it only returns NULL once every slot has actually + * been checked -- unlike a scan-then-single-CAS-attempt, which could give up + * even while other FREE frames remain (spurious failure under contention). */ + framebuff = tx_st20p_claim_available(ctx, ST20P_TX_FRAME_FREE, ST20P_TX_FRAME_IN_USER); if (!framebuff && ctx->block_get) { /* wait here */ - mt_pthread_mutex_unlock(&ctx->lock); mt_pthread_mutex_lock(&ctx->block_wake_mutex); - if (!__atomic_load_n(&ctx->lc_destroying, __ATOMIC_ACQUIRE)) - mt_pthread_cond_timedwait_ns(&ctx->block_wake_cond, &ctx->block_wake_mutex, - ctx->block_timeout_ns); + while (!ctx->block_wake_pending && + !atomic_load_explicit(&ctx->lc_destroying, memory_order_acquire)) { + int _ret = mt_pthread_cond_timedwait_ns( + &ctx->block_wake_cond, &ctx->block_wake_mutex, ctx->block_timeout_ns); + if (_ret) break; + } + ctx->block_wake_pending = false; mt_pthread_mutex_unlock(&ctx->block_wake_mutex); - if (__atomic_load_n(&ctx->lc_destroying, __ATOMIC_ACQUIRE)) goto out; + if (atomic_load_explicit(&ctx->lc_destroying, memory_order_acquire)) goto out; /* get again */ - mt_pthread_mutex_lock(&ctx->lock); - framebuff = tx_st20p_next_available(ctx, ST20P_TX_FRAME_FREE); + framebuff = + tx_st20p_claim_available(ctx, ST20P_TX_FRAME_FREE, ST20P_TX_FRAME_IN_USER); } /* not any free frame */ if (!framebuff) { - mt_pthread_mutex_unlock(&ctx->lock); goto out; } - framebuff->stat = ST20P_TX_FRAME_IN_USER; framebuff->frame_done_cb_called = false; - framebuff->seq_number = ctx->framebuff_sequence_number++; - mt_pthread_mutex_unlock(&ctx->lock); + framebuff->seq_number = + atomic_fetch_add_explicit(&ctx->framebuff_sequence_number, 1, memory_order_relaxed); dbg("%s(%d), frame %u succ\n", __func__, idx, framebuff->idx); frame = tx_st20p_user_frame(ctx, framebuff); @@ -771,9 +814,10 @@ int st20p_tx_put_frame(st20p_tx_handle handle, struct st_frame* frame) { MT_HANDLE_GUARD(ctx, MT_ST20_HANDLE_PIPELINE_TX, -EIO); - if (ST20P_TX_FRAME_IN_USER != framebuff->stat) { + if (ST20P_TX_FRAME_IN_USER != + atomic_load_explicit(&framebuff->stat, memory_order_acquire)) { err("%s(%d), frame %u not in user %d\n", __func__, idx, producer_idx, - framebuff->stat); + (int)atomic_load_explicit(&framebuff->stat, memory_order_relaxed)); ret = -EIO; goto out; } @@ -790,7 +834,7 @@ int st20p_tx_put_frame(st20p_tx_handle handle, struct st_frame* frame) { if (frame->user_meta_size > framebuff->user_meta_buffer_size) { err("%s(%d), frame %u user meta size %" PRId64 " too large\n", __func__, idx, producer_idx, frame->user_meta_size); - framebuff->stat = ST20P_TX_FRAME_FREE; + atomic_store_explicit(&framebuff->stat, ST20P_TX_FRAME_FREE, memory_order_release); ret = -EIO; goto out; } @@ -806,11 +850,13 @@ int st20p_tx_put_frame(st20p_tx_handle handle, struct st_frame* frame) { if (ctx->internal_converter) { /* convert internal */ ctx->internal_converter->convert_func(&framebuff->src, &framebuff->dst); - framebuff->stat = ST20P_TX_FRAME_CONVERTED; + atomic_store_explicit(&framebuff->stat, ST20P_TX_FRAME_CONVERTED, + memory_order_release); } else if (ctx->derive) { - framebuff->stat = ST20P_TX_FRAME_CONVERTED; + atomic_store_explicit(&framebuff->stat, ST20P_TX_FRAME_CONVERTED, + memory_order_release); } else { - framebuff->stat = ST20P_TX_FRAME_READY; + atomic_store_explicit(&framebuff->stat, ST20P_TX_FRAME_READY, memory_order_release); st20_convert_notify_frame_ready(ctx->convert_impl); } ctx->stat_put_frame++; @@ -842,16 +888,17 @@ int st20p_tx_put_frame_abort(st20p_tx_handle handle, struct st_frame* frame) { MT_HANDLE_GUARD(ctx, MT_ST20_HANDLE_PIPELINE_TX, -EIO); - if (ST20P_TX_FRAME_IN_USER != framebuff->stat) { + if (ST20P_TX_FRAME_IN_USER != + atomic_load_explicit(&framebuff->stat, memory_order_acquire)) { err("%s(%d), frame %u not in user %d\n", __func__, idx, producer_idx, - framebuff->stat); + (int)atomic_load_explicit(&framebuff->stat, memory_order_relaxed)); ret = -EIO; goto out; } - framebuff->stat = ST20P_TX_FRAME_FREE; + atomic_store_explicit(&framebuff->stat, ST20P_TX_FRAME_FREE, memory_order_release); ctx->stat_drop_frame++; - __atomic_fetch_add(&ctx->stat_frames_dropped, 1, __ATOMIC_RELAXED); + atomic_fetch_add_explicit(&ctx->stat_frames_dropped, 1, memory_order_relaxed); dbg("%s(%d), frame %u aborted\n", __func__, idx, producer_idx); ret = 0; out: @@ -875,9 +922,10 @@ int st20p_tx_put_ext_frame(st20p_tx_handle handle, struct st_frame* frame, goto out; } - if (ST20P_TX_FRAME_IN_USER != framebuff->stat) { + if (ST20P_TX_FRAME_IN_USER != + atomic_load_explicit(&framebuff->stat, memory_order_acquire)) { err("%s(%d), frame %u not in user %d\n", __func__, idx, producer_idx, - framebuff->stat); + (int)atomic_load_explicit(&framebuff->stat, memory_order_relaxed)); ret = -EIO; goto out; } @@ -902,7 +950,8 @@ int st20p_tx_put_ext_frame(st20p_tx_handle handle, struct st_frame* frame, framebuff->dst.iova[0] = ext_frame->iova[0]; framebuff->dst.opaque = ext_frame->opaque; framebuff->dst.flags |= ST_FRAME_FLAG_EXT_BUF; - framebuff->stat = ST20P_TX_FRAME_CONVERTED; + atomic_store_explicit(&framebuff->stat, ST20P_TX_FRAME_CONVERTED, + memory_order_release); } else { for (int plane = 0; plane < planes; plane++) { framebuff->src.addr[plane] = ext_frame->addr[plane]; @@ -921,13 +970,14 @@ int st20p_tx_put_ext_frame(st20p_tx_handle handle, struct st_frame* frame, } if (ctx->internal_converter) { /* convert internal */ ctx->internal_converter->convert_func(&framebuff->src, &framebuff->dst); - framebuff->stat = ST20P_TX_FRAME_CONVERTED; + atomic_store_explicit(&framebuff->stat, ST20P_TX_FRAME_CONVERTED, + memory_order_release); if (ctx->ops.notify_frame_done && !framebuff->frame_done_cb_called) { ctx->ops.notify_frame_done(ctx->ops.priv, &framebuff->src); framebuff->frame_done_cb_called = true; } } else { - framebuff->stat = ST20P_TX_FRAME_READY; + atomic_store_explicit(&framebuff->stat, ST20P_TX_FRAME_READY, memory_order_release); st20_convert_notify_frame_ready(ctx->convert_impl); } } @@ -967,20 +1017,18 @@ int st20p_tx_notify_ext_frame_free(st20p_tx_handle handle, struct st_frame* fram goto out; } - mt_pthread_mutex_lock(&ctx->lock); - if (ST20P_TX_FRAME_IN_USER != framebuff->stat) { + if (ST20P_TX_FRAME_IN_USER != + atomic_load_explicit(&framebuff->stat, memory_order_acquire)) { /* Frame is not in IN_USER, this happens when the converter already released * the ext buffer during put_ext_frame and the frame went CONVERTED -> FREE * without the IN_USER step. Silently succeed so callers can always call * this unconditionally from notify_frame_done. */ - mt_pthread_mutex_unlock(&ctx->lock); dbg("%s(%d), frame %u not in_user (stat %d), skip\n", __func__, idx, framebuff->idx, - framebuff->stat); + (int)atomic_load_explicit(&framebuff->stat, memory_order_relaxed)); ret = 0; goto out; } - framebuff->stat = ST20P_TX_FRAME_FREE; - mt_pthread_mutex_unlock(&ctx->lock); + atomic_store_explicit(&framebuff->stat, ST20P_TX_FRAME_FREE, memory_order_release); tx_st20p_notify_frame_available(ctx); @@ -1043,7 +1091,6 @@ st20p_tx_handle st20p_tx_create(mtl_handle mt, struct st20p_tx_ops* ops) { ctx->src_size = src_size; rte_atomic32_set(&ctx->stat_convert_fail, 0); rte_atomic32_set(&ctx->stat_busy, 0); - mt_pthread_mutex_init(&ctx->lock, NULL); mt_pthread_mutex_init(&ctx->block_wake_mutex, NULL); mt_pthread_cond_wait_init(&ctx->block_wake_cond); @@ -1141,7 +1188,6 @@ int st20p_tx_free(st20p_tx_handle handle) { tx_st20p_uinit_src_fbs(ctx); - mt_pthread_mutex_destroy(&ctx->lock); mt_pthread_mutex_destroy(&ctx->block_wake_mutex); mt_pthread_cond_destroy(&ctx->block_wake_cond); notice("%s(%d), succ\n", __func__, ctx->idx); @@ -1229,9 +1275,9 @@ int st20p_tx_get_session_stats(st20p_tx_handle handle, struct st20_tx_user_stats if (ret < 0) goto out; /* Overlay pipeline-tracked frame-level counters; transport never sets these. */ stats->common.stat_frames_sent = - __atomic_load_n(&ctx->stat_frames_sent, __ATOMIC_RELAXED); + atomic_load_explicit(&ctx->stat_frames_sent, memory_order_relaxed); stats->common.stat_frames_dropped = - __atomic_load_n(&ctx->stat_frames_dropped, __ATOMIC_RELAXED); + atomic_load_explicit(&ctx->stat_frames_dropped, memory_order_relaxed); ret = 0; out: MT_HANDLE_RELEASE(ctx); @@ -1249,8 +1295,8 @@ int st20p_tx_reset_session_stats(st20p_tx_handle handle) { MT_HANDLE_GUARD(ctx, MT_ST20_HANDLE_PIPELINE_TX, 0); - __atomic_store_n(&ctx->stat_frames_sent, 0, __ATOMIC_RELAXED); - __atomic_store_n(&ctx->stat_frames_dropped, 0, __ATOMIC_RELAXED); + atomic_store_explicit(&ctx->stat_frames_sent, 0, memory_order_relaxed); + atomic_store_explicit(&ctx->stat_frames_dropped, 0, memory_order_relaxed); ret = st20_tx_reset_session_stats(ctx->transport); MT_HANDLE_RELEASE(ctx); return ret; diff --git a/lib/src/st2110/pipeline/st20_pipeline_tx.h b/lib/src/st2110/pipeline/st20_pipeline_tx.h index b0d553130..0ed0949e4 100644 --- a/lib/src/st2110/pipeline/st20_pipeline_tx.h +++ b/lib/src/st2110/pipeline/st20_pipeline_tx.h @@ -20,7 +20,7 @@ enum st20p_tx_frame_status { }; struct st20p_tx_frame { - enum st20p_tx_frame_status stat; + _Atomic uint32_t stat; struct st_frame src; /* before converting */ struct st_frame dst; /* converted */ struct st20_convert_frame_meta convert_frame; @@ -47,9 +47,8 @@ struct st20p_tx_ctx { st20_tx_handle transport; uint16_t framebuff_cnt; - uint32_t framebuff_sequence_number; + _Atomic uint32_t framebuff_sequence_number; struct st20p_tx_frame* framebuffs; - pthread_mutex_t lock; int usdt_frame_cnt; struct st20_convert_session_impl* convert_impl; @@ -66,6 +65,7 @@ struct st20p_tx_ctx { pthread_cond_t block_wake_cond; pthread_mutex_t block_wake_mutex; uint64_t block_timeout_ns; + bool block_wake_pending; rte_atomic32_t stat_convert_fail; rte_atomic32_t stat_busy; diff --git a/lib/src/st2110/pipeline/st22_pipeline_rx.c b/lib/src/st2110/pipeline/st22_pipeline_rx.c index 147696b6d..531187a69 100644 --- a/lib/src/st2110/pipeline/st22_pipeline_rx.c +++ b/lib/src/st2110/pipeline/st22_pipeline_rx.c @@ -94,6 +94,27 @@ static struct st22p_rx_frame* rx_st22p_next_available( return NULL; } +/* Scan from idx_start for a framebuff in state `desired` and atomically claim + * it by transitioning it to `claimed`. A concurrent thread can win the race on + * the scanned candidate between the scan and the claim; on a lost race this + * keeps scanning instead of giving up, so the caller only sees NULL once every + * slot has genuinely been checked and found unavailable. */ +static struct st22p_rx_frame* rx_st22p_claim_available( + struct st22p_rx_ctx* ctx, uint16_t idx_start, enum st22p_rx_frame_status desired, + enum st22p_rx_frame_status claimed) { + struct st22p_rx_frame* framebuff; + + while ((framebuff = rx_st22p_next_available(ctx, idx_start, desired))) { + uint32_t expected = desired; + if (atomic_compare_exchange_strong_explicit(&framebuff->stat, &expected, claimed, + memory_order_acq_rel, + memory_order_relaxed)) + return framebuff; + } + + return NULL; +} + static int rx_st22p_frame_ready(void* priv, void* frame, struct st22_rx_frame_meta* meta) { struct st22p_rx_ctx* ctx = priv; @@ -102,13 +123,11 @@ static int rx_st22p_frame_ready(void* priv, void* frame, if (!ctx->ready) return -EBUSY; /* not ready */ - mt_pthread_mutex_lock(&ctx->lock); framebuff = rx_st22p_next_available(ctx, ctx->framebuff_producer_idx, ST22P_RX_FRAME_FREE); /* not any free frame */ if (!framebuff) { rte_atomic32_inc(&ctx->stat_busy); - mt_pthread_mutex_unlock(&ctx->lock); return -EBUSY; } @@ -122,7 +141,6 @@ static int rx_st22p_frame_ready(void* priv, void* frame, if (ret < 0) { err("%s(%d), query_ext_frame for frame %u fail %d\n", __func__, ctx->idx, framebuff->idx, ret); - mt_pthread_mutex_unlock(&ctx->lock); return ret; } @@ -139,7 +157,6 @@ static int rx_st22p_frame_ready(void* priv, void* frame, if (ret < 0) { err("%s(%d), ext_frame check frame %u fail %d\n", __func__, ctx->idx, framebuff->idx, ret); - mt_pthread_mutex_unlock(&ctx->lock); return ret; } } @@ -170,14 +187,12 @@ static int rx_st22p_frame_ready(void* priv, void* frame, framebuff->stat = ST22P_RX_FRAME_DECODED; /* point to next */ ctx->framebuff_producer_idx = rx_st22p_next_idx(ctx, framebuff->idx); - mt_pthread_mutex_unlock(&ctx->lock); rx_st22p_notify_frame_available(ctx); return 0; } framebuff->stat = ST22P_RX_FRAME_READY; /* point to next */ ctx->framebuff_producer_idx = rx_st22p_next_idx(ctx, framebuff->idx); - mt_pthread_mutex_unlock(&ctx->lock); dbg("%s(%d), frame %u succ\n", __func__, ctx->idx, framebuff->idx); rx_st22p_decode_notify_frame_ready(ctx); @@ -237,28 +252,25 @@ static struct st22_decode_frame_meta* rx_st22p_decode_get_frame(void* priv) { ctx->stat_decode_get_frame_try++; - mt_pthread_mutex_lock(&ctx->lock); - framebuff = - rx_st22p_next_available(ctx, ctx->framebuff_decode_idx, ST22P_RX_FRAME_READY); + /* Claim READY -> IN_DECODING. rx_st22p_claim_available() retries across the + * ring on a lost CAS race, so it only returns NULL once every slot has + * actually been checked -- unlike a scan-then-single-CAS-attempt, which could + * give up even while other READY frames remain. */ + framebuff = rx_st22p_claim_available(ctx, ctx->framebuff_decode_idx, + ST22P_RX_FRAME_READY, ST22P_RX_FRAME_IN_DECODING); if (!framebuff && ctx->decode_block_get) { /* wait here for block mode */ - mt_pthread_mutex_unlock(&ctx->lock); rx_st22p_decode_get_block_wait(ctx); /* get again */ - mt_pthread_mutex_lock(&ctx->lock); - framebuff = - rx_st22p_next_available(ctx, ctx->framebuff_decode_idx, ST22P_RX_FRAME_READY); + framebuff = rx_st22p_claim_available( + ctx, ctx->framebuff_decode_idx, ST22P_RX_FRAME_READY, ST22P_RX_FRAME_IN_DECODING); } /* not any ready frame */ if (!framebuff) { - mt_pthread_mutex_unlock(&ctx->lock); dbg("%s(%d), no ready frame\n", __func__, idx); goto out; } - - framebuff->stat = ST22P_RX_FRAME_IN_DECODING; /* point to next */ ctx->framebuff_decode_idx = rx_st22p_next_idx(ctx, framebuff->idx); - mt_pthread_mutex_unlock(&ctx->lock); ctx->stat_decode_get_frame_succ++; ret_frame = &framebuff->decode_frame; @@ -580,32 +592,29 @@ struct st_frame* st22p_rx_get_frame(st22p_rx_handle handle) { ctx->stat_get_frame_try++; - mt_pthread_mutex_lock(&ctx->lock); - framebuff = - rx_st22p_next_available(ctx, ctx->framebuff_consumer_idx, ST22P_RX_FRAME_DECODED); + /* Claim DECODED->IN_USER. rx_st22p_claim_available() retries across the ring + * on a lost CAS race, so it only returns NULL once every slot has actually + * been checked -- unlike a scan-then-single-CAS-attempt, which could give up + * even while other DECODED frames remain (spurious failure under contention). */ + framebuff = rx_st22p_claim_available(ctx, ctx->framebuff_consumer_idx, + ST22P_RX_FRAME_DECODED, ST22P_RX_FRAME_IN_USER); if (!framebuff && ctx->block_get) { - mt_pthread_mutex_unlock(&ctx->lock); mt_pthread_mutex_lock(&ctx->block_wake_mutex); - if (!__atomic_load_n(&ctx->lc_destroying, __ATOMIC_ACQUIRE)) + if (!atomic_load_explicit(&ctx->lc_destroying, memory_order_acquire)) mt_pthread_cond_timedwait_ns(&ctx->block_wake_cond, &ctx->block_wake_mutex, ctx->block_timeout_ns); mt_pthread_mutex_unlock(&ctx->block_wake_mutex); - if (__atomic_load_n(&ctx->lc_destroying, __ATOMIC_ACQUIRE)) goto out; + if (atomic_load_explicit(&ctx->lc_destroying, memory_order_acquire)) goto out; /* get again */ - mt_pthread_mutex_lock(&ctx->lock); - framebuff = - rx_st22p_next_available(ctx, ctx->framebuff_consumer_idx, ST22P_RX_FRAME_DECODED); + framebuff = rx_st22p_claim_available(ctx, ctx->framebuff_consumer_idx, + ST22P_RX_FRAME_DECODED, ST22P_RX_FRAME_IN_USER); } /* not any decoded frame */ if (!framebuff) { - mt_pthread_mutex_unlock(&ctx->lock); goto out; } - - framebuff->stat = ST22P_RX_FRAME_IN_USER; - /* point to next */ + /* point to next (best-effort hint; the CAS above is the real guard) */ ctx->framebuff_consumer_idx = rx_st22p_next_idx(ctx, framebuff->idx); - mt_pthread_mutex_unlock(&ctx->lock); dbg("%s(%d), frame %u succ\n", __func__, idx, framebuff->idx); ctx->stat_get_frame_succ++; @@ -765,7 +774,6 @@ st22p_rx_handle st22p_rx_create(mtl_handle mt, struct st22p_rx_ops* ops) { } rte_atomic32_set(&ctx->stat_decode_fail, 0); rte_atomic32_set(&ctx->stat_busy, 0); - mt_pthread_mutex_init(&ctx->lock, NULL); mt_pthread_mutex_init(&ctx->decode_block_wake_mutex, NULL); mt_pthread_cond_wait_init(&ctx->decode_block_wake_cond); @@ -846,7 +854,6 @@ int st22p_rx_free(st22p_rx_handle handle) { } rx_st22p_uinit_dst_fbs(ctx); - mt_pthread_mutex_destroy(&ctx->lock); mt_pthread_mutex_destroy(&ctx->block_wake_mutex); mt_pthread_cond_destroy(&ctx->block_wake_cond); mt_pthread_mutex_destroy(&ctx->decode_block_wake_mutex); diff --git a/lib/src/st2110/pipeline/st22_pipeline_rx.h b/lib/src/st2110/pipeline/st22_pipeline_rx.h index 074042d94..737cdb554 100644 --- a/lib/src/st2110/pipeline/st22_pipeline_rx.h +++ b/lib/src/st2110/pipeline/st22_pipeline_rx.h @@ -18,7 +18,7 @@ enum st22p_rx_frame_status { }; struct st22p_rx_frame { - enum st22p_rx_frame_status stat; + _Atomic uint32_t stat; struct st_frame src; /* before decoding */ struct st_frame dst; /* decoded */ struct st22_decode_frame_meta decode_frame; @@ -45,7 +45,6 @@ struct st22p_rx_ctx { uint16_t framebuff_decode_idx; uint16_t framebuff_consumer_idx; struct st22p_rx_frame* framebuffs; - pthread_mutex_t lock; /* for ST22P_RX_FLAG_BLOCK_GET */ bool block_get; diff --git a/lib/src/st2110/pipeline/st22_pipeline_tx.c b/lib/src/st2110/pipeline/st22_pipeline_tx.c index 7fdf4e872..b8f0cde0a 100644 --- a/lib/src/st2110/pipeline/st22_pipeline_tx.c +++ b/lib/src/st2110/pipeline/st22_pipeline_tx.c @@ -27,6 +27,7 @@ static inline struct st_frame* tx_st22p_user_frame(struct st22p_tx_ctx* ctx, static void tx_st22p_block_wake(struct st22p_tx_ctx* ctx) { /* notify block */ mt_pthread_mutex_lock(&ctx->block_wake_mutex); + ctx->block_wake_pending = true; mt_pthread_cond_signal(&ctx->block_wake_cond); mt_pthread_mutex_unlock(&ctx->block_wake_mutex); } @@ -105,6 +106,27 @@ static struct st22p_tx_frame* tx_st22p_newest_available( return framebuff_newest; } +/* Scan for a framebuff in state `desired` and atomically claim it by + * transitioning it to `claimed`. A concurrent thread can win the race on the + * scanned candidate between the scan and the claim; on a lost race this keeps + * scanning instead of giving up, so the caller only sees NULL once every slot + * has genuinely been checked and found unavailable. */ +static struct st22p_tx_frame* tx_st22p_claim_available( + struct st22p_tx_ctx* ctx, enum st22p_tx_frame_status desired, + enum st22p_tx_frame_status claimed) { + struct st22p_tx_frame* framebuff; + + while ((framebuff = tx_st22p_next_available(ctx, desired))) { + uint32_t expected = desired; + if (atomic_compare_exchange_strong_explicit(&framebuff->stat, &expected, claimed, + memory_order_acq_rel, + memory_order_relaxed)) + return framebuff; + } + + return NULL; +} + /* Check if the newest ENCODED frame has missed its transmission window. * Drops the frame (ENCODED -> DROPPED -> FREE) if cur_tai > frame_tai + frame_period. * @@ -141,8 +163,6 @@ static bool tx_st22p_if_frame_late(struct st22p_tx_ctx* ctx, rtp_ts = frame->rtp_timestamp; framebuff->stat = ST22P_TX_FRAME_DROPPED; - mt_pthread_mutex_unlock(&ctx->lock); - notice("%s(%d), frame %u drop late by %" PRIu64 "ns (> period %" PRIu64 "ns), cur %" PRIu64 " frame %" PRIu64 "\n", __func__, ctx->idx, framebuff->seq_number, cur_tai - frame_tai, frame_period_ns, @@ -157,13 +177,10 @@ static bool tx_st22p_if_frame_late(struct st22p_tx_ctx* ctx, if (ctx->ops.notify_frame_late) ctx->ops.notify_frame_late(ctx->ops.priv, 0); MT_USDT_ST22P_TX_FRAME_DROP(ctx->idx, framebuff->idx, rtp_ts); - mt_pthread_mutex_lock(&ctx->lock); framebuff->stat = ST22P_TX_FRAME_FREE; - mt_pthread_mutex_unlock(&ctx->lock); tx_st22p_notify_frame_available(ctx); - mt_pthread_mutex_lock(&ctx->lock); return true; /* frame was dropped, caller should retry */ } @@ -175,21 +192,34 @@ static int tx_st22p_next_frame(void* priv, uint16_t* next_frame_idx, if (!ctx->ready) return -EBUSY; /* not ready */ - mt_pthread_mutex_lock(&ctx->lock); - do { + while (1) { framebuff = tx_st22p_newest_available(ctx, ST22P_TX_FRAME_ENCODED); if (!framebuff) break; /* no encoded frame available */ - if (drop_cnt >= ST_TX_DROP_MAX_BATCH) { - info("%s(%d), max drop batch %d reached, stopping\n", __func__, ctx->idx, drop_cnt); - framebuff = NULL; - break; + + if (tx_st22p_if_frame_late(ctx, framebuff)) { + /* frame missed its TX window and was dropped; cap the batch then rescan */ + if (++drop_cnt >= ST_TX_DROP_MAX_BATCH) { + info("%s(%d), max drop batch %d reached, stopping\n", __func__, ctx->idx, + drop_cnt); + framebuff = NULL; + break; + } + continue; } - drop_cnt++; - } while (tx_st22p_if_frame_late(ctx, framebuff)); + + /* Claim ENCODED -> IN_TRANSMITTING atomically. newest_available() is a + * lock-free scan, so the slot may have been taken by a late-drop or + * (defensively) another consumer between the scan and here; on a lost race + * the CAS fails and we rescan. Mirrors the FREE->IN_USER claim. */ + uint32_t expected = ST22P_TX_FRAME_ENCODED; + if (atomic_compare_exchange_strong_explicit( + &framebuff->stat, &expected, ST22P_TX_FRAME_IN_TRANSMITTING, + memory_order_acq_rel, memory_order_relaxed)) + break; /* claimed */ + } /* not any encoded frame */ if (!framebuff) { - mt_pthread_mutex_unlock(&ctx->lock); /* When drop-when-late is active, ensure the app knows about free slots so it * can refill the pipeline promptly after drops freed frames. */ if (ctx->ops.flags & ST22P_TX_FLAG_DROP_WHEN_LATE) { @@ -199,7 +229,6 @@ static int tx_st22p_next_frame(void* priv, uint16_t* next_frame_idx, return -EBUSY; } - framebuff->stat = ST22P_TX_FRAME_IN_TRANSMITTING; *next_frame_idx = framebuff->idx; struct st_frame* frame = tx_st22p_user_frame(ctx, framebuff); @@ -211,7 +240,6 @@ static int tx_st22p_next_frame(void* priv, uint16_t* next_frame_idx, framebuff->idx, meta->timestamp); } meta->codestream_size = framebuff->dst.data_size; - mt_pthread_mutex_unlock(&ctx->lock); dbg("%s(%d), frame %u succ, frame_idx: %u\n", __func__, ctx->idx, framebuff->idx, framebuff->idx); MT_USDT_ST22P_TX_FRAME_NEXT(ctx->idx, framebuff->idx); @@ -231,7 +259,6 @@ static int tx_st22p_frame_done(void* priv, uint16_t frame_idx, framebuff->dst.timestamp = meta->timestamp; framebuff->src.rtp_timestamp = framebuff->dst.rtp_timestamp = meta->rtp_timestamp; - mt_pthread_mutex_lock(&ctx->lock); if (ST22P_TX_FRAME_IN_TRANSMITTING == framebuff->stat) { ret = 0; framebuff->stat = ST22P_TX_FRAME_FREE; @@ -241,7 +268,6 @@ static int tx_st22p_frame_done(void* priv, uint16_t frame_idx, err("%s(%d), err status %d for frame %u\n", __func__, ctx->idx, framebuff->stat, frame_idx); } - mt_pthread_mutex_unlock(&ctx->lock); if (ctx->ops.notify_frame_done && !framebuff->frame_done_cb_called) { /* notify app which frame done */ @@ -310,25 +336,24 @@ static struct st22_encode_frame_meta* tx_st22p_encode_get_frame(void* priv) { ctx->stat_encode_get_frame_try++; - mt_pthread_mutex_lock(&ctx->lock); - framebuff = tx_st22p_next_available(ctx, ST22P_TX_FRAME_READY); + /* Claim READY->IN_ENCODING. tx_st22p_claim_available() retries across the + * ring on a lost CAS race, so it only returns NULL once every slot has + * actually been checked -- unlike a scan-then-single-CAS-attempt, which + * could give up even while other READY frames remain. */ + framebuff = + tx_st22p_claim_available(ctx, ST22P_TX_FRAME_READY, ST22P_TX_FRAME_IN_ENCODING); if (!framebuff && ctx->encode_block_get) { /* wait here for block mode */ - mt_pthread_mutex_unlock(&ctx->lock); tx_st22p_encode_get_block_wait(ctx); /* get again */ - mt_pthread_mutex_lock(&ctx->lock); - framebuff = tx_st22p_next_available(ctx, ST22P_TX_FRAME_READY); + framebuff = + tx_st22p_claim_available(ctx, ST22P_TX_FRAME_READY, ST22P_TX_FRAME_IN_ENCODING); } /* not any free frame */ if (!framebuff) { - mt_pthread_mutex_unlock(&ctx->lock); dbg("%s(%d), no ready frame\n", __func__, idx); goto out; } - framebuff->stat = ST22P_TX_FRAME_IN_ENCODING; - mt_pthread_mutex_unlock(&ctx->lock); - ctx->stat_encode_get_frame_succ++; dbg("%s(%d), frame %u succ\n", __func__, idx, framebuff->idx); ret_frame = &framebuff->encode_frame; @@ -354,9 +379,7 @@ static int tx_st22p_encode_put_frame(void* priv, struct st22_encode_frame_meta* MT_HANDLE_GUARD(ctx, MT_ST22_HANDLE_PIPELINE_TX, -EIO); - mt_pthread_mutex_lock(&ctx->lock); if (ST22P_TX_FRAME_IN_ENCODING != framebuff->stat) { - mt_pthread_mutex_unlock(&ctx->lock); err("%s(%d), frame %u not in encoding %d\n", __func__, idx, encode_idx, framebuff->stat); ret = -EIO; @@ -372,12 +395,10 @@ static int tx_st22p_encode_put_frame(void* priv, struct st22_encode_frame_meta* __func__, idx, encode_idx, result, data_size, ST22_ENCODE_MIN_FRAME_SZ, max_size); framebuff->stat = ST22P_TX_FRAME_FREE; - mt_pthread_mutex_unlock(&ctx->lock); tx_st22p_notify_frame_available(ctx); rte_atomic32_inc(&ctx->stat_encode_fail); } else { framebuff->stat = ST22P_TX_FRAME_ENCODED; - mt_pthread_mutex_unlock(&ctx->lock); } MT_USDT_ST22P_TX_ENCODE_PUT(idx, framebuff->idx, frame->src->addr[0], @@ -727,30 +748,34 @@ struct st_frame* st22p_tx_get_frame(st22p_tx_handle handle) { ctx->stat_get_frame_try++; - mt_pthread_mutex_lock(&ctx->lock); - framebuff = tx_st22p_next_available(ctx, ST22P_TX_FRAME_FREE); + /* Claim FREE->IN_USER. tx_st22p_claim_available() retries across the ring + * on a lost CAS race, so it only returns NULL once every slot has actually + * been checked -- unlike a scan-then-single-CAS-attempt, which could give up + * even while other FREE frames remain (spurious failure under contention). */ + framebuff = tx_st22p_claim_available(ctx, ST22P_TX_FRAME_FREE, ST22P_TX_FRAME_IN_USER); if (!framebuff && ctx->block_get) { - mt_pthread_mutex_unlock(&ctx->lock); mt_pthread_mutex_lock(&ctx->block_wake_mutex); - if (!__atomic_load_n(&ctx->lc_destroying, __ATOMIC_ACQUIRE)) - mt_pthread_cond_timedwait_ns(&ctx->block_wake_cond, &ctx->block_wake_mutex, - ctx->block_timeout_ns); + while (!ctx->block_wake_pending && + !atomic_load_explicit(&ctx->lc_destroying, memory_order_acquire)) { + int _ret = mt_pthread_cond_timedwait_ns( + &ctx->block_wake_cond, &ctx->block_wake_mutex, ctx->block_timeout_ns); + if (_ret) break; + } + ctx->block_wake_pending = false; mt_pthread_mutex_unlock(&ctx->block_wake_mutex); - if (__atomic_load_n(&ctx->lc_destroying, __ATOMIC_ACQUIRE)) goto out; + if (atomic_load_explicit(&ctx->lc_destroying, memory_order_acquire)) goto out; /* get again */ - mt_pthread_mutex_lock(&ctx->lock); - framebuff = tx_st22p_next_available(ctx, ST22P_TX_FRAME_FREE); + framebuff = + tx_st22p_claim_available(ctx, ST22P_TX_FRAME_FREE, ST22P_TX_FRAME_IN_USER); } /* not any free frame */ if (!framebuff) { - mt_pthread_mutex_unlock(&ctx->lock); goto out; } - framebuff->stat = ST22P_TX_FRAME_IN_USER; framebuff->frame_done_cb_called = false; - framebuff->seq_number = ctx->framebuff_sequence_number++; - mt_pthread_mutex_unlock(&ctx->lock); + framebuff->seq_number = + atomic_fetch_add_explicit(&ctx->framebuff_sequence_number, 1, memory_order_relaxed); dbg("%s(%d), frame %u succ\n", __func__, idx, framebuff->idx); if (ctx->ops.interlaced) { /* init second_field but user still can customize */ @@ -979,7 +1004,6 @@ st22p_tx_handle st22p_tx_create(mtl_handle mt, struct st22p_tx_ops* ops) { ctx->wake_on_destroy = (void (*)(void*))tx_st22p_block_wake_all; ctx->src_size = src_size; rte_atomic32_set(&ctx->stat_encode_fail, 0); - mt_pthread_mutex_init(&ctx->lock, NULL); mt_pthread_mutex_init(&ctx->encode_block_wake_mutex, NULL); mt_pthread_cond_wait_init(&ctx->encode_block_wake_cond); @@ -1066,7 +1090,6 @@ int st22p_tx_free(st22p_tx_handle handle) { } tx_st22p_uinit_src_fbs(ctx); - mt_pthread_mutex_destroy(&ctx->lock); mt_pthread_mutex_destroy(&ctx->block_wake_mutex); mt_pthread_cond_destroy(&ctx->block_wake_cond); mt_pthread_mutex_destroy(&ctx->encode_block_wake_mutex); diff --git a/lib/src/st2110/pipeline/st22_pipeline_tx.h b/lib/src/st2110/pipeline/st22_pipeline_tx.h index 2e2e18593..b371a7859 100644 --- a/lib/src/st2110/pipeline/st22_pipeline_tx.h +++ b/lib/src/st2110/pipeline/st22_pipeline_tx.h @@ -20,7 +20,7 @@ enum st22p_tx_frame_status { }; struct st22p_tx_frame { - enum st22p_tx_frame_status stat; + _Atomic uint32_t stat; struct st_frame src; /* before encoding */ struct st_frame dst; /* encoded */ struct st22_encode_frame_meta encode_frame; @@ -45,15 +45,15 @@ struct st22p_tx_ctx { st22_tx_handle transport; uint16_t framebuff_cnt; - uint32_t framebuff_sequence_number; + _Atomic uint32_t framebuff_sequence_number; struct st22p_tx_frame* framebuffs; - pthread_mutex_t lock; /* protect framebuffs */ /* for ST22P_TX_FLAG_BLOCK_GET */ bool block_get; pthread_cond_t block_wake_cond; pthread_mutex_t block_wake_mutex; uint64_t block_timeout_ns; + bool block_wake_pending; struct st22_encode_session_impl* encode_impl; /* for ST22_ENCODER_RESP_FLAG_BLOCK_GET */ diff --git a/lib/src/st2110/pipeline/st30_pipeline_rx.c b/lib/src/st2110/pipeline/st30_pipeline_rx.c index 3ade1b283..56d530fc5 100644 --- a/lib/src/st2110/pipeline/st30_pipeline_rx.c +++ b/lib/src/st2110/pipeline/st30_pipeline_rx.c @@ -53,7 +53,7 @@ static struct st30p_rx_frame* rx_st30p_next_available( /* check ready frame from idx_start */ while (1) { framebuff = &ctx->framebuffs[idx]; - if (desired == framebuff->stat) { + if (desired == atomic_load_explicit(&framebuff->stat, memory_order_acquire)) { /* find one desired */ return framebuff; } @@ -68,13 +68,33 @@ static struct st30p_rx_frame* rx_st30p_next_available( return NULL; } +/* Scan from idx_start for a framebuff in state `desired` and atomically claim + * it by transitioning it to `claimed`. A concurrent thread can win the race on + * the scanned candidate between the scan and the claim; on a lost race this + * keeps scanning instead of giving up, so the caller only sees NULL once every + * slot has genuinely been checked and found unavailable. */ +static struct st30p_rx_frame* rx_st30p_claim_available( + struct st30p_rx_ctx* ctx, uint16_t idx_start, enum st30p_rx_frame_status desired, + enum st30p_rx_frame_status claimed) { + struct st30p_rx_frame* framebuff; + + while ((framebuff = rx_st30p_next_available(ctx, idx_start, desired))) { + uint32_t expected = desired; + if (atomic_compare_exchange_strong_explicit(&framebuff->stat, &expected, claimed, + memory_order_acq_rel, + memory_order_relaxed)) + return framebuff; + } + + return NULL; +} + static int rx_st30p_frame_ready(void* priv, void* addr, struct st30_rx_frame_meta* meta) { struct st30p_rx_ctx* ctx = priv; struct st30p_rx_frame* framebuff; if (!ctx->ready) return -EBUSY; /* not ready */ - mt_pthread_mutex_lock(&ctx->lock); framebuff = rx_st30p_next_available(ctx, ctx->framebuff_producer_idx, ST30P_RX_FRAME_FREE); @@ -83,8 +103,7 @@ static int rx_st30p_frame_ready(void* priv, void* addr, struct st30_rx_frame_met ctx->stat_busy++; /* relaxed atomic: written from tasklet, read from any thread via * st30p_rx_get_session_stats(); no ordering vs other state required. */ - __atomic_fetch_add(&ctx->stat_frames_dropped, 1, __ATOMIC_RELAXED); - mt_pthread_mutex_unlock(&ctx->lock); + atomic_fetch_add_explicit(&ctx->stat_frames_dropped, 1, memory_order_relaxed); return -EBUSY; } @@ -96,10 +115,9 @@ static int rx_st30p_frame_ready(void* priv, void* addr, struct st30_rx_frame_met frame->receive_timestamp = meta->timestamp_first_pkt; frame->rtp_timestamp = meta->rtp_timestamp; frame->status = meta->status; - framebuff->stat = ST30P_RX_FRAME_READY; + atomic_store_explicit(&framebuff->stat, ST30P_RX_FRAME_READY, memory_order_release); /* point to next */ ctx->framebuff_producer_idx = rx_st30p_next_idx(ctx, framebuff->idx); - mt_pthread_mutex_unlock(&ctx->lock); dbg("%s(%d), frame %u(%p) succ\n", __func__, ctx->idx, framebuff->idx, frame->addr); /* notify app to a ready frame */ @@ -183,7 +201,7 @@ static int rx_st30p_init_fbs(struct st30p_rx_ctx* ctx, struct st30p_rx_ops* ops) struct st30p_rx_frame* framebuff = &frames[i]; struct st30_frame* frame = &framebuff->frame; - framebuff->stat = ST30P_RX_FRAME_FREE; + atomic_store_explicit(&framebuff->stat, ST30P_RX_FRAME_FREE, memory_order_relaxed); framebuff->idx = i; /* addr will be resolved later in rx_st30p_frame_ready */ @@ -211,12 +229,10 @@ static int rx_st30p_stat(void* priv) { if (!ctx->ready) return -EBUSY; /* not ready */ - mt_pthread_mutex_lock(&ctx->lock); producer_idx = ctx->framebuff_producer_idx; consumer_idx = ctx->framebuff_consumer_idx; producer_stat = framebuff[producer_idx].stat; consumer_stat = framebuff[consumer_idx].stat; - mt_pthread_mutex_unlock(&ctx->lock); notice("RX_st30p(%d,%s), p(%d:%s) c(%d:%s)\n", ctx->idx, ctx->ops_name, producer_idx, rx_st30p_stat_name(producer_stat), consumer_idx, @@ -297,43 +313,39 @@ struct st30_frame* st30p_rx_get_frame(st30p_rx_handle handle) { ctx->stat_get_frame_try++; - mt_pthread_mutex_lock(&ctx->lock); - - framebuff = - rx_st30p_next_available(ctx, ctx->framebuff_consumer_idx, ST30P_RX_FRAME_READY); + /* Claim READY->IN_USER. rx_st30p_claim_available() retries across the ring + * on a lost CAS race, so it only returns NULL once every slot has actually + * been checked -- unlike a scan-then-single-CAS-attempt, which could give up + * even while other READY frames remain (spurious failure under contention). */ + framebuff = rx_st30p_claim_available(ctx, ctx->framebuff_consumer_idx, + ST30P_RX_FRAME_READY, ST30P_RX_FRAME_IN_USER); if (!framebuff && ctx->block_get) { /* wait here */ - mt_pthread_mutex_unlock(&ctx->lock); mt_pthread_mutex_lock(&ctx->block_wake_mutex); while (!ctx->block_wake_pending && - !__atomic_load_n(&ctx->lc_destroying, __ATOMIC_ACQUIRE)) { + !atomic_load_explicit(&ctx->lc_destroying, memory_order_acquire)) { int _ret = mt_pthread_cond_timedwait_ns( &ctx->block_wake_cond, &ctx->block_wake_mutex, ctx->block_timeout_ns); if (_ret) break; } ctx->block_wake_pending = false; mt_pthread_mutex_unlock(&ctx->block_wake_mutex); - if (__atomic_load_n(&ctx->lc_destroying, __ATOMIC_ACQUIRE)) goto out; + if (atomic_load_explicit(&ctx->lc_destroying, memory_order_acquire)) goto out; /* get again */ - mt_pthread_mutex_lock(&ctx->lock); - framebuff = - rx_st30p_next_available(ctx, ctx->framebuff_consumer_idx, ST30P_RX_FRAME_READY); + framebuff = rx_st30p_claim_available(ctx, ctx->framebuff_consumer_idx, + ST30P_RX_FRAME_READY, ST30P_RX_FRAME_IN_USER); } /* not any converted frame */ if (!framebuff) { - mt_pthread_mutex_unlock(&ctx->lock); goto out; } - - framebuff->stat = ST30P_RX_FRAME_IN_USER; - /* point to next */ + /* point to next (best-effort hint; the CAS above is the real guard) */ ctx->framebuff_consumer_idx = rx_st30p_next_idx(ctx, framebuff->idx); - mt_pthread_mutex_unlock(&ctx->lock); frame = &framebuff->frame; ctx->stat_get_frame_succ++; - __atomic_fetch_add(&ctx->stat_frames_received, 1, __ATOMIC_RELAXED); + atomic_fetch_add_explicit(&ctx->stat_frames_received, 1, memory_order_relaxed); if (frame->status == ST_FRAME_STATUS_CORRUPTED) - __atomic_fetch_add(&ctx->stat_frames_corrupted, 1, __ATOMIC_RELAXED); + atomic_fetch_add_explicit(&ctx->stat_frames_corrupted, 1, memory_order_relaxed); MT_USDT_ST30P_RX_FRAME_GET(idx, framebuff->idx, frame->addr); dbg("%s(%d), frame %u(%p) succ\n", __func__, idx, framebuff->idx, frame->addr); /* check if dump USDT enabled */ @@ -356,16 +368,17 @@ int st30p_rx_put_frame(st30p_rx_handle handle, struct st30_frame* frame) { MT_HANDLE_GUARD(ctx, MT_ST30_HANDLE_PIPELINE_RX, -EIO); - if (ST30P_RX_FRAME_IN_USER != framebuff->stat) { + if (ST30P_RX_FRAME_IN_USER != + atomic_load_explicit(&framebuff->stat, memory_order_acquire)) { err("%s(%d), frame %u not in user %d\n", __func__, idx, consumer_idx, - framebuff->stat); + (int)atomic_load_explicit(&framebuff->stat, memory_order_relaxed)); ret = -EIO; goto out; } /* free the frame */ st30_rx_put_framebuff(ctx->transport, frame->addr); - framebuff->stat = ST30P_RX_FRAME_FREE; + atomic_store_explicit(&framebuff->stat, ST30P_RX_FRAME_FREE, memory_order_release); ctx->stat_put_frame++; MT_USDT_ST30P_RX_FRAME_PUT(idx, framebuff->idx, frame->addr); @@ -385,16 +398,17 @@ int st30p_rx_put_frame_abort(st30p_rx_handle handle, struct st30_frame* frame) { MT_HANDLE_GUARD(ctx, MT_ST30_HANDLE_PIPELINE_RX, -EIO); - if (ST30P_RX_FRAME_IN_USER != framebuff->stat) { + if (ST30P_RX_FRAME_IN_USER != + atomic_load_explicit(&framebuff->stat, memory_order_acquire)) { err("%s(%d), frame %u not in user %d\n", __func__, idx, consumer_idx, - framebuff->stat); + (int)atomic_load_explicit(&framebuff->stat, memory_order_relaxed)); ret = -EIO; goto out; } /* free the frame without processing */ st30_rx_put_framebuff(ctx->transport, frame->addr); - framebuff->stat = ST30P_RX_FRAME_FREE; + atomic_store_explicit(&framebuff->stat, ST30P_RX_FRAME_FREE, memory_order_release); dbg("%s(%d), frame %u aborted\n", __func__, idx, consumer_idx); ret = 0; out: @@ -435,7 +449,6 @@ int st30p_rx_free(st30p_rx_handle handle) { rx_st30p_uinit_fbs(ctx); rx_st30p_usdt_dump_close(ctx); - mt_pthread_mutex_destroy(&ctx->lock); mt_pthread_mutex_destroy(&ctx->block_wake_mutex); mt_pthread_cond_destroy(&ctx->block_wake_cond); notice("%s(%d), succ\n", __func__, ctx->idx); @@ -487,7 +500,6 @@ st30p_rx_handle st30p_rx_create(mtl_handle mt, struct st30p_rx_ops* ops) { ctx->wake_on_destroy = (void (*)(void*))rx_st30p_block_wake; ctx->usdt_dump_fd = -1; - mt_pthread_mutex_init(&ctx->lock, NULL); mt_pthread_mutex_init(&ctx->block_wake_mutex, NULL); mt_pthread_cond_wait_init(&ctx->block_wake_cond); ctx->block_timeout_ns = NS_PER_S; @@ -568,11 +580,11 @@ int st30p_rx_get_session_stats(st30p_rx_handle handle, struct st30_rx_user_stats if (ret < 0) goto out; /* Overlay pipeline-tracked frame-level counters; transport never sets these. */ stats->common.stat_frames_received = - __atomic_load_n(&ctx->stat_frames_received, __ATOMIC_RELAXED); + atomic_load_explicit(&ctx->stat_frames_received, memory_order_relaxed); stats->common.stat_frames_dropped = - __atomic_load_n(&ctx->stat_frames_dropped, __ATOMIC_RELAXED); + atomic_load_explicit(&ctx->stat_frames_dropped, memory_order_relaxed); stats->common.stat_frames_corrupted = - __atomic_load_n(&ctx->stat_frames_corrupted, __ATOMIC_RELAXED); + atomic_load_explicit(&ctx->stat_frames_corrupted, memory_order_relaxed); ret = 0; out: MT_HANDLE_RELEASE(ctx); @@ -590,9 +602,9 @@ int st30p_rx_reset_session_stats(st30p_rx_handle handle) { MT_HANDLE_GUARD(ctx, MT_ST30_HANDLE_PIPELINE_RX, 0); - __atomic_store_n(&ctx->stat_frames_received, 0, __ATOMIC_RELAXED); - __atomic_store_n(&ctx->stat_frames_dropped, 0, __ATOMIC_RELAXED); - __atomic_store_n(&ctx->stat_frames_corrupted, 0, __ATOMIC_RELAXED); + atomic_store_explicit(&ctx->stat_frames_received, 0, memory_order_relaxed); + atomic_store_explicit(&ctx->stat_frames_dropped, 0, memory_order_relaxed); + atomic_store_explicit(&ctx->stat_frames_corrupted, 0, memory_order_relaxed); ret = st30_rx_reset_session_stats(ctx->transport); MT_HANDLE_RELEASE(ctx); return ret; diff --git a/lib/src/st2110/pipeline/st30_pipeline_rx.h b/lib/src/st2110/pipeline/st30_pipeline_rx.h index a2232cd3e..515d4e65b 100644 --- a/lib/src/st2110/pipeline/st30_pipeline_rx.h +++ b/lib/src/st2110/pipeline/st30_pipeline_rx.h @@ -16,7 +16,7 @@ enum st30p_rx_frame_status { }; struct st30p_rx_frame { - enum st30p_rx_frame_status stat; + _Atomic uint32_t stat; struct st30_frame frame; uint16_t idx; }; @@ -39,7 +39,6 @@ struct st30p_rx_ctx { uint16_t framebuff_producer_idx; uint16_t framebuff_consumer_idx; struct st30p_rx_frame* framebuffs; - pthread_mutex_t lock; bool ready; /* usdt dump */ diff --git a/lib/src/st2110/pipeline/st30_pipeline_tx.c b/lib/src/st2110/pipeline/st30_pipeline_tx.c index 10ca17213..deb5ae4f6 100644 --- a/lib/src/st2110/pipeline/st30_pipeline_tx.c +++ b/lib/src/st2110/pipeline/st30_pipeline_tx.c @@ -22,6 +22,7 @@ static const char* tx_st30p_stat_name(enum st30p_tx_frame_status stat) { static void tx_st30p_block_wake(struct st30p_tx_ctx* ctx) { /* notify block */ mt_pthread_mutex_lock(&ctx->block_wake_mutex); + ctx->block_wake_pending = true; mt_pthread_cond_signal(&ctx->block_wake_cond); mt_pthread_mutex_unlock(&ctx->block_wake_mutex); } @@ -43,7 +44,7 @@ static struct st30p_tx_frame* tx_st30p_next_available( for (int idx = 0; idx < ctx->framebuff_cnt; idx++) { framebuff = &ctx->framebuffs[idx]; - if (desired == framebuff->stat) { + if (desired == atomic_load_explicit(&framebuff->stat, memory_order_acquire)) { return framebuff; } } @@ -68,6 +69,27 @@ static struct st30p_tx_frame* tx_st30p_newest_available( return framebuff_newest; } +/* Scan for a framebuff in state `desired` and atomically claim it by + * transitioning it to `claimed`. A concurrent thread can win the race on the + * scanned candidate between the scan and the claim; on a lost race this keeps + * scanning instead of giving up, so the caller only sees NULL once every slot + * has genuinely been checked and found unavailable. */ +static struct st30p_tx_frame* tx_st30p_claim_available( + struct st30p_tx_ctx* ctx, enum st30p_tx_frame_status desired, + enum st30p_tx_frame_status claimed) { + struct st30p_tx_frame* framebuff; + + while ((framebuff = tx_st30p_next_available(ctx, desired))) { + uint32_t expected = desired; + if (atomic_compare_exchange_strong_explicit(&framebuff->stat, &expected, claimed, + memory_order_acq_rel, + memory_order_relaxed)) + return framebuff; + } + + return NULL; +} + /* Check if the oldest READY frame has missed its transmission window. * Drops the frame (READY -> DROPPED -> FREE) if cur_tai > frame_tai + frame_period. * @@ -103,11 +125,9 @@ static bool tx_st30p_if_frame_late(struct st30p_tx_ctx* ctx, ctx->stat_drop_frame++; /* relaxed atomic: written from both tasklet (late drop) and user thread * (put_frame_abort), and read from any thread via st30p_tx_get_session_stats(). */ - __atomic_fetch_add(&ctx->stat_frames_dropped, 1, __ATOMIC_RELAXED); + atomic_fetch_add_explicit(&ctx->stat_frames_dropped, 1, memory_order_relaxed); rtp_ts = frame->rtp_timestamp; - framebuff->stat = ST30P_TX_FRAME_DROPPED; - - mt_pthread_mutex_unlock(&ctx->lock); + atomic_store_explicit(&framebuff->stat, ST30P_TX_FRAME_DROPPED, memory_order_release); dbg("%s(%d), frame %u drop late by %" PRIu64 "ns (> period %" PRIu64 "ns), cur %" PRIu64 " frame %" PRIu64 "\n", @@ -123,13 +143,10 @@ static bool tx_st30p_if_frame_late(struct st30p_tx_ctx* ctx, if (ctx->ops.notify_frame_late) ctx->ops.notify_frame_late(ctx->ops.priv, 0); MT_USDT_ST30P_TX_FRAME_DROP(ctx->idx, framebuff->idx, rtp_ts); - mt_pthread_mutex_lock(&ctx->lock); - framebuff->stat = ST30P_TX_FRAME_FREE; - mt_pthread_mutex_unlock(&ctx->lock); + atomic_store_explicit(&framebuff->stat, ST30P_TX_FRAME_FREE, memory_order_release); tx_st30p_notify_frame_available(ctx); - mt_pthread_mutex_lock(&ctx->lock); return true; /* frame was dropped, caller should retry */ } @@ -143,7 +160,6 @@ static int tx_st30p_next_frame(void* priv, uint16_t* next_frame_idx, if (!ctx->ready) return -EBUSY; /* not ready */ - mt_pthread_mutex_lock(&ctx->lock); do { framebuff = tx_st30p_newest_available(ctx, ST30P_TX_FRAME_READY); if (!framebuff) break; /* no ready frame available */ @@ -157,7 +173,6 @@ static int tx_st30p_next_frame(void* priv, uint16_t* next_frame_idx, /* not any ready frame */ if (!framebuff) { - mt_pthread_mutex_unlock(&ctx->lock); /* When drop-when-late is active, ensure the app knows about free slots so it * can refill the pipeline promptly after drops freed frames. */ if (ctx->ops.flags & ST30P_TX_FLAG_DROP_WHEN_LATE) { @@ -167,7 +182,17 @@ static int tx_st30p_next_frame(void* priv, uint16_t* next_frame_idx, return -EBUSY; } - framebuff->stat = ST30P_TX_FRAME_IN_TRANSMITTING; + /* Atomically claim READY -> IN_TRANSMITTING. A concurrent transport thread + * may have already taken this slot between the scan and now; on a lost + * race return -EBUSY and let the caller (transport loop) retry. */ + { + uint32_t expected_stat = ST30P_TX_FRAME_READY; + if (!atomic_compare_exchange_strong_explicit( + &framebuff->stat, &expected_stat, ST30P_TX_FRAME_IN_TRANSMITTING, + memory_order_acq_rel, memory_order_relaxed)) { + return -EBUSY; + } + } *next_frame_idx = framebuff->idx; if (ctx->ops.flags & (ST30P_TX_FLAG_USER_PACING)) { @@ -176,7 +201,6 @@ static int tx_st30p_next_frame(void* priv, uint16_t* next_frame_idx, meta->timestamp = frame->timestamp; } - mt_pthread_mutex_unlock(&ctx->lock); dbg("%s(%d), frame %u succ\n", __func__, ctx->idx, framebuff->idx); MT_USDT_ST30P_TX_FRAME_NEXT(ctx->idx, framebuff->idx); @@ -195,17 +219,16 @@ static int tx_st30p_frame_done(void* priv, uint16_t frame_idx, frame->epoch = meta->epoch; frame->rtp_timestamp = meta->rtp_timestamp; - mt_pthread_mutex_lock(&ctx->lock); - if (ST30P_TX_FRAME_IN_TRANSMITTING == framebuff->stat) { + if (ST30P_TX_FRAME_IN_TRANSMITTING == + atomic_load_explicit(&framebuff->stat, memory_order_acquire)) { ret = 0; - framebuff->stat = ST30P_TX_FRAME_FREE; + atomic_store_explicit(&framebuff->stat, ST30P_TX_FRAME_FREE, memory_order_release); dbg("%s(%d), done_idx %u\n", __func__, ctx->idx, frame_idx); } else { ret = -EIO; - err("%s(%d), err status %d for frame %u\n", __func__, ctx->idx, framebuff->stat, - frame_idx); + err("%s(%d), err status %d for frame %u\n", __func__, ctx->idx, + (int)atomic_load_explicit(&framebuff->stat, memory_order_relaxed), frame_idx); } - mt_pthread_mutex_unlock(&ctx->lock); if (ctx->ops.notify_frame_done && !framebuff->frame_done_cb_called) { /* notify app which frame done */ @@ -213,7 +236,8 @@ static int tx_st30p_frame_done(void* priv, uint16_t frame_idx, ctx->ops.notify_frame_done(ctx->ops.priv, frame); framebuff->frame_done_cb_called = true; } - if (ret == 0) __atomic_fetch_add(&ctx->stat_frames_sent, 1, __ATOMIC_RELAXED); + if (ret == 0) + atomic_fetch_add_explicit(&ctx->stat_frames_sent, 1, memory_order_relaxed); /* notify app can get frame */ tx_st30p_notify_frame_available(ctx); @@ -324,7 +348,7 @@ static int tx_st30p_init_fbs(struct st30p_tx_ctx* ctx, struct st30p_tx_ops* ops) struct st30p_tx_frame* framebuff = &frames[i]; struct st30_frame* frame = &framebuff->frame; - framebuff->stat = ST30P_TX_FRAME_FREE; + atomic_store_explicit(&framebuff->stat, ST30P_TX_FRAME_FREE, memory_order_relaxed); framebuff->idx = i; /* addr will be resolved later in tx_st30p_create_transport */ @@ -472,30 +496,34 @@ struct st30_frame* st30p_tx_get_frame(st30p_tx_handle handle) { ctx->stat_get_frame_try++; - mt_pthread_mutex_lock(&ctx->lock); - framebuff = tx_st30p_next_available(ctx, ST30P_TX_FRAME_FREE); + /* Claim FREE->IN_USER. tx_st30p_claim_available() retries across the ring + * on a lost CAS race, so it only returns NULL once every slot has actually + * been checked -- unlike a scan-then-single-CAS-attempt, which could give up + * even while other FREE frames remain (spurious failure under contention). */ + framebuff = tx_st30p_claim_available(ctx, ST30P_TX_FRAME_FREE, ST30P_TX_FRAME_IN_USER); if (!framebuff && ctx->block_get) { /* wait here */ - mt_pthread_mutex_unlock(&ctx->lock); mt_pthread_mutex_lock(&ctx->block_wake_mutex); - if (!__atomic_load_n(&ctx->lc_destroying, __ATOMIC_ACQUIRE)) - mt_pthread_cond_timedwait_ns(&ctx->block_wake_cond, &ctx->block_wake_mutex, - ctx->block_timeout_ns); + while (!ctx->block_wake_pending && + !atomic_load_explicit(&ctx->lc_destroying, memory_order_acquire)) { + int _ret = mt_pthread_cond_timedwait_ns( + &ctx->block_wake_cond, &ctx->block_wake_mutex, ctx->block_timeout_ns); + if (_ret) break; + } + ctx->block_wake_pending = false; mt_pthread_mutex_unlock(&ctx->block_wake_mutex); - if (__atomic_load_n(&ctx->lc_destroying, __ATOMIC_ACQUIRE)) goto out; + if (atomic_load_explicit(&ctx->lc_destroying, memory_order_acquire)) goto out; /* get again */ - mt_pthread_mutex_lock(&ctx->lock); - framebuff = tx_st30p_next_available(ctx, ST30P_TX_FRAME_FREE); + framebuff = + tx_st30p_claim_available(ctx, ST30P_TX_FRAME_FREE, ST30P_TX_FRAME_IN_USER); } /* not any free frame */ if (!framebuff) { - mt_pthread_mutex_unlock(&ctx->lock); goto out; } - framebuff->stat = ST30P_TX_FRAME_IN_USER; framebuff->frame_done_cb_called = false; - framebuff->seq_number = ctx->framebuff_seq_number++; - mt_pthread_mutex_unlock(&ctx->lock); + framebuff->seq_number = + atomic_fetch_add_explicit(&ctx->framebuff_seq_number, 1, memory_order_relaxed); frame = &framebuff->frame; ctx->stat_get_frame_succ++; @@ -521,20 +549,18 @@ int st30p_tx_put_frame(st30p_tx_handle handle, struct st30_frame* frame) { MT_HANDLE_GUARD(ctx, MT_ST30_HANDLE_PIPELINE_TX, -EIO); - mt_pthread_mutex_lock(&ctx->lock); - if (ST30P_TX_FRAME_IN_USER != framebuff->stat) { + if (ST30P_TX_FRAME_IN_USER != + atomic_load_explicit(&framebuff->stat, memory_order_acquire)) { err("%s(%d), frame %u not in user %d\n", __func__, idx, producer_idx, - framebuff->stat); - mt_pthread_mutex_unlock(&ctx->lock); + (int)atomic_load_explicit(&framebuff->stat, memory_order_relaxed)); ret = -EIO; goto out; } - framebuff->stat = ST30P_TX_FRAME_READY; + atomic_store_explicit(&framebuff->stat, ST30P_TX_FRAME_READY, memory_order_release); ctx->stat_put_frame++; MT_USDT_ST30P_TX_FRAME_PUT(idx, framebuff->idx, frame->addr); dbg("%s(%d), frame %u(%p) succ\n", __func__, idx, producer_idx, frame->addr); - mt_pthread_mutex_unlock(&ctx->lock); ret = 0; out: MT_HANDLE_RELEASE(ctx); @@ -550,20 +576,18 @@ int st30p_tx_put_frame_abort(st30p_tx_handle handle, struct st30_frame* frame) { MT_HANDLE_GUARD(ctx, MT_ST30_HANDLE_PIPELINE_TX, -EIO); - mt_pthread_mutex_lock(&ctx->lock); - if (ST30P_TX_FRAME_IN_USER != framebuff->stat) { + if (ST30P_TX_FRAME_IN_USER != + atomic_load_explicit(&framebuff->stat, memory_order_acquire)) { err("%s(%d), frame %u not in user %d\n", __func__, idx, producer_idx, - framebuff->stat); - mt_pthread_mutex_unlock(&ctx->lock); + (int)atomic_load_explicit(&framebuff->stat, memory_order_relaxed)); ret = -EIO; goto out; } - framebuff->stat = ST30P_TX_FRAME_FREE; + atomic_store_explicit(&framebuff->stat, ST30P_TX_FRAME_FREE, memory_order_release); ctx->stat_drop_frame++; - __atomic_fetch_add(&ctx->stat_frames_dropped, 1, __ATOMIC_RELAXED); + atomic_fetch_add_explicit(&ctx->stat_frames_dropped, 1, memory_order_relaxed); dbg("%s(%d), frame %u aborted\n", __func__, idx, producer_idx); - mt_pthread_mutex_unlock(&ctx->lock); ret = 0; out: MT_HANDLE_RELEASE(ctx); @@ -601,7 +625,6 @@ int st30p_tx_free(st30p_tx_handle handle) { tx_st30p_usdt_dump_close(ctx); - mt_pthread_mutex_destroy(&ctx->lock); mt_pthread_mutex_destroy(&ctx->block_wake_mutex); mt_pthread_cond_destroy(&ctx->block_wake_cond); notice("%s(%d), succ\n", __func__, ctx->idx); @@ -652,7 +675,6 @@ st30p_tx_handle st30p_tx_create(mtl_handle mt, struct st30p_tx_ops* ops) { ctx->type = MT_ST30_HANDLE_PIPELINE_TX; ctx->wake_on_destroy = (void (*)(void*))tx_st30p_block_wake; ctx->usdt_dump_fd = -1; - mt_pthread_mutex_init(&ctx->lock, NULL); mt_pthread_mutex_init(&ctx->block_wake_mutex, NULL); mt_pthread_cond_wait_init(&ctx->block_wake_cond); @@ -798,9 +820,9 @@ int st30p_tx_get_session_stats(st30p_tx_handle handle, struct st30_tx_user_stats if (ret < 0) goto out; /* Overlay pipeline-tracked frame-level counters; transport never sets these. */ stats->common.stat_frames_sent = - __atomic_load_n(&ctx->stat_frames_sent, __ATOMIC_RELAXED); + atomic_load_explicit(&ctx->stat_frames_sent, memory_order_relaxed); stats->common.stat_frames_dropped = - __atomic_load_n(&ctx->stat_frames_dropped, __ATOMIC_RELAXED); + atomic_load_explicit(&ctx->stat_frames_dropped, memory_order_relaxed); ret = 0; out: MT_HANDLE_RELEASE(ctx); @@ -818,8 +840,8 @@ int st30p_tx_reset_session_stats(st30p_tx_handle handle) { MT_HANDLE_GUARD(ctx, MT_ST30_HANDLE_PIPELINE_TX, 0); - __atomic_store_n(&ctx->stat_frames_sent, 0, __ATOMIC_RELAXED); - __atomic_store_n(&ctx->stat_frames_dropped, 0, __ATOMIC_RELAXED); + atomic_store_explicit(&ctx->stat_frames_sent, 0, memory_order_relaxed); + atomic_store_explicit(&ctx->stat_frames_dropped, 0, memory_order_relaxed); ret = st30_tx_reset_session_stats(ctx->transport); MT_HANDLE_RELEASE(ctx); return ret; diff --git a/lib/src/st2110/pipeline/st30_pipeline_tx.h b/lib/src/st2110/pipeline/st30_pipeline_tx.h index e0a7e9c1a..c7d23875e 100644 --- a/lib/src/st2110/pipeline/st30_pipeline_tx.h +++ b/lib/src/st2110/pipeline/st30_pipeline_tx.h @@ -23,7 +23,7 @@ enum st30p_tx_frame_status { }; struct st30p_tx_frame { - enum st30p_tx_frame_status stat; + _Atomic uint32_t stat; struct st30_frame frame; uint16_t idx; uint32_t seq_number; @@ -45,9 +45,8 @@ struct st30p_tx_ctx { st30_tx_handle transport; uint16_t framebuff_cnt; - uint32_t framebuff_seq_number; + _Atomic uint32_t framebuff_seq_number; struct st30p_tx_frame* framebuffs; - pthread_mutex_t lock; bool ready; /* usdt dump */ @@ -61,6 +60,7 @@ struct st30p_tx_ctx { pthread_cond_t block_wake_cond; pthread_mutex_t block_wake_mutex; uint64_t block_timeout_ns; + bool block_wake_pending; /* get frame stat */ uint32_t stat_get_frame_try; diff --git a/lib/src/st2110/pipeline/st40_pipeline_rx.c b/lib/src/st2110/pipeline/st40_pipeline_rx.c index ef6ad9c60..0cee229cf 100644 --- a/lib/src/st2110/pipeline/st40_pipeline_rx.c +++ b/lib/src/st2110/pipeline/st40_pipeline_rx.c @@ -53,7 +53,7 @@ static struct st40p_rx_frame* rx_st40p_next_available( /* check ready frame from idx_start */ while (1) { framebuff = &ctx->framebuffs[idx]; - if (desired == framebuff->stat) { + if (desired == atomic_load_explicit(&framebuff->stat, memory_order_acquire)) { /* find one desired */ return framebuff; } @@ -68,6 +68,27 @@ static struct st40p_rx_frame* rx_st40p_next_available( return NULL; } +/* Scan from idx_start for a framebuff in state `desired` and atomically claim + * it by transitioning it to `claimed`. A concurrent thread can win the race on + * the scanned candidate between the scan and the claim; on a lost race this + * keeps scanning instead of giving up, so the caller only sees NULL once every + * slot has genuinely been checked and found unavailable. */ +static struct st40p_rx_frame* rx_st40p_claim_available( + struct st40p_rx_ctx* ctx, uint16_t idx_start, enum st40p_rx_frame_status desired, + enum st40p_rx_frame_status claimed) { + struct st40p_rx_frame* framebuff; + + while ((framebuff = rx_st40p_next_available(ctx, idx_start, desired))) { + uint32_t expected = desired; + if (atomic_compare_exchange_strong_explicit(&framebuff->stat, &expected, claimed, + memory_order_acq_rel, + memory_order_relaxed)) + return framebuff; + } + + return NULL; +} + /* FRAME_LEVEL transport delivery callback. Transport hands us a fully * assembled frame (UDW buffer + meta). We claim a FREE pipeline framebuff, * point it at the transport-owned UDW buffer (zero-copy) and copy meta into @@ -79,13 +100,11 @@ static int rx_st40p_frame_ready(void* priv, void* addr, struct st40_rx_frame_met if (!ctx->ready) return -EBUSY; - mt_pthread_mutex_lock(&ctx->lock); framebuff = rx_st40p_next_available(ctx, ctx->framebuff_producer_idx, ST40P_RX_FRAME_FREE); if (!framebuff) { ctx->stat_busy++; - __atomic_fetch_add(&ctx->stat_frames_dropped, 1, __ATOMIC_RELAXED); - mt_pthread_mutex_unlock(&ctx->lock); + atomic_fetch_add_explicit(&ctx->stat_frames_dropped, 1, memory_order_relaxed); /* returning <0 makes the transport reclaim the slot to FREE */ return -EBUSY; } @@ -116,12 +135,11 @@ static int rx_st40p_frame_ready(void* priv, void* addr, struct st40_rx_frame_met frame_info->epoch = 0; frame_info->status = meta->status; - framebuff->stat = ST40P_RX_FRAME_READY; + atomic_store_explicit(&framebuff->stat, ST40P_RX_FRAME_READY, memory_order_release); ctx->framebuff_producer_idx = rx_st40p_next_idx(ctx, framebuff->idx); - __atomic_fetch_add(&ctx->stat_frames_received, 1, __ATOMIC_RELAXED); + atomic_fetch_add_explicit(&ctx->stat_frames_received, 1, memory_order_relaxed); if (frame_info->seq_discont) - __atomic_fetch_add(&ctx->stat_frames_corrupted, 1, __ATOMIC_RELAXED); - mt_pthread_mutex_unlock(&ctx->lock); + atomic_fetch_add_explicit(&ctx->stat_frames_corrupted, 1, memory_order_relaxed); rx_st40p_notify_frame_available(ctx); MT_USDT_ST40P_RX_FRAME_AVAILABLE(ctx->idx, framebuff->idx, frame_info->meta_num); @@ -205,7 +223,7 @@ static int rx_st40p_init_fbs(struct st40p_rx_ctx* ctx, struct st40p_rx_ops* ops) for (uint16_t i = 0; i < ctx->framebuff_cnt; i++) { framebuff = &frames[i]; frame_info = &framebuff->frame_info; - framebuff->stat = ST40P_RX_FRAME_FREE; + atomic_store_explicit(&framebuff->stat, ST40P_RX_FRAME_FREE, memory_order_relaxed); framebuff->idx = i; /* udw_buff_addr is bound at frame_ready time to the transport's pool @@ -250,12 +268,10 @@ static int rx_st40p_stat(void* priv) { enum st40p_rx_frame_status producer_stat; enum st40p_rx_frame_status consumer_stat; - mt_pthread_mutex_lock(&ctx->lock); producer_idx = ctx->framebuff_producer_idx; consumer_idx = ctx->framebuff_consumer_idx; producer_stat = framebuff[producer_idx].stat; consumer_stat = framebuff[consumer_idx].stat; - mt_pthread_mutex_unlock(&ctx->lock); notice("RX_st40p(%d,%s), p(%d:%s) c(%d:%s)\n", ctx->idx, ctx->ops_name, producer_idx, rx_st40p_stat_name(producer_stat), consumer_idx, @@ -320,38 +336,34 @@ struct st40_frame_info* st40p_rx_get_frame(st40p_rx_handle handle) { ctx->stat_get_frame_try++; - mt_pthread_mutex_lock(&ctx->lock); - - framebuff = - rx_st40p_next_available(ctx, ctx->framebuff_consumer_idx, ST40P_RX_FRAME_READY); + /* Claim READY->IN_USER. rx_st40p_claim_available() retries across the ring + * on a lost CAS race, so it only returns NULL once every slot has actually + * been checked -- unlike a scan-then-single-CAS-attempt, which could give up + * even while other READY frames remain (spurious failure under contention). */ + framebuff = rx_st40p_claim_available(ctx, ctx->framebuff_consumer_idx, + ST40P_RX_FRAME_READY, ST40P_RX_FRAME_IN_USER); if (!framebuff && ctx->block_get) { /* wait here */ - mt_pthread_mutex_unlock(&ctx->lock); mt_pthread_mutex_lock(&ctx->block_wake_mutex); while (!ctx->block_wake_pending && - !__atomic_load_n(&ctx->lc_destroying, __ATOMIC_ACQUIRE)) { + !atomic_load_explicit(&ctx->lc_destroying, memory_order_acquire)) { int _ret = mt_pthread_cond_timedwait_ns( &ctx->block_wake_cond, &ctx->block_wake_mutex, ctx->block_timeout_ns); if (_ret) break; } ctx->block_wake_pending = false; mt_pthread_mutex_unlock(&ctx->block_wake_mutex); - if (__atomic_load_n(&ctx->lc_destroying, __ATOMIC_ACQUIRE)) goto out; + if (atomic_load_explicit(&ctx->lc_destroying, memory_order_acquire)) goto out; /* get again */ - mt_pthread_mutex_lock(&ctx->lock); - framebuff = - rx_st40p_next_available(ctx, ctx->framebuff_consumer_idx, ST40P_RX_FRAME_READY); + framebuff = rx_st40p_claim_available(ctx, ctx->framebuff_consumer_idx, + ST40P_RX_FRAME_READY, ST40P_RX_FRAME_IN_USER); } /* not any ready frame */ if (!framebuff) { - mt_pthread_mutex_unlock(&ctx->lock); goto out; } - - framebuff->stat = ST40P_RX_FRAME_IN_USER; - /* point to next */ + /* point to next (best-effort hint; the CAS above is the real guard) */ ctx->framebuff_consumer_idx = rx_st40p_next_idx(ctx, framebuff->idx); - mt_pthread_mutex_unlock(&ctx->lock); frame_info = &framebuff->frame_info; ctx->stat_get_frame_succ++; @@ -379,9 +391,10 @@ int st40p_rx_put_frame(st40p_rx_handle handle, struct st40_frame_info* frame_inf MT_HANDLE_GUARD(ctx, MT_ST40_HANDLE_PIPELINE_RX, -EIO); - if (ST40P_RX_FRAME_IN_USER != framebuff->stat) { + if (ST40P_RX_FRAME_IN_USER != + atomic_load_explicit(&framebuff->stat, memory_order_acquire)) { err("%s(%d), frame %u not in user %d\n", __func__, idx, consumer_idx, - framebuff->stat); + (int)atomic_load_explicit(&framebuff->stat, memory_order_relaxed)); ret = -EIO; goto out; } @@ -408,7 +421,7 @@ int st40p_rx_put_frame(st40p_rx_handle handle, struct st40_frame_info* frame_inf frame_info->receive_timestamp = 0; frame_info->second_field = false; frame_info->interlaced = false; - framebuff->stat = ST40P_RX_FRAME_FREE; + atomic_store_explicit(&framebuff->stat, ST40P_RX_FRAME_FREE, memory_order_release); ctx->stat_put_frame++; MT_USDT_ST40P_RX_FRAME_PUT(idx, consumer_idx, meta_num_before_reset); @@ -428,9 +441,10 @@ int st40p_rx_put_frame_abort(st40p_rx_handle handle, struct st40_frame_info* fra MT_HANDLE_GUARD(ctx, MT_ST40_HANDLE_PIPELINE_RX, -EIO); - if (ST40P_RX_FRAME_IN_USER != framebuff->stat) { + if (ST40P_RX_FRAME_IN_USER != + atomic_load_explicit(&framebuff->stat, memory_order_acquire)) { err("%s(%d), frame %u not in user %d\n", __func__, idx, consumer_idx, - framebuff->stat); + (int)atomic_load_explicit(&framebuff->stat, memory_order_relaxed)); ret = -EIO; goto out; } @@ -444,7 +458,7 @@ int st40p_rx_put_frame_abort(st40p_rx_handle handle, struct st40_frame_info* fra /* reset frame for reuse without processing */ frame_info->meta_num = 0; frame_info->udw_buffer_fill = 0; - framebuff->stat = ST40P_RX_FRAME_FREE; + atomic_store_explicit(&framebuff->stat, ST40P_RX_FRAME_FREE, memory_order_release); dbg("%s(%d), frame %u aborted\n", __func__, idx, consumer_idx); ret = 0; out: @@ -485,7 +499,6 @@ int st40p_rx_free(st40p_rx_handle handle) { rx_st40p_uinit_fbs(ctx); - mt_pthread_mutex_destroy(&ctx->lock); mt_pthread_mutex_destroy(&ctx->block_wake_mutex); mt_pthread_cond_destroy(&ctx->block_wake_cond); notice("%s(%d), succ\n", __func__, ctx->idx); @@ -540,7 +553,6 @@ st40p_rx_handle st40p_rx_create(mtl_handle mt, struct st40p_rx_ops* ops) { ctx->port_id[i] = UINT16_MAX; } - mt_pthread_mutex_init(&ctx->lock, NULL); mt_pthread_mutex_init(&ctx->block_wake_mutex, NULL); mt_pthread_cond_wait_init(&ctx->block_wake_cond); ctx->block_timeout_ns = NS_PER_S; @@ -621,11 +633,11 @@ int st40p_rx_get_session_stats(st40p_rx_handle handle, struct st40_rx_user_stats if (ret < 0) goto out; /* Overlay pipeline-tracked frame-level counters; transport never sets these. */ stats->common.stat_frames_received = - __atomic_load_n(&ctx->stat_frames_received, __ATOMIC_RELAXED); + atomic_load_explicit(&ctx->stat_frames_received, memory_order_relaxed); stats->common.stat_frames_dropped = - __atomic_load_n(&ctx->stat_frames_dropped, __ATOMIC_RELAXED); + atomic_load_explicit(&ctx->stat_frames_dropped, memory_order_relaxed); stats->common.stat_frames_corrupted = - __atomic_load_n(&ctx->stat_frames_corrupted, __ATOMIC_RELAXED); + atomic_load_explicit(&ctx->stat_frames_corrupted, memory_order_relaxed); ret = 0; out: MT_HANDLE_RELEASE(ctx); @@ -643,9 +655,9 @@ int st40p_rx_reset_session_stats(st40p_rx_handle handle) { MT_HANDLE_GUARD(ctx, MT_ST40_HANDLE_PIPELINE_RX, -EIO); - __atomic_store_n(&ctx->stat_frames_received, 0, __ATOMIC_RELAXED); - __atomic_store_n(&ctx->stat_frames_dropped, 0, __ATOMIC_RELAXED); - __atomic_store_n(&ctx->stat_frames_corrupted, 0, __ATOMIC_RELAXED); + atomic_store_explicit(&ctx->stat_frames_received, 0, memory_order_relaxed); + atomic_store_explicit(&ctx->stat_frames_dropped, 0, memory_order_relaxed); + atomic_store_explicit(&ctx->stat_frames_corrupted, 0, memory_order_relaxed); ret = st40_rx_reset_session_stats(ctx->transport); MT_HANDLE_RELEASE(ctx); return ret; diff --git a/lib/src/st2110/pipeline/st40_pipeline_rx.h b/lib/src/st2110/pipeline/st40_pipeline_rx.h index 852b30ebd..3060cce51 100644 --- a/lib/src/st2110/pipeline/st40_pipeline_rx.h +++ b/lib/src/st2110/pipeline/st40_pipeline_rx.h @@ -41,7 +41,6 @@ struct st40p_rx_ctx { uint16_t framebuff_producer_idx; uint16_t framebuff_consumer_idx; struct st40p_rx_frame* framebuffs; - pthread_mutex_t lock; bool ready; /* for ST40P_RX_FLAG_BLOCK_GET */ @@ -67,7 +66,7 @@ struct st40p_rx_ctx { }; struct st40p_rx_frame { - enum st40p_rx_frame_status stat; + _Atomic uint32_t stat; struct st40_frame_info frame_info; struct st40_meta meta[ST40_MAX_META]; uint16_t idx; diff --git a/lib/src/st2110/pipeline/st40_pipeline_tx.c b/lib/src/st2110/pipeline/st40_pipeline_tx.c index 5f4a28e19..100a3e7a9 100644 --- a/lib/src/st2110/pipeline/st40_pipeline_tx.c +++ b/lib/src/st2110/pipeline/st40_pipeline_tx.c @@ -23,6 +23,7 @@ static const char* tx_st40p_stat_name(enum st40p_tx_frame_status stat) { static void tx_st40p_block_wake(struct st40p_tx_ctx* ctx) { /* notify block */ mt_pthread_mutex_lock(&ctx->block_wake_mutex); + ctx->block_wake_pending = true; mt_pthread_cond_signal(&ctx->block_wake_cond); mt_pthread_mutex_unlock(&ctx->block_wake_mutex); } @@ -62,7 +63,7 @@ static struct st40p_tx_frame* tx_st40p_next_available( /* check ready frame from idx_start */ for (uint16_t idx = 0; idx < ctx->framebuff_cnt; idx++) { framebuff = &ctx->framebuffs[idx]; - if (desired == framebuff->stat) { + if (desired == atomic_load_explicit(&framebuff->stat, memory_order_acquire)) { return framebuff; } } @@ -71,6 +72,27 @@ static struct st40p_tx_frame* tx_st40p_next_available( return NULL; } +/* Scan for a framebuff in state `desired` and atomically claim it by + * transitioning it to `claimed`. A concurrent thread can win the race on the + * scanned candidate between the scan and the claim; on a lost race this keeps + * scanning instead of giving up, so the caller only sees NULL once every slot + * has genuinely been checked and found unavailable. */ +static struct st40p_tx_frame* tx_st40p_claim_available( + struct st40p_tx_ctx* ctx, enum st40p_tx_frame_status desired, + enum st40p_tx_frame_status claimed) { + struct st40p_tx_frame* framebuff; + + while ((framebuff = tx_st40p_next_available(ctx, desired))) { + uint32_t expected = desired; + if (atomic_compare_exchange_strong_explicit(&framebuff->stat, &expected, claimed, + memory_order_acq_rel, + memory_order_relaxed)) + return framebuff; + } + + return NULL; +} + /* Check if the newest READY frame has missed its transmission window. * Drops the frame (READY -> DROPPED -> FREE) if cur_tai > frame_tai + frame_period. * @@ -106,11 +128,9 @@ static bool tx_st40p_if_frame_late(struct st40p_tx_ctx* ctx, ctx->stat_drop_frame++; /* relaxed atomic: written from both tasklet (late drop) and user thread * (put_frame_abort), and read from any thread via st40p_tx_get_session_stats(). */ - __atomic_fetch_add(&ctx->stat_frames_dropped, 1, __ATOMIC_RELAXED); + atomic_fetch_add_explicit(&ctx->stat_frames_dropped, 1, memory_order_relaxed); rtp_ts = frame_info->rtp_timestamp; - framebuff->stat = ST40P_TX_FRAME_DROPPED; - - mt_pthread_mutex_unlock(&ctx->lock); + atomic_store_explicit(&framebuff->stat, ST40P_TX_FRAME_DROPPED, memory_order_release); dbg("%s(%d), frame %u drop late by %" PRIu64 "ns (> period %" PRIu64 "ns), cur %" PRIu64 " frame %" PRIu64 "\n", @@ -126,13 +146,10 @@ static bool tx_st40p_if_frame_late(struct st40p_tx_ctx* ctx, if (ctx->ops.notify_frame_late) ctx->ops.notify_frame_late(ctx->ops.priv, 0); MT_USDT_ST40P_TX_FRAME_DROP(ctx->idx, framebuff->idx, rtp_ts); - mt_pthread_mutex_lock(&ctx->lock); - framebuff->stat = ST40P_TX_FRAME_FREE; - mt_pthread_mutex_unlock(&ctx->lock); + atomic_store_explicit(&framebuff->stat, ST40P_TX_FRAME_FREE, memory_order_release); tx_st40p_notify_frame_available(ctx); - mt_pthread_mutex_lock(&ctx->lock); return true; /* frame was dropped, caller should retry */ } @@ -145,7 +162,6 @@ static int tx_st40p_next_frame(void* priv, uint16_t* next_frame_idx, if (!ctx->ready) return -EBUSY; /* not ready */ - mt_pthread_mutex_lock(&ctx->lock); do { framebuff = tx_st40p_newest_available(ctx, ST40P_TX_FRAME_READY); if (!framebuff) break; /* no ready frame available */ @@ -159,7 +175,6 @@ static int tx_st40p_next_frame(void* priv, uint16_t* next_frame_idx, /* not any ready frame */ if (!framebuff) { - mt_pthread_mutex_unlock(&ctx->lock); /* When drop-when-late is active, ensure the app knows about free slots so it * can refill the pipeline promptly after drops freed frames. */ if (ctx->ops.flags & ST40P_TX_FLAG_DROP_WHEN_LATE) { @@ -169,7 +184,17 @@ static int tx_st40p_next_frame(void* priv, uint16_t* next_frame_idx, return -EBUSY; } - framebuff->stat = ST40P_TX_FRAME_IN_TRANSMITTING; + /* Atomically claim READY -> IN_TRANSMITTING. A concurrent transport thread + * may have already taken this slot between the scan and now; on a lost + * race return -EBUSY and let the caller (transport loop) retry. */ + { + uint32_t expected_stat = ST40P_TX_FRAME_READY; + if (!atomic_compare_exchange_strong_explicit( + &framebuff->stat, &expected_stat, ST40P_TX_FRAME_IN_TRANSMITTING, + memory_order_acq_rel, memory_order_relaxed)) { + return -EBUSY; + } + } *next_frame_idx = framebuff->idx; if (ctx->ops.flags & (ST40P_TX_FLAG_USER_PACING)) { @@ -177,7 +202,6 @@ static int tx_st40p_next_frame(void* priv, uint16_t* next_frame_idx, meta->timestamp = framebuff->frame_info.timestamp; } - mt_pthread_mutex_unlock(&ctx->lock); dbg("%s(%d), frame %u succ\n", __func__, ctx->idx, framebuff->idx); MT_USDT_ST40P_TX_FRAME_NEXT(ctx->idx, framebuff->idx); @@ -201,17 +225,16 @@ static int tx_st40p_frame_done(void* priv, uint16_t frame_idx, frame_info->interlaced = ctx->ops.interlaced; frame_info->second_field = ctx->ops.interlaced ? meta->second_field : false; - mt_pthread_mutex_lock(&ctx->lock); - if (ST40P_TX_FRAME_IN_TRANSMITTING == framebuff->stat) { + if (ST40P_TX_FRAME_IN_TRANSMITTING == + atomic_load_explicit(&framebuff->stat, memory_order_acquire)) { ret = 0; - framebuff->stat = ST40P_TX_FRAME_FREE; + atomic_store_explicit(&framebuff->stat, ST40P_TX_FRAME_FREE, memory_order_release); dbg("%s(%d), done_idx %u\n", __func__, ctx->idx, frame_idx); } else { ret = -EIO; - err("%s(%d), err status %d for frame %u\n", __func__, ctx->idx, framebuff->stat, - frame_idx); + err("%s(%d), err status %d for frame %u\n", __func__, ctx->idx, + (int)atomic_load_explicit(&framebuff->stat, memory_order_relaxed), frame_idx); } - mt_pthread_mutex_unlock(&ctx->lock); if (ctx->ops.notify_frame_done && !framebuff->frame_done_cb_called) { /* notify app which frame done */ @@ -219,7 +242,8 @@ static int tx_st40p_frame_done(void* priv, uint16_t frame_idx, ctx->ops.notify_frame_done(ctx->ops.priv, frame_info); framebuff->frame_done_cb_called = true; } - if (ret == 0) __atomic_fetch_add(&ctx->stat_frames_sent, 1, __ATOMIC_RELAXED); + if (ret == 0) + atomic_fetch_add_explicit(&ctx->stat_frames_sent, 1, memory_order_relaxed); /* notify app can get frame */ tx_st40p_notify_frame_available(ctx); @@ -353,7 +377,7 @@ static int tx_st40p_init_fbs(struct st40p_tx_ctx* ctx, struct st40p_tx_ops* ops) for (uint16_t i = 0; i < ctx->framebuff_cnt; i++) { framebuff = &frames[i]; frame_info = &framebuff->frame_info; - framebuff->stat = ST40P_TX_FRAME_FREE; + atomic_store_explicit(&framebuff->stat, ST40P_TX_FRAME_FREE, memory_order_relaxed); framebuff->idx = i; frame_info->udw_buff_addr = mt_rte_zmalloc_socket(ops->max_udw_buff_size, soc_id); @@ -464,31 +488,35 @@ struct st40_frame_info* st40p_tx_get_frame(st40p_tx_handle handle) { ctx->stat_get_frame_try++; - mt_pthread_mutex_lock(&ctx->lock); - framebuff = tx_st40p_next_available(ctx, ST40P_TX_FRAME_FREE); + /* Claim FREE->IN_USER. tx_st40p_claim_available() retries across the ring + * on a lost CAS race, so it only returns NULL once every slot has actually + * been checked -- unlike a scan-then-single-CAS-attempt, which could give up + * even while other FREE frames remain (spurious failure under contention). */ + framebuff = tx_st40p_claim_available(ctx, ST40P_TX_FRAME_FREE, ST40P_TX_FRAME_IN_USER); if (!framebuff && ctx->block_get) { /* wait here */ - mt_pthread_mutex_unlock(&ctx->lock); mt_pthread_mutex_lock(&ctx->block_wake_mutex); - if (!__atomic_load_n(&ctx->lc_destroying, __ATOMIC_ACQUIRE)) - mt_pthread_cond_timedwait_ns(&ctx->block_wake_cond, &ctx->block_wake_mutex, - ctx->block_timeout_ns); + while (!ctx->block_wake_pending && + !atomic_load_explicit(&ctx->lc_destroying, memory_order_acquire)) { + int _ret = mt_pthread_cond_timedwait_ns( + &ctx->block_wake_cond, &ctx->block_wake_mutex, ctx->block_timeout_ns); + if (_ret) break; + } + ctx->block_wake_pending = false; mt_pthread_mutex_unlock(&ctx->block_wake_mutex); - if (__atomic_load_n(&ctx->lc_destroying, __ATOMIC_ACQUIRE)) goto out; + if (atomic_load_explicit(&ctx->lc_destroying, memory_order_acquire)) goto out; /* get again */ - mt_pthread_mutex_lock(&ctx->lock); - framebuff = tx_st40p_next_available(ctx, ST40P_TX_FRAME_FREE); + framebuff = + tx_st40p_claim_available(ctx, ST40P_TX_FRAME_FREE, ST40P_TX_FRAME_IN_USER); } /* not any free frame */ if (!framebuff) { - mt_pthread_mutex_unlock(&ctx->lock); goto out; } - framebuff->stat = ST40P_TX_FRAME_IN_USER; framebuff->frame_done_cb_called = false; - framebuff->seq_number = ctx->framebuff_seq_number++; - mt_pthread_mutex_unlock(&ctx->lock); + framebuff->seq_number = + atomic_fetch_add_explicit(&ctx->framebuff_seq_number, 1, memory_order_relaxed); frame_info = &framebuff->frame_info; ctx->stat_get_frame_succ++; @@ -508,9 +536,10 @@ int st40p_tx_put_frame(st40p_tx_handle handle, struct st40_frame_info* frame_inf MT_HANDLE_GUARD(ctx, MT_ST40_HANDLE_PIPELINE_TX, -EIO); - if (ST40P_TX_FRAME_IN_USER != framebuff->stat) { + if (ST40P_TX_FRAME_IN_USER != + atomic_load_explicit(&framebuff->stat, memory_order_acquire)) { err("%s(%d), frame %u not in user %d\n", __func__, idx, producer_idx, - framebuff->stat); + (int)atomic_load_explicit(&framebuff->stat, memory_order_relaxed)); ret = -EIO; goto out; } @@ -526,7 +555,7 @@ int st40p_tx_put_frame(st40p_tx_handle handle, struct st40_frame_info* frame_inf } framebuff->frame_info.udw_buffer_fill = 0; - framebuff->stat = ST40P_TX_FRAME_READY; + atomic_store_explicit(&framebuff->stat, ST40P_TX_FRAME_READY, memory_order_release); ctx->stat_put_frame++; MT_USDT_ST40P_TX_FRAME_PUT(idx, framebuff->idx, framebuff->anc_frame->data); dbg("%s(%d), frame %u(%p) succ\n", __func__, idx, producer_idx, framebuff->anc_frame); @@ -545,16 +574,17 @@ int st40p_tx_put_frame_abort(st40p_tx_handle handle, struct st40_frame_info* fra MT_HANDLE_GUARD(ctx, MT_ST40_HANDLE_PIPELINE_TX, -EIO); - if (ST40P_TX_FRAME_IN_USER != framebuff->stat) { + if (ST40P_TX_FRAME_IN_USER != + atomic_load_explicit(&framebuff->stat, memory_order_acquire)) { err("%s(%d), frame %u not in user %d\n", __func__, idx, producer_idx, - framebuff->stat); + (int)atomic_load_explicit(&framebuff->stat, memory_order_relaxed)); ret = -EIO; goto out; } - framebuff->stat = ST40P_TX_FRAME_FREE; + atomic_store_explicit(&framebuff->stat, ST40P_TX_FRAME_FREE, memory_order_release); ctx->stat_drop_frame++; - __atomic_fetch_add(&ctx->stat_frames_dropped, 1, __ATOMIC_RELAXED); + atomic_fetch_add_explicit(&ctx->stat_frames_dropped, 1, memory_order_relaxed); dbg("%s(%d), frame %u aborted\n", __func__, idx, producer_idx); ret = 0; out: @@ -591,7 +621,6 @@ int st40p_tx_free(st40p_tx_handle handle) { } tx_st40p_uinit_fbs(ctx); - mt_pthread_mutex_destroy(&ctx->lock); mt_pthread_mutex_destroy(&ctx->block_wake_mutex); mt_pthread_cond_destroy(&ctx->block_wake_cond); notice("%s(%d), succ\n", __func__, ctx->idx); @@ -642,7 +671,6 @@ st40p_tx_handle st40p_tx_create(mtl_handle mt, struct st40p_tx_ops* ops) { ctx->impl = impl; ctx->type = MT_ST40_HANDLE_PIPELINE_TX; ctx->wake_on_destroy = (void (*)(void*))tx_st40p_block_wake; - mt_pthread_mutex_init(&ctx->lock, NULL); mt_pthread_mutex_init(&ctx->block_wake_mutex, NULL); mt_pthread_cond_wait_init(&ctx->block_wake_cond); @@ -784,9 +812,9 @@ int st40p_tx_get_session_stats(st40p_tx_handle handle, struct st40_tx_user_stats if (ret < 0) goto out; /* Overlay pipeline-tracked frame-level counters; transport never sets these. */ stats->common.stat_frames_sent = - __atomic_load_n(&ctx->stat_frames_sent, __ATOMIC_RELAXED); + atomic_load_explicit(&ctx->stat_frames_sent, memory_order_relaxed); stats->common.stat_frames_dropped = - __atomic_load_n(&ctx->stat_frames_dropped, __ATOMIC_RELAXED); + atomic_load_explicit(&ctx->stat_frames_dropped, memory_order_relaxed); ret = 0; out: MT_HANDLE_RELEASE(ctx); @@ -804,8 +832,8 @@ int st40p_tx_reset_session_stats(st40p_tx_handle handle) { MT_HANDLE_GUARD(ctx, MT_ST40_HANDLE_PIPELINE_TX, 0); - __atomic_store_n(&ctx->stat_frames_sent, 0, __ATOMIC_RELAXED); - __atomic_store_n(&ctx->stat_frames_dropped, 0, __ATOMIC_RELAXED); + atomic_store_explicit(&ctx->stat_frames_sent, 0, memory_order_relaxed); + atomic_store_explicit(&ctx->stat_frames_dropped, 0, memory_order_relaxed); ret = st40_tx_reset_session_stats(ctx->transport); MT_HANDLE_RELEASE(ctx); return ret; diff --git a/lib/src/st2110/pipeline/st40_pipeline_tx.h b/lib/src/st2110/pipeline/st40_pipeline_tx.h index 42ea27fa1..0a2674166 100644 --- a/lib/src/st2110/pipeline/st40_pipeline_tx.h +++ b/lib/src/st2110/pipeline/st40_pipeline_tx.h @@ -41,9 +41,8 @@ struct st40p_tx_ctx { st40_tx_handle transport; uint16_t framebuff_cnt; - uint32_t framebuff_seq_number; + _Atomic uint32_t framebuff_seq_number; struct st40p_tx_frame* framebuffs; - pthread_mutex_t lock; bool ready; int frames_per_sec; @@ -53,6 +52,7 @@ struct st40p_tx_ctx { pthread_cond_t block_wake_cond; pthread_mutex_t block_wake_mutex; uint64_t block_timeout_ns; + bool block_wake_pending; /* get frame stat */ uint32_t stat_get_frame_try; @@ -65,7 +65,7 @@ struct st40p_tx_ctx { }; struct st40p_tx_frame { - enum st40p_tx_frame_status stat; + _Atomic uint32_t stat; struct st40_frame_info frame_info; uint16_t idx; /** Pointer to the main ancillary frame buffer */ diff --git a/tests/integration_tests/noctx/meson.build b/tests/integration_tests/noctx/meson.build index 37d7bcadb..9d145ad07 100644 --- a/tests/integration_tests/noctx/meson.build +++ b/tests/integration_tests/noctx/meson.build @@ -17,6 +17,7 @@ noctx_sources = files( 'testcases/st20p_redundant_tests.cpp', 'testcases/st20p_user_pacing_tests.cpp', 'testcases/st20p_user_timestamp_tests.cpp', + 'testcases/st20p_stability_tests.cpp', 'testcases/st40i_tests.cpp', 'testcases/st40p_auto_detect_tests.cpp', 'testcases/st40p_user_pacing_tests.cpp', diff --git a/tests/integration_tests/noctx/testcases/st20p_stability_tests.cpp b/tests/integration_tests/noctx/testcases/st20p_stability_tests.cpp new file mode 100644 index 000000000..f3afb03e8 --- /dev/null +++ b/tests/integration_tests/noctx/testcases/st20p_stability_tests.cpp @@ -0,0 +1,164 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright(c) 2026 Intel Corporation + * + * Long-running multi-threaded stability test for the ST20p TX pipeline + * framebuffer ring (lib/src/st2110/pipeline/st20_pipeline_tx.c). + * + * Many application producer threads pound st20p_tx_get_frame / + * st20p_tx_put_frame on ONE handle while the real transport tasklet consumes + * via the get_next_frame callback. This is the on-hardware counterpart of the + * unit-level St20PipelineConcurrency tests: it validates the same lock-free + * ownership protocol (FREE->IN_USER CAS, CONVERTED->IN_TRANSMITTING CAS) under + * a genuine DPDK transport, real pacing and real memory for a sustained + * duration. + * + * A per-framebuffer ownership token (holder[]) detects any moment where two + * actors believe they own the same slot. The buffer is written while owned so + * ASan catches any use-after-free / out-of-bounds if the protocol ever hands + * the same slot to two threads. + * + * Duration defaults to 30 minutes; override with NOCTX_STABILITY_SECONDS + * (e.g. NOCTX_STABILITY_SECONDS=30 for a smoke run). Thread and framebuffer + * counts are overridable via NOCTX_STABILITY_PRODUCERS / NOCTX_STABILITY_FBS. + */ + +#include +#include +#include +#include +#include +#include +#include + +#include "core/test_fixture.hpp" +#include "handlers/st20p_handler.hpp" +#include "strategies/st20p_strategies.hpp" + +namespace { + +int envInt(const char* name, int fallback) { + const char* v = std::getenv(name); + if (!v || !*v) return fallback; + int parsed = std::atoi(v); + return parsed > 0 ? parsed : fallback; +} + +} // namespace + +TEST_F(NoCtxTest, st20p_tx_multithread_stability) { + initDefaultContext(); + + /* Small ring + many producers = maximum contention on the claim CAS. */ + const int kFrameCnt = envInt("NOCTX_STABILITY_FBS", 4); + const int kProducers = envInt("NOCTX_STABILITY_PRODUCERS", 8); + const int kSeconds = envInt("NOCTX_STABILITY_SECONDS", 1800); /* 30 min default */ + + auto bundle = createSt20pHandlerBundle( + /*createTx=*/true, /*createRx=*/false, + [](St20pHandler* h) { return new St20pDefaultTimestamp(h); }, + [kFrameCnt](St20pHandler* h) { + h->sessionsOpsTx.framebuff_cnt = (uint16_t)kFrameCnt; + }); + + auto* handler = bundle.handler; + ASSERT_NE(handler, nullptr); + st20p_tx_handle tx = handler->sessionsHandleTx; + ASSERT_NE(tx, nullptr); + + /* Map each framebuffer address to its slot index so a returned st_frame can + * be attributed to a specific slot for ownership tracking. */ + std::unordered_map addr2idx; + for (int i = 0; i < kFrameCnt; i++) { + void* a = st20p_tx_get_fb_addr(tx, i); + ASSERT_NE(a, nullptr) << "framebuffer " << i << " has no address"; + addr2idx[a] = i; + } + + std::vector> holder(kFrameCnt); + for (auto& h : holder) h.store(0); + + const size_t frameSize = st20p_tx_frame_size(tx); + ASSERT_GT(frameSize, 0u); + + std::atomic stop{false}; + std::atomic ownership_violation{false}; + std::atomic api_error{false}; + std::atomic bad_frame{false}; + std::atomic produced{0}; + + StartFakePtpClock(); + ASSERT_GE(mtl_start(ctx->handle), 0); + + auto producer = [&](int id) { + while (!stop.load(std::memory_order_relaxed)) { + struct st_frame* f = st20p_tx_get_frame(tx); + if (!f) { + std::this_thread::sleep_for(std::chrono::microseconds(50)); + continue; + } + if (!f->addr[0]) { + bad_frame.store(true); + continue; + } + auto it = addr2idx.find(f->addr[0]); + if (it == addr2idx.end()) { + bad_frame.store(true); /* returned a buffer we never registered */ + } else { + int idx = it->second; + /* Claim the slot; a non-zero previous owner means a double-claim. */ + if (holder[idx].exchange(id) != 0) ownership_violation.store(true); + /* Write while owned: ASan flags any aliasing to another live slot. */ + std::memset(f->addr[0], (uint8_t)id, 64); + if (holder[idx].exchange(0) != id) ownership_violation.store(true); + } + f->data_size = frameSize; + if (st20p_tx_put_frame(tx, f) < 0) api_error.store(true); + produced.fetch_add(1, std::memory_order_relaxed); + } + }; + + std::vector threads; + for (int p = 1; p <= kProducers; p++) threads.emplace_back(producer, p); + + const auto start = std::chrono::steady_clock::now(); + const auto budget = std::chrono::seconds(kSeconds); + uint64_t last_report = 0; + while (std::chrono::steady_clock::now() - start < budget) { + if (ownership_violation.load() || api_error.load() || bad_frame.load() || + HasFailure()) + break; + std::this_thread::sleep_for(std::chrono::milliseconds(500)); + /* Heartbeat every ~60s so a 30-minute run shows liveness. */ + uint64_t elapsed = (uint64_t)std::chrono::duration_cast( + std::chrono::steady_clock::now() - start) + .count(); + if (elapsed - last_report >= 60) { + last_report = elapsed; + fprintf(stderr, "st20p_tx_multithread_stability: %llus, produced %llu\n", + (unsigned long long)elapsed, (unsigned long long)produced.load()); + } + } + + stop.store(true); + for (auto& t : threads) t.join(); + + EXPECT_FALSE(ownership_violation.load()) + << "two threads owned the same framebuffer simultaneously"; + EXPECT_FALSE(api_error.load()) << "st20p_tx_put_frame returned an error"; + EXPECT_FALSE(bad_frame.load()) << "get_frame returned a null/unregistered buffer"; + EXPECT_GT(produced.load(), 0u) << "no frames were produced"; + + struct st20_tx_user_stats stats; + memset(&stats, 0, sizeof(stats)); + if (st20p_tx_get_session_stats(tx, &stats) == 0) { + EXPECT_LE(stats.common.stat_frames_sent, produced.load()) + << "transport reported more sent than produced (duplicate transmit)"; + fprintf(stderr, + "st20p_tx_multithread_stability: produced %llu, sent %llu, dropped %llu\n", + (unsigned long long)produced.load(), + (unsigned long long)stats.common.stat_frames_sent, + (unsigned long long)stats.common.stat_frames_dropped); + } + + handler->stopSession(); +} diff --git a/tests/unit/meson.build b/tests/unit/meson.build index 94135cda9..0d0554835 100644 --- a/tests/unit/meson.build +++ b/tests/unit/meson.build @@ -52,10 +52,24 @@ unit_sources = [ 'session/st20/timestamp_source_test.cpp', 'pipeline/st20p_harness.c', 'pipeline/st20p_test.cpp', + 'pipeline/st20p_rx_concurrency_test.cpp', + 'pipeline/st20p_tx_harness.c', + 'pipeline/st20p_tx_concurrency_test.cpp', + 'pipeline/st20p_tx_blocking_test.cpp', + 'pipeline/st20p_concurrency_test.cpp', + 'pipeline/st20p_concurrency_stress_test.cpp', 'pipeline/st30p_harness.c', 'pipeline/st30p_test.cpp', + 'pipeline/st30p_tx_harness.c', + 'pipeline/st30p_concurrency_test.cpp', 'pipeline/st40p_harness.c', 'pipeline/st40p_test.cpp', + 'pipeline/st40p_tx_harness.c', + 'pipeline/st40p_concurrency_test.cpp', + 'pipeline/st22p_harness.c', + 'pipeline/st22p_tx_harness.c', + 'pipeline/st22p_concurrency_test.cpp', + 'pipeline/st22p_concurrency_stress_test.cpp', 'main.cpp', ] diff --git a/tests/unit/pipeline/st20p_concurrency_stress_test.cpp b/tests/unit/pipeline/st20p_concurrency_stress_test.cpp new file mode 100644 index 000000000..4e0a8d342 --- /dev/null +++ b/tests/unit/pipeline/st20p_concurrency_stress_test.cpp @@ -0,0 +1,235 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright(c) 2026 Intel Corporation + * + * Adversarial ST20p (video) TX pipeline concurrency tests: deliberately try to + * break the lock-free framebuffer ring by driving topologies harder than the + * library's normal usage. + * + * TxConcurrentConsumersNoDoubleClaim + * Runs next_frame from MANY threads at once. The claim + * CONVERTED -> IN_TRANSMITTING was a scan (newest_available) followed by a + * plain store -- NOT atomic. Two consumers can both observe the same + * CONVERTED slot and both claim it. FAILS before the claim is a CAS, + * PASSES after. Mirrors the Tx/RxConcurrentConverters CAS tests. + * + * TxMixedPutAbortNoViolation + * Full supported topology under one roof: N producers randomly complete + * (put_frame) or cancel (put_frame_abort) each slot, plus one transport + * consumer. Exercises the IN_USER -> FREE abort edge racing the + * FREE -> IN_USER reclaim, which the base suite never touches. + */ + +#include +#include +#include +#include + +#include +#include +#include +#include + +#include "pipeline/st20p_tx_harness.h" + +namespace { + +constexpr auto kRunBudget = std::chrono::seconds(45); + +inline void dwell() { + for (volatile int i = 0; i < 64; i++) { + } +} + +inline void pin_worker(std::thread& t, int slot) { + long nproc = sysconf(_SC_NPROCESSORS_ONLN); + if (nproc <= 1) return; + cpu_set_t one; + CPU_ZERO(&one); + CPU_SET(1 + (slot % (int)(nproc - 1)), &one); + pthread_setaffinity_np(t.native_handle(), sizeof(one), &one); +} + +} // namespace + +TEST(St20PipelineConcurrencyStress, TxConcurrentConsumersNoDoubleClaim) { + ASSERT_EQ(ut20p_tx_init(), 0) << "EAL init failed"; + + constexpr int kFrameCnt = 8; + constexpr int kProducers = 3; + constexpr int kConsumers = 4; + constexpr int kTarget = 50000; + + ut20p_tx_ctx* ctx = ut20p_tx_ctx_create(kFrameCnt); + ASSERT_NE(ctx, nullptr); + + /* holder[idx]: 0 free/queued, >0 owning producer id, -1 a consumer. */ + std::vector> holder(kFrameCnt); + for (auto& h : holder) h.store(0); + + std::atomic to_produce{kTarget}; + std::atomic produced{0}; + std::atomic consumed{0}; + std::atomic stop{false}; + std::atomic ownership_violation{false}; + std::atomic api_error{false}; + + auto producer = [&](int id) { + while (!stop.load(std::memory_order_relaxed)) { + if (to_produce.fetch_sub(1, std::memory_order_relaxed) <= 0) break; + struct st_frame* f = nullptr; + while ((f = ut20p_tx_get_frame(ctx)) == nullptr) { + if (stop.load(std::memory_order_relaxed)) return; + } + int idx = ut20p_tx_frame_idx(f); + if (holder[idx].exchange(id) != 0) ownership_violation.store(true); + dwell(); + if (holder[idx].exchange(0) != id) ownership_violation.store(true); + if (ut20p_tx_put_frame(ctx, f) != 0) api_error.store(true); + produced.fetch_add(1, std::memory_order_relaxed); + } + }; + + auto consumer = [&]() { + while (consumed.load(std::memory_order_relaxed) < kTarget) { + if (stop.load(std::memory_order_relaxed)) break; + uint16_t idx = 0; + if (ut20p_tx_next_frame(ctx, &idx) != 0) continue; /* nothing CONVERTED */ + /* If two consumers claimed the same slot, the second exchange sees -1. */ + if (holder[idx].exchange(-1) != 0) ownership_violation.store(true); + dwell(); + if (holder[idx].exchange(0) != -1) ownership_violation.store(true); + if (ut20p_tx_frame_done(ctx, idx) != 0) api_error.store(true); + consumed.fetch_add(1, std::memory_order_relaxed); + } + }; + + std::vector threads; + for (int c = 0; c < kConsumers; c++) threads.emplace_back(consumer); + for (int p = 1; p <= kProducers; p++) threads.emplace_back(producer, p); + for (size_t i = 0; i < threads.size(); i++) pin_worker(threads[i], (int)i); + + const auto start = std::chrono::steady_clock::now(); + bool timed_out = false; + while (consumed.load(std::memory_order_relaxed) < kTarget) { + if (ownership_violation.load() || api_error.load()) break; /* fail fast */ + if (std::chrono::steady_clock::now() - start > kRunBudget) { + timed_out = true; + break; + } + std::this_thread::sleep_for(std::chrono::milliseconds(2)); + } + stop.store(true); + for (auto& t : threads) t.join(); + + EXPECT_FALSE(timed_out) << "deadlock/livelock: consumed " << consumed.load() << "/" + << kTarget; + EXPECT_FALSE(ownership_violation.load()) + << "two consumer threads claimed the same framebuffer simultaneously"; + EXPECT_FALSE(api_error.load()) << "frame_done returned an error (double-claim)"; + EXPECT_TRUE(ut20p_tx_all_free(ctx)) << "framebuffer leaked (not back to FREE)"; + + ut20p_tx_ctx_destroy(ctx); +} + +TEST(St20PipelineConcurrencyStress, TxMixedPutAbortNoViolation) { + ASSERT_EQ(ut20p_tx_init(), 0) << "EAL init failed"; + + constexpr int kFrameCnt = 8; + constexpr int kProducers = 6; + constexpr int kTarget = 100000; + + ut20p_tx_ctx* ctx = ut20p_tx_ctx_create(kFrameCnt); + ASSERT_NE(ctx, nullptr); + + std::vector> holder(kFrameCnt); + for (auto& h : holder) h.store(0); + + std::atomic to_get{kTarget}; /* remaining get_frame attempts */ + std::atomic producers_live{kProducers}; /* producers still running */ + std::atomic aborted{0}; + std::atomic put{0}; /* handed to consumer */ + std::atomic transmitted{0}; + std::atomic stop{false}; + std::atomic ownership_violation{false}; + std::atomic api_error{false}; + + auto producer = [&](int id) { + uint32_t rng = 0x9e3779b9u * (uint32_t)id + 1u; + while (!stop.load(std::memory_order_relaxed)) { + if (to_get.fetch_sub(1, std::memory_order_relaxed) <= 0) break; + struct st_frame* f = nullptr; + while ((f = ut20p_tx_get_frame(ctx)) == nullptr) { + if (stop.load(std::memory_order_relaxed)) { + producers_live.fetch_sub(1, std::memory_order_release); + return; + } + } + int idx = ut20p_tx_frame_idx(f); + if (holder[idx].exchange(id) != 0) ownership_violation.store(true); + dwell(); + rng ^= rng << 13; + rng ^= rng >> 17; + rng ^= rng << 5; + bool do_abort = (rng & 3u) == 0u; /* ~25% of frames cancelled */ + if (holder[idx].exchange(0) != id) ownership_violation.store(true); + if (do_abort) { + if (ut20p_tx_put_frame_abort(ctx, f) != 0) api_error.store(true); + aborted.fetch_add(1, std::memory_order_relaxed); + } else { + if (ut20p_tx_put_frame(ctx, f) != 0) api_error.store(true); + put.fetch_add(1, std::memory_order_relaxed); + } + } + producers_live.fetch_sub(1, std::memory_order_release); + }; + + /* `put` is only final once every producer has exited; until then the consumer + * must keep draining even if it momentarily catches up. */ + auto done = [&]() { + return producers_live.load(std::memory_order_acquire) == 0 && + transmitted.load(std::memory_order_relaxed) >= + put.load(std::memory_order_relaxed); + }; + + auto consumer = [&]() { + while (!stop.load(std::memory_order_relaxed)) { + if (done()) break; + uint16_t idx = 0; + if (ut20p_tx_next_frame(ctx, &idx) != 0) continue; + if (holder[idx].exchange(-1) != 0) ownership_violation.store(true); + dwell(); + if (holder[idx].exchange(0) != -1) ownership_violation.store(true); + if (ut20p_tx_frame_done(ctx, idx) != 0) api_error.store(true); + transmitted.fetch_add(1, std::memory_order_relaxed); + } + }; + + std::vector threads; + threads.emplace_back(consumer); + for (int p = 1; p <= kProducers; p++) threads.emplace_back(producer, p); + for (size_t i = 0; i < threads.size(); i++) pin_worker(threads[i], (int)i); + + const auto start = std::chrono::steady_clock::now(); + bool timed_out = false; + while (!done()) { + if (std::chrono::steady_clock::now() - start > kRunBudget) { + timed_out = true; + break; + } + std::this_thread::sleep_for(std::chrono::milliseconds(2)); + } + stop.store(true); + for (auto& t : threads) t.join(); + + EXPECT_FALSE(timed_out) << "deadlock/livelock: transmitted " << transmitted.load() + << " put " << put.load(); + EXPECT_FALSE(ownership_violation.load()) + << "two actors held the same framebuffer during mixed put/abort"; + EXPECT_FALSE(api_error.load()) << "put/abort/frame_done returned an error"; + EXPECT_EQ(aborted.load() + put.load(), kTarget) << "frames lost between get and put"; + EXPECT_EQ(transmitted.load(), put.load()) << "transmitted != put (lost/dup on wire)"; + EXPECT_EQ(ut20p_tx_stat_frames_sent(ctx), (uint64_t)put.load()); + EXPECT_TRUE(ut20p_tx_all_free(ctx)) << "framebuffer leaked (not back to FREE)"; + + ut20p_tx_ctx_destroy(ctx); +} diff --git a/tests/unit/pipeline/st20p_concurrency_test.cpp b/tests/unit/pipeline/st20p_concurrency_test.cpp new file mode 100644 index 000000000..a90dcb74f --- /dev/null +++ b/tests/unit/pipeline/st20p_concurrency_test.cpp @@ -0,0 +1,553 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright(c) 2026 Intel Corporation + * + * ST20p (video) pipeline-layer concurrency tests for the lock-free framebuffer + * ring. Mirrors st30p_concurrency_test: many threads hammer the atomic claim + * (compare-exchange) and assert single-ownership, conservation and + * deadlock-freedom. + * + * Topologies: + * TX: N app producers (get_frame/put_frame) + 1 transport consumer + * (next_frame/frame_done). + * RX: 1 transport producer (inject_frame) + N app consumers + * (get_frame/put_frame). + * + * Convert-path TOCTOU tests (RxConcurrentConvertersNoDoubleClaim, + * TxConcurrentConvertersNoDoubleClaim): + * These tests expose the race in rx_st20p_convert_get_frame / + * tx_st20p_convert_get_frame where a scan (next_available) followed by a + * plain stat write is NOT atomic: two concurrent converter threads can both + * see READY and both claim the same framebuffer. The fix is a CAS + * (READY -> IN_CONVERTING) in the claim path. The tests will FAIL before + * the CAS is added and PASS after. + */ + +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include + +#include "pipeline/st20p_harness.h" /* RX role: ut20p_* */ +#include "pipeline/st20p_tx_harness.h" /* TX role: ut20p_tx_* */ + +namespace { + +/* Wall-clock budget for a whole run. With each worker pinned to its own core a + * correct lock-free design finishes in well under a second; only a genuine + * livelock/deadlock approaches this. */ +constexpr auto kRunBudget = std::chrono::seconds(45); + +/* Brief on-CPU dwell that widens the ownership window so a concurrent + * violation has time to be observed by a second actor. */ +inline void dwell() { + for (volatile int i = 0; i < 64; i++) { + } +} + +/* Pin a worker to its own core so the lock-free ring is exercised, not the + * scheduler. An unpinned busy-spin loop on a multi-socket NUMA host is migrated + * and co-located by the scheduler, collapsing throughput by ~600x and tripping + * the deadlock budget on a design that is in fact lock-free. + * + * Core 0 is skipped on purpose: ut_eal_init() starts DPDK with "-c1", which + * pins the calling (main) thread to core 0 -- so the process affinity mask is + * {0} after init and must NOT be used as the candidate set. We spread workers + * across cores [1, nproc) by slot instead. Best-effort: failure leaves the + * thread unpinned. */ +inline void pin_worker(std::thread& t, int slot) { + long nproc = sysconf(_SC_NPROCESSORS_ONLN); + if (nproc <= 1) return; + cpu_set_t one; + CPU_ZERO(&one); + CPU_SET(1 + (slot % (int)(nproc - 1)), &one); + pthread_setaffinity_np(t.native_handle(), sizeof(one), &one); +} + +} // namespace + +/* ------------------------------------------------------------------ TX ---- */ + +TEST(St20PipelineConcurrency, TxMultiProducerSingleConsumerNoDeadlock) { + ASSERT_EQ(ut20p_tx_init(), 0) << "EAL init failed"; + + constexpr int kFrameCnt = 8; + constexpr int kProducers = 4; + constexpr int kTarget = 50000; + + ut20p_tx_ctx* ctx = ut20p_tx_ctx_create(kFrameCnt); + ASSERT_NE(ctx, nullptr); + + /* holder[idx]: 0 = free/queued, >0 = owning producer id, -1 = consumer. */ + std::vector> holder(kFrameCnt); + for (auto& h : holder) h.store(0); + + std::atomic to_produce{kTarget}; /* remaining production slots */ + std::atomic produced{0}; /* frames handed to transport */ + std::atomic consumed{0}; /* frames completed by transport */ + std::atomic stop{false}; + std::atomic ownership_violation{false}; + std::atomic api_error{false}; /* worker threads cannot call gtest macros */ + + auto producer = [&](int id) { + while (!stop.load(std::memory_order_relaxed)) { + if (to_produce.fetch_sub(1, std::memory_order_relaxed) <= 0) break; + struct st_frame* f = nullptr; + while ((f = ut20p_tx_get_frame(ctx)) == nullptr) { + if (stop.load(std::memory_order_relaxed)) return; + } + int idx = ut20p_tx_frame_idx(f); + if (holder[idx].exchange(id) != 0) ownership_violation.store(true); + dwell(); + if (holder[idx].exchange(0) != id) ownership_violation.store(true); + if (ut20p_tx_put_frame(ctx, f) != 0) api_error.store(true); + produced.fetch_add(1, std::memory_order_relaxed); + } + }; + + auto consumer = [&]() { + while (consumed.load(std::memory_order_relaxed) < kTarget) { + if (stop.load(std::memory_order_relaxed)) break; + uint16_t idx = 0; + if (ut20p_tx_next_frame(ctx, &idx) != 0) continue; /* nothing CONVERTED yet */ + if (holder[idx].exchange(-1) != 0) ownership_violation.store(true); + dwell(); + if (holder[idx].exchange(0) != -1) ownership_violation.store(true); + if (ut20p_tx_frame_done(ctx, idx) != 0) api_error.store(true); + consumed.fetch_add(1, std::memory_order_relaxed); + } + }; + + std::vector threads; + threads.emplace_back(consumer); + for (int p = 1; p <= kProducers; p++) threads.emplace_back(producer, p); + for (size_t i = 0; i < threads.size(); i++) pin_worker(threads[i], (int)i); + + const auto start = std::chrono::steady_clock::now(); + bool timed_out = false; + while (consumed.load(std::memory_order_relaxed) < kTarget) { + if (std::chrono::steady_clock::now() - start > kRunBudget) { + timed_out = true; + break; + } + std::this_thread::sleep_for(std::chrono::milliseconds(2)); + } + stop.store(true); + for (auto& t : threads) t.join(); + + if (timed_out) { + fprintf(stderr, "[TX timeout] produced=%d consumed=%d\n", produced.load(), + consumed.load()); + for (int i = 0; i < kFrameCnt; i++) + fprintf(stderr, " buf %d stat=%d holder=%d\n", i, ut20p_tx_frame_stat(ctx, i), + holder[i].load()); + } + + EXPECT_FALSE(timed_out) << "deadlock/livelock: consumed " << consumed.load() << "/" + << kTarget; + EXPECT_FALSE(ownership_violation.load()) << "two actors held the same framebuffer"; + EXPECT_FALSE(api_error.load()) << "put_frame/frame_done returned an error"; + EXPECT_EQ(produced.load(), kTarget); + EXPECT_EQ(consumed.load(), kTarget) << "frames lost or duplicated"; + EXPECT_EQ(ut20p_tx_stat_frames_sent(ctx), (uint64_t)kTarget); + EXPECT_TRUE(ut20p_tx_all_free(ctx)) << "framebuffer leaked (not back to FREE)"; + + ut20p_tx_ctx_destroy(ctx); +} + +/* ------------------------------------------------------------------ RX ---- */ + +TEST(St20PipelineConcurrency, RxSingleProducerMultiConsumerNoDeadlock) { + ASSERT_EQ(ut20p_init(), 0) << "EAL init failed"; + + constexpr int kFrameCnt = 8; + constexpr int kConsumers = 4; + constexpr int kTarget = 50000; + + ut20p_ctx* ctx = ut20p_ctx_create(kFrameCnt); + ASSERT_NE(ctx, nullptr); + + /* holder[idx]: 0 = free/queued, >0 = owning consumer id. */ + std::vector> holder(kFrameCnt); + for (auto& h : holder) h.store(0); + + std::atomic produced{0}; + std::atomic consumed{0}; + std::atomic stop{false}; + std::atomic ownership_violation{false}; + std::atomic api_error{false}; /* worker threads cannot call gtest macros */ + + auto producer = [&]() { + uint32_t ts = 1; + while (produced.load(std::memory_order_relaxed) < kTarget) { + if (stop.load(std::memory_order_relaxed)) break; + if (ut20p_inject_frame(ctx, ST_FRAME_STATUS_COMPLETE, ts++) == 0) + produced.fetch_add(1, std::memory_order_relaxed); + /* else -EBUSY: ring full, consumers will drain it; retry. */ + } + }; + + auto consumer = [&](int id) { + while (consumed.load(std::memory_order_relaxed) < kTarget) { + if (stop.load(std::memory_order_relaxed)) break; + struct st_frame* f = ut20p_get_frame(ctx); + if (!f) continue; /* nothing READY yet */ + int idx = ut20p_frame_idx(f); + if (holder[idx].exchange(id) != 0) ownership_violation.store(true); + dwell(); + if (holder[idx].exchange(0) != id) ownership_violation.store(true); + if (ut20p_put_frame(ctx, f) != 0) api_error.store(true); + consumed.fetch_add(1, std::memory_order_relaxed); + } + }; + + std::vector threads; + threads.emplace_back(producer); + for (int c = 1; c <= kConsumers; c++) threads.emplace_back(consumer, c); + for (size_t i = 0; i < threads.size(); i++) pin_worker(threads[i], (int)i); + + const auto start = std::chrono::steady_clock::now(); + bool timed_out = false; + while (consumed.load(std::memory_order_relaxed) < kTarget) { + if (std::chrono::steady_clock::now() - start > kRunBudget) { + timed_out = true; + break; + } + std::this_thread::sleep_for(std::chrono::milliseconds(2)); + } + stop.store(true); + for (auto& t : threads) t.join(); + + if (timed_out) { + fprintf(stderr, "[RX timeout] produced=%d consumed=%d\n", produced.load(), + consumed.load()); + for (int i = 0; i < kFrameCnt; i++) + fprintf(stderr, " buf %d stat=%d holder=%d\n", i, ut20p_frame_stat(ctx, i), + holder[i].load()); + } + + EXPECT_FALSE(timed_out) << "deadlock/livelock: consumed " << consumed.load() << "/" + << kTarget; + EXPECT_FALSE(ownership_violation.load()) << "two consumers held the same framebuffer"; + EXPECT_FALSE(api_error.load()) << "put_frame returned an error"; + EXPECT_EQ(produced.load(), kTarget); + EXPECT_EQ(consumed.load(), kTarget) << "frames lost or duplicated"; + EXPECT_TRUE(ut20p_framebuff_cnt(ctx) == kFrameCnt); + + ut20p_ctx_destroy(ctx); +} + +/* ---- RX: concurrent external-converter claim (TOCTOU without CAS) ---- */ + +/* Two converter threads simultaneously scan for READY frames and try to claim + * them. Without a CAS in rx_st20p_convert_get_frame both threads can observe + * READY and proceed past the check before either writes IN_CONVERTING, causing + * a double-claim. The test detects this via the holder[] exchange pattern. + * + * Expected: FAIL before the READY->IN_CONVERTING CAS is added to production + * code; PASS after. */ +TEST(St20PipelineConcurrency, RxConcurrentConvertersNoDoubleClaim) { + ASSERT_EQ(ut20p_init(), 0) << "EAL init failed"; + + constexpr int kFrameCnt = 8; + constexpr int kConverters = 4; + constexpr int kTarget = 50000; + + ut20p_ctx* ctx = ut20p_ctx_create(kFrameCnt); + ASSERT_NE(ctx, nullptr); + + /* holder[idx]: 0 = free/in-convert, >0 = owning converter id. */ + std::vector> holder(kFrameCnt); + for (auto& h : holder) h.store(0); + + std::atomic claimed{0}; + std::atomic stop{false}; + std::atomic ownership_violation{false}; + std::atomic api_error{false}; + + /* Refiller: keeps putting frames back to READY so converters always have + * work. Runs independently of the converter threads. */ + auto refiller = [&]() { + while (!stop.load(std::memory_order_relaxed)) { + for (int i = 0; i < kFrameCnt; i++) { + /* Only reset slots that are back to FREE (put_frame released them). */ + int s = ut20p_frame_stat(ctx, i); + if (s == 0 /* FREE */) ut20p_set_frame_ready(ctx, i); + } + } + }; + + auto converter = [&](int id) { + while (claimed.load(std::memory_order_relaxed) < kTarget) { + if (stop.load(std::memory_order_relaxed)) break; + struct st20_convert_frame_meta* meta = ut20p_convert_get_frame(ctx); + if (!meta) continue; + int idx = ut20p_convert_frame_idx(meta); + /* Atomically mark ownership; if another thread already owns this + * slot the exchange returns non-zero -> ownership violation. */ + if (holder[idx].exchange(id) != 0) ownership_violation.store(true); + dwell(); + if (holder[idx].exchange(0) != id) ownership_violation.store(true); + /* Return the frame with result=-1 (convert-failed path) so the pipeline + * releases it directly back to FREE, keeping the ring draining without + * needing a separate app-consumer thread. The ownership invariant is + * identical regardless of the result code. */ + if (ut20p_convert_put_frame(ctx, meta, -1) != 0) api_error.store(true); + claimed.fetch_add(1, std::memory_order_relaxed); + } + }; + + std::vector threads; + threads.emplace_back(refiller); + for (int c = 1; c <= kConverters; c++) threads.emplace_back(converter, c); + for (size_t i = 0; i < threads.size(); i++) pin_worker(threads[i], (int)i); + + const auto start = std::chrono::steady_clock::now(); + bool timed_out = false; + while (claimed.load(std::memory_order_relaxed) < kTarget) { + if (std::chrono::steady_clock::now() - start > kRunBudget) { + timed_out = true; + break; + } + std::this_thread::sleep_for(std::chrono::milliseconds(2)); + } + stop.store(true); + for (auto& t : threads) t.join(); + + if (timed_out) { + fprintf(stderr, "[RX-convert timeout] claimed=%d\n", claimed.load()); + for (int i = 0; i < kFrameCnt; i++) + fprintf(stderr, " buf %d stat=%d holder=%d\n", i, ut20p_frame_stat(ctx, i), + holder[i].load()); + } + + EXPECT_FALSE(timed_out) << "deadlock/livelock: claimed " << claimed.load() << "/" + << kTarget; + EXPECT_FALSE(ownership_violation.load()) + << "two converter threads held the same framebuffer simultaneously"; + EXPECT_FALSE(api_error.load()) << "convert_put_frame returned an error"; + + ut20p_ctx_destroy(ctx); +} + +/* ---- TX: concurrent external-converter claim (TOCTOU without CAS) ---- */ + +/* Mirror of RxConcurrentConvertersNoDoubleClaim for the TX pipeline. + * tx_st20p_convert_get_frame has the same TOCTOU pattern: scan for READY, + * write IN_CONVERTING — no CAS. The fix is a CAS (READY->IN_CONVERTING) + * identical to the RX fix. */ +TEST(St20PipelineConcurrency, TxConcurrentConvertersNoDoubleClaim) { + ASSERT_EQ(ut20p_tx_init(), 0) << "EAL init failed"; + + constexpr int kFrameCnt = 8; + constexpr int kConverters = 4; + constexpr int kTarget = 50000; + + ut20p_tx_ctx* ctx = ut20p_tx_ctx_create(kFrameCnt); + ASSERT_NE(ctx, nullptr); + + std::vector> holder(kFrameCnt); + for (auto& h : holder) h.store(0); + + std::atomic claimed{0}; + std::atomic stop{false}; + std::atomic ownership_violation{false}; + std::atomic api_error{false}; + + auto refiller = [&]() { + while (!stop.load(std::memory_order_relaxed)) { + for (int i = 0; i < kFrameCnt; i++) { + int s = ut20p_tx_frame_stat(ctx, i); + if (s == 0 /* FREE */) ut20p_tx_set_frame_ready(ctx, i); + } + } + }; + + auto converter = [&](int id) { + while (claimed.load(std::memory_order_relaxed) < kTarget) { + if (stop.load(std::memory_order_relaxed)) break; + struct st20_convert_frame_meta* meta = ut20p_tx_convert_get_frame(ctx); + if (!meta) continue; + int idx = ut20p_tx_convert_frame_idx(meta); + if (holder[idx].exchange(id) != 0) ownership_violation.store(true); + dwell(); + if (holder[idx].exchange(0) != id) ownership_violation.store(true); + if (ut20p_tx_convert_put_frame(ctx, meta, 0) != 0) api_error.store(true); + claimed.fetch_add(1, std::memory_order_relaxed); + } + }; + + std::vector threads; + threads.emplace_back(refiller); + for (int c = 1; c <= kConverters; c++) threads.emplace_back(converter, c); + for (size_t i = 0; i < threads.size(); i++) pin_worker(threads[i], (int)i); + + const auto start = std::chrono::steady_clock::now(); + bool timed_out = false; + while (claimed.load(std::memory_order_relaxed) < kTarget) { + if (std::chrono::steady_clock::now() - start > kRunBudget) { + timed_out = true; + break; + } + std::this_thread::sleep_for(std::chrono::milliseconds(2)); + } + stop.store(true); + for (auto& t : threads) t.join(); + + if (timed_out) { + fprintf(stderr, "[TX-convert timeout] claimed=%d\n", claimed.load()); + for (int i = 0; i < kFrameCnt; i++) + fprintf(stderr, " buf %d stat=%d holder=%d\n", i, ut20p_tx_frame_stat(ctx, i), + holder[i].load()); + } + + EXPECT_FALSE(timed_out) << "deadlock/livelock: claimed " << claimed.load() << "/" + << kTarget; + EXPECT_FALSE(ownership_violation.load()) + << "two converter threads held the same TX framebuffer simultaneously"; + EXPECT_FALSE(api_error.load()) << "convert_put_frame returned an error"; + + ut20p_tx_ctx_destroy(ctx); +} + +/* ---- TX: two-phase manual external-frame release lifecycle ---- */ + +/* Exercises the ST20P_TX_FLAG_EXT_FRAME_MANUAL_RELEASE path, the one TX + * lifecycle the other cases skip: frame_done parks the slot in IN_USER instead + * of returning it to FREE, and a *separate* thread later calls + * notify_ext_frame_free (IN_USER -> FREE). This adds a second cross-thread + * hand-off to the ring, so the full walk is: + * + * producer: FREE -> IN_USER -> CONVERTED (get_frame / put_frame) + * consumer: CONVERTED -> IN_TRANSMITTING -> IN_USER (next_frame / frame_done) + * releaser: IN_USER -> FREE (notify_ext_frame_free) + * + * holder[] threads a single ownership token through every stage; any stage + * observing the wrong predecessor token is a lost-slot / double-claim bug. + * The consumer->releaser hand-off uses an explicit SPSC queue so each slot is + * released exactly once (mirrors the app contract: one owner per frame). */ +TEST(St20PipelineConcurrency, TxManualReleaseLifecycleNoDoubleClaim) { + ASSERT_EQ(ut20p_tx_init(), 0) << "EAL init failed"; + + constexpr int kFrameCnt = 8; + constexpr int kProducers = 4; + constexpr int kTarget = 50000; + + ut20p_tx_ctx* ctx = ut20p_tx_ctx_create(kFrameCnt); + ASSERT_NE(ctx, nullptr); + ut20p_tx_ctx_set_manual_release(ctx); + + /* holder[idx] ownership token: + * 0 free/queued in the ring + * >0 owning producer id + * -1 consumer (transmitting) + * -2 parked IN_USER, awaiting manual release */ + std::vector> holder(kFrameCnt); + for (auto& h : holder) h.store(0); + + std::atomic to_produce{kTarget}; + std::atomic produced{0}; + std::atomic transmitted{0}; + std::atomic released{0}; + std::atomic stop{false}; + std::atomic ownership_violation{false}; + std::atomic api_error{false}; + + /* consumer -> releaser hand-off of parked (IN_USER) slot indices. */ + std::mutex q_mtx; + std::queue parked; + + auto producer = [&](int id) { + while (!stop.load(std::memory_order_relaxed)) { + if (to_produce.fetch_sub(1, std::memory_order_relaxed) <= 0) break; + struct st_frame* f = nullptr; + while ((f = ut20p_tx_get_frame(ctx)) == nullptr) { + if (stop.load(std::memory_order_relaxed)) return; + } + int idx = ut20p_tx_frame_idx(f); + if (holder[idx].exchange(id) != 0) ownership_violation.store(true); + dwell(); + if (holder[idx].exchange(0) != id) ownership_violation.store(true); + if (ut20p_tx_put_frame(ctx, f) != 0) api_error.store(true); + produced.fetch_add(1, std::memory_order_relaxed); + } + }; + + auto consumer = [&]() { + while (transmitted.load(std::memory_order_relaxed) < kTarget) { + if (stop.load(std::memory_order_relaxed)) break; + uint16_t idx = 0; + if (ut20p_tx_next_frame(ctx, &idx) != 0) continue; /* nothing CONVERTED */ + if (holder[idx].exchange(-1) != 0) ownership_violation.store(true); + dwell(); + /* frame_done parks the slot in IN_USER (manual-release flag set). */ + if (ut20p_tx_frame_done(ctx, idx) != 0) api_error.store(true); + if (holder[idx].exchange(-2) != -1) ownership_violation.store(true); + transmitted.fetch_add(1, std::memory_order_relaxed); + std::lock_guard lk(q_mtx); + parked.push(idx); + } + }; + + auto releaser = [&]() { + while (released.load(std::memory_order_relaxed) < kTarget) { + if (stop.load(std::memory_order_relaxed)) break; + uint16_t idx = 0; + { + std::lock_guard lk(q_mtx); + if (parked.empty()) continue; + idx = parked.front(); + parked.pop(); + } + if (holder[idx].exchange(0) != -2) ownership_violation.store(true); + if (ut20p_tx_notify_ext_frame_free(ctx, idx) != 0) api_error.store(true); + released.fetch_add(1, std::memory_order_relaxed); + } + }; + + std::vector threads; + threads.emplace_back(consumer); + threads.emplace_back(releaser); + for (int p = 1; p <= kProducers; p++) threads.emplace_back(producer, p); + for (size_t i = 0; i < threads.size(); i++) pin_worker(threads[i], (int)i); + + const auto start = std::chrono::steady_clock::now(); + bool timed_out = false; + while (released.load(std::memory_order_relaxed) < kTarget) { + if (std::chrono::steady_clock::now() - start > kRunBudget) { + timed_out = true; + break; + } + std::this_thread::sleep_for(std::chrono::milliseconds(2)); + } + stop.store(true); + for (auto& t : threads) t.join(); + + if (timed_out) { + fprintf(stderr, + "[TX manual-release timeout] produced=%d transmitted=%d released=%d\n", + produced.load(), transmitted.load(), released.load()); + for (int i = 0; i < kFrameCnt; i++) + fprintf(stderr, " buf %d stat=%d holder=%d\n", i, ut20p_tx_frame_stat(ctx, i), + holder[i].load()); + } + + EXPECT_FALSE(timed_out) << "deadlock/livelock: released " << released.load() << "/" + << kTarget; + EXPECT_FALSE(ownership_violation.load()) + << "two actors held the same framebuffer during the manual-release lifecycle"; + EXPECT_FALSE(api_error.load()) << "put_frame/frame_done/notify_ext_frame_free error"; + EXPECT_EQ(produced.load(), kTarget); + EXPECT_EQ(transmitted.load(), kTarget) << "frames lost or duplicated on transmit"; + EXPECT_EQ(released.load(), kTarget) << "frames lost or duplicated on release"; + EXPECT_EQ(ut20p_tx_stat_frames_sent(ctx), (uint64_t)kTarget); + EXPECT_TRUE(ut20p_tx_all_free(ctx)) << "framebuffer leaked (not back to FREE)"; + + ut20p_tx_ctx_destroy(ctx); +} diff --git a/tests/unit/pipeline/st20p_harness.c b/tests/unit/pipeline/st20p_harness.c index e5412544e..2e03ea813 100644 --- a/tests/unit/pipeline/st20p_harness.c +++ b/tests/unit/pipeline/st20p_harness.c @@ -97,6 +97,11 @@ ut20p_ctx* ut20p_ctx_create(int framebuff_cnt) { * `dst = src`, so setting src.priv is sufficient. */ ctx->framebuffs[i].src.priv = &ctx->framebuffs[i]; ctx->framebuffs[i].dst.priv = &ctx->framebuffs[i]; + /* convert_frame.priv lets ut20p_convert_frame_idx() recover the + * owning slot index from a st20_convert_frame_meta pointer. */ + ctx->framebuffs[i].convert_frame.src = &ctx->framebuffs[i].src; + ctx->framebuffs[i].convert_frame.dst = &ctx->framebuffs[i].dst; + ctx->framebuffs[i].convert_frame.priv = &ctx->framebuffs[i]; } struct st20p_rx_ctx* p = &ctx->pipeline; @@ -114,18 +119,11 @@ ut20p_ctx* ut20p_ctx_create(int framebuff_cnt) { * but our stubs ignore it. */ p->transport = (st20_rx_handle)(uintptr_t)0x1; - if (pthread_mutex_init(&p->lock, NULL) != 0) { - free(ctx->framebuffs); - free(ctx); - return NULL; - } - return ctx; } void ut20p_ctx_destroy(ut20p_ctx* ctx) { if (!ctx) return; - pthread_mutex_destroy(&ctx->pipeline.lock); free(ctx->framebuffs); free(ctx); } @@ -160,6 +158,37 @@ struct st_frame* ut20p_get_frame(ut20p_ctx* ctx) { int ut20p_put_frame(ut20p_ctx* ctx, struct st_frame* frame) { return st20p_rx_put_frame(&ctx->pipeline, frame); } +void ut20p_set_frame_ready(ut20p_ctx* ctx, int idx) { + __atomic_store_n(&ctx->framebuffs[idx].stat, ST20P_RX_FRAME_READY, __ATOMIC_RELEASE); +} + +struct st20_convert_frame_meta* ut20p_convert_get_frame(ut20p_ctx* ctx) { + return rx_st20p_convert_get_frame(&ctx->pipeline); +} + +int ut20p_convert_put_frame(ut20p_ctx* ctx, struct st20_convert_frame_meta* frame, + int result) { + return rx_st20p_convert_put_frame(&ctx->pipeline, frame, result); +} + +int ut20p_convert_frame_idx(const struct st20_convert_frame_meta* meta) { + const struct st20p_rx_frame* framebuff = meta->priv; + return framebuff->idx; +} +/* ── concurrency-test helpers ─────────────────────────────────────────── */ + +int ut20p_frame_idx(const struct st_frame* frame) { + const struct st20p_rx_frame* framebuff = frame->priv; + return framebuff->idx; +} + +int ut20p_framebuff_cnt(const ut20p_ctx* ctx) { + return ctx->framebuff_cnt; +} + +int ut20p_frame_stat(const ut20p_ctx* ctx, int i) { + return (int)ctx->framebuffs[i].stat; +} /* ── stat accessors ───────────────────────────────────────────────────── */ diff --git a/tests/unit/pipeline/st20p_harness.h b/tests/unit/pipeline/st20p_harness.h index 6d0d9f9e5..7c841995b 100644 --- a/tests/unit/pipeline/st20p_harness.h +++ b/tests/unit/pipeline/st20p_harness.h @@ -50,6 +50,35 @@ struct st_frame* ut20p_get_frame(ut20p_ctx* ctx); /** Wraps st20p_rx_put_frame(). */ int ut20p_put_frame(ut20p_ctx* ctx, struct st_frame* frame); +/** + * Set framebuffer i directly to READY state, bypassing the normal + * inject path. Used by the concurrent-converter tests which need + * frames in READY (not CONVERTED) so that rx_st20p_convert_get_frame + * finds them and tries to claim them. + */ +void ut20p_set_frame_ready(ut20p_ctx* ctx, int idx); + +/** Wraps rx_st20p_convert_get_frame() — the external converter claim. */ +struct st20_convert_frame_meta* ut20p_convert_get_frame(ut20p_ctx* ctx); + +/** Wraps rx_st20p_convert_put_frame(). result 0 = success, <0 = fail. */ +int ut20p_convert_put_frame(ut20p_ctx* ctx, struct st20_convert_frame_meta* frame, + int result); + +/** Buffer index encoded in a convert-frame meta pointer. */ +int ut20p_convert_frame_idx(const struct st20_convert_frame_meta* meta); + +/* ── concurrency-test helpers ─────────────────────────────────────────── */ + +/** Buffer index that a user-facing frame belongs to. */ +int ut20p_frame_idx(const struct st_frame* frame); + +/** Total framebuffer count. */ +int ut20p_framebuff_cnt(const ut20p_ctx* ctx); + +/** Raw stat value of framebuffer i (for diagnostics). */ +int ut20p_frame_stat(const ut20p_ctx* ctx, int i); + /* ── stat accessors ───────────────────────────────────────────────── */ uint64_t ut20p_stat_frames_received(const ut20p_ctx* ctx); diff --git a/tests/unit/pipeline/st20p_rx_concurrency_test.cpp b/tests/unit/pipeline/st20p_rx_concurrency_test.cpp new file mode 100644 index 000000000..ab0b31f90 --- /dev/null +++ b/tests/unit/pipeline/st20p_rx_concurrency_test.cpp @@ -0,0 +1,132 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright(c) 2026 Intel Corporation + * + * ST20p (video) RX pipeline-layer concurrency test for the lock-free + * framebuffer ring. + * + * st20p_rx_get_frame() claims a delivered slot by scanning + * rx_st20p_next_available() for a candidate (CONVERTED, or READY in the + * internal-converter path) and then issuing one compare-exchange to move it + * to IN_USER. The CAS itself prevents two consumers from ever owning the same + * slot, but a naive implementation gives up and reports "no frame available" + * the instant that single CAS loses a race -- even when other delivered slots + * exist -- instead of continuing the scan for another candidate. + * + * next_available() scans the ring from a shared consumer cursor, so when many + * threads call get_frame() at (approximately) the same instant they converge + * on the same candidate. With N consumer threads releasing at once against N + * delivered slots, a "scan once + single CAS attempt" implementation lets only + * the CAS winner succeed per candidate and leaves the rest reporting "no + * frame" -- a spurious claim failure -- even though N-1 other slots are still + * deliverable. The fix is for the claim itself to retry the scan on a lost + * race, so a single get_frame() call only returns NULL once every slot has + * genuinely been checked and found unavailable. + */ + +#include +#include +#include +#include + +#include +#include +#include +#include + +#include "pipeline/st20p_harness.h" + +namespace { + +/* Pin a worker to its own core so all consumers are genuinely running in + * parallel when they hit the barrier release, instead of the scheduler + * serializing them onto one core. Core 0 is skipped on purpose: ut_eal_init() + * starts DPDK with "-c1", which pins the calling (main) thread to core 0. + * Best-effort: failure leaves the thread unpinned. */ +inline void pin_worker(std::thread& t, int slot) { + long nproc = sysconf(_SC_NPROCESSORS_ONLN); + if (nproc <= 1) return; + cpu_set_t one; + CPU_ZERO(&one); + CPU_SET(1 + (slot % (int)(nproc - 1)), &one); + pthread_setaffinity_np(t.native_handle(), sizeof(one), &one); +} + +} // namespace + +TEST(St20PipelineRxConcurrency, MultiConsumerNoSpuriousClaimFailure) { + ASSERT_EQ(ut20p_init(), 0) << "EAL init failed"; + + constexpr int kFrameCnt = 8; + constexpr int kThreads = kFrameCnt; /* one thread per slot: none should miss */ + constexpr int kTrials = 200; + constexpr auto kRunBudget = std::chrono::seconds(45); + + ut20p_ctx* ctx = ut20p_ctx_create(kFrameCnt); + ASSERT_NE(ctx, nullptr); + + int spurious_failures = 0; + int ownership_violations = 0; + + const auto start = std::chrono::steady_clock::now(); + for (int trial = 0; trial < kTrials; trial++) { + ASSERT_LT(std::chrono::steady_clock::now() - start, kRunBudget) + << "deadlock/livelock across trials"; + + /* Fill the whole ring: each inject drives one FREE slot to CONVERTED + * (derive path), the state get_frame() consumes. */ + for (int i = 0; i < kFrameCnt; i++) { + ASSERT_EQ(ut20p_inject_frame(ctx, ST_FRAME_STATUS_COMPLETE, (uint32_t)(i + 1)), 0); + } + + std::atomic ready{0}; + std::atomic go{false}; + std::vector results(kThreads, nullptr); + std::vector threads; + + for (int t = 0; t < kThreads; t++) { + threads.emplace_back([&, t]() { + ready.fetch_add(1, std::memory_order_relaxed); + while (!go.load(std::memory_order_acquire)) { + } + /* single-shot: exactly one get_frame() call per thread, no retry. */ + results[t] = ut20p_get_frame(ctx); + }); + pin_worker(threads[t], t); + } + while (ready.load(std::memory_order_relaxed) < kThreads) { + } + go.store(true, std::memory_order_release); + for (auto& th : threads) th.join(); + + int got = 0; + std::vector idx_owner(kFrameCnt, -1); + for (int t = 0; t < kThreads; t++) { + if (!results[t]) continue; + got++; + int idx = ut20p_frame_idx(results[t]); + if (idx_owner[idx] != -1) ownership_violations++; /* two threads, one slot */ + idx_owner[idx] = t; + } + if (got < kFrameCnt) spurious_failures += (kFrameCnt - got); + + /* Release claimed slots, then single-threaded drain any slots a buggy + * get_frame() spuriously left behind, so every trial starts from an + * all-FREE ring regardless of the outcome above (contention is over, so + * this drain always succeeds). */ + for (auto* f : results) { + if (f) { + ASSERT_EQ(ut20p_put_frame(ctx, f), 0); + } + } + struct st_frame* leftover; + while ((leftover = ut20p_get_frame(ctx)) != nullptr) { + ASSERT_EQ(ut20p_put_frame(ctx, leftover), 0); + } + } + + EXPECT_EQ(ownership_violations, 0) << "two consumer threads claimed the same slot"; + EXPECT_EQ(spurious_failures, 0) + << "get_frame() reported \"no frame\" while a delivered slot still existed"; + + ut20p_ctx_destroy(ctx); +} diff --git a/tests/unit/pipeline/st20p_tx_blocking_test.cpp b/tests/unit/pipeline/st20p_tx_blocking_test.cpp new file mode 100644 index 000000000..62eebb80a --- /dev/null +++ b/tests/unit/pipeline/st20p_tx_blocking_test.cpp @@ -0,0 +1,73 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright(c) 2026 Intel Corporation + * + * ST20p (video) TX pipeline blocking get_frame() wake-loss regression test. + * + * In ST20P_TX_FLAG_BLOCK_GET mode, st20p_tx_get_frame() blocks on a condition + * variable when no FREE slot is available and is woken by tx_st20p_block_wake() + * (fired from frame_done, the public wake API, and the destroy hook). + * + * The producer of FREE slots (the transport frame_done callback) runs on a + * different thread than the app consumer, so a wake can be signalled in the + * window after the consumer's claim fails but before it enters + * pthread_cond_timedwait. pthread_cond_signal only wakes threads already + * waiting, so that wake is lost: a single-shot "if (!destroying) timedwait()" + * consumer then sleeps the entire block timeout despite the wake, and a + * spurious wakeup lets it return NULL before the timeout elapses. + * + * The fix records the wake in a sticky block_wake_pending flag and re-checks it + * in a predicate loop, so a wake posted before the wait is observed + * immediately. This test posts the wake while no consumer is waiting (the exact + * lost-wake window) and asserts the next blocking get_frame() returns promptly + * instead of sleeping the full timeout. + */ + +#include + +#include + +#include "pipeline/st20p_tx_harness.h" + +TEST(St20PipelineTxBlocking, WakePostedBeforeWaitIsNotLost) { + ASSERT_EQ(ut20p_tx_init(), 0) << "EAL init failed"; + + constexpr int kFrameCnt = 2; + /* Long enough that a lost wake (full-timeout block) is unmistakably distinct + * from the correct near-instant return, yet short enough to bound the suite + * if the bug regresses. */ + constexpr uint64_t kBlockTimeoutNs = 2ULL * 1000 * 1000 * 1000; /* 2 s */ + + ut20p_tx_ctx* ctx = ut20p_tx_ctx_create(kFrameCnt); + ASSERT_NE(ctx, nullptr); + ut20p_tx_ctx_enable_blocking(ctx, kBlockTimeoutNs); + + /* Drain every FREE slot so the next get_frame() must enter the blocking wait + * path (its initial claim finds nothing). These claims return immediately + * because slots are FREE. */ + struct st_frame* held[kFrameCnt]; + for (int i = 0; i < kFrameCnt; i++) { + held[i] = ut20p_tx_get_frame(ctx); + ASSERT_NE(held[i], nullptr) << "initial claim of FREE slot " << i << " failed"; + } + + /* Post a wake while no consumer is waiting yet -- exactly the lost-wake + * window a real frame_done producer hits. A correct implementation records it + * (block_wake_pending) so the very next blocking get_frame observes it and + * skips the sleep; the buggy single-shot wait discards the signal and blocks + * for the full kBlockTimeoutNs. */ + ut20p_tx_wake_block(ctx); + + const auto t0 = std::chrono::steady_clock::now(); + struct st_frame* frame = ut20p_tx_get_frame(ctx); /* no FREE slot -> wait path */ + const auto elapsed = std::chrono::steady_clock::now() - t0; + + /* No slot was actually freed, so the claim still fails -- what matters is that + * it failed FAST, proving the pre-posted wake was not lost. */ + EXPECT_EQ(frame, nullptr) << "no slot was freed; get_frame must return NULL"; + EXPECT_LT(elapsed, std::chrono::nanoseconds(kBlockTimeoutNs) / 4) + << "blocking get_frame slept ~full timeout: the pre-posted wake was lost"; + + for (int i = 0; i < kFrameCnt; i++) + ASSERT_EQ(ut20p_tx_put_frame_abort(ctx, held[i]), 0); + ut20p_tx_ctx_destroy(ctx); +} diff --git a/tests/unit/pipeline/st20p_tx_concurrency_test.cpp b/tests/unit/pipeline/st20p_tx_concurrency_test.cpp new file mode 100644 index 000000000..00928d546 --- /dev/null +++ b/tests/unit/pipeline/st20p_tx_concurrency_test.cpp @@ -0,0 +1,118 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright(c) 2026 Intel Corporation + * + * ST20p (video) TX pipeline-layer concurrency test for the lock-free + * framebuffer ring. + * + * get_frame() claims a FREE slot by scanning tx_st20p_next_available() for a + * candidate and then issuing one compare-exchange on it. The CAS itself + * prevents two producers from ever owning the same slot, but a naive + * implementation gives up and reports "no frame available" the instant that + * single CAS loses a race -- even when other FREE slots exist -- instead of + * continuing the scan for another candidate. + * + * next_available() always scans the ring from index 0, so when many threads + * call get_frame() at (approximately) the same instant they overwhelmingly + * converge on the same lowest-index FREE candidate. With N producer threads + * releasing at once against N FREE slots, a "scan once + single CAS attempt" + * implementation lets only the CAS winner succeed per candidate and leaves + * the rest reporting "no frame" -- a spurious claim failure -- even though + * N-1 other slots are still FREE. The fix is for the claim itself to retry + * the scan on a lost race, so a single get_frame() call only returns NULL + * once every slot has genuinely been checked and found unavailable. + */ + +#include +#include +#include +#include + +#include +#include +#include +#include + +#include "pipeline/st20p_tx_harness.h" + +namespace { + +/* Pin a worker to its own core so all producers are genuinely running in + * parallel when they hit the barrier release, instead of the scheduler + * serializing them onto one core. Core 0 is skipped on purpose: ut_eal_init() + * starts DPDK with "-c1", which pins the calling (main) thread to core 0. + * Best-effort: failure leaves the thread unpinned. */ +inline void pin_worker(std::thread& t, int slot) { + long nproc = sysconf(_SC_NPROCESSORS_ONLN); + if (nproc <= 1) return; + cpu_set_t one; + CPU_ZERO(&one); + CPU_SET(1 + (slot % (int)(nproc - 1)), &one); + pthread_setaffinity_np(t.native_handle(), sizeof(one), &one); +} + +} // namespace + +TEST(St20PipelineTxConcurrency, MultiProducerNoSpuriousClaimFailure) { + ASSERT_EQ(ut20p_tx_init(), 0) << "EAL init failed"; + + constexpr int kFrameCnt = 8; + constexpr int kThreads = kFrameCnt; /* one thread per slot: none should miss */ + constexpr int kTrials = 200; + constexpr auto kRunBudget = std::chrono::seconds(45); + + ut20p_tx_ctx* ctx = ut20p_tx_ctx_create(kFrameCnt); + ASSERT_NE(ctx, nullptr); + + int spurious_failures = 0; + int ownership_violations = 0; + + const auto start = std::chrono::steady_clock::now(); + for (int trial = 0; trial < kTrials; trial++) { + ASSERT_LT(std::chrono::steady_clock::now() - start, kRunBudget) + << "deadlock/livelock across trials"; + + std::atomic ready{0}; + std::atomic go{false}; + std::vector results(kThreads, nullptr); + std::vector threads; + + for (int t = 0; t < kThreads; t++) { + threads.emplace_back([&, t]() { + ready.fetch_add(1, std::memory_order_relaxed); + while (!go.load(std::memory_order_acquire)) { + } + /* single-shot: exactly one get_frame() call per thread, no retry. */ + results[t] = ut20p_tx_get_frame(ctx); + }); + pin_worker(threads[t], t); + } + while (ready.load(std::memory_order_relaxed) < kThreads) { + } + go.store(true, std::memory_order_release); + for (auto& th : threads) th.join(); + + int got = 0; + std::vector idx_owner(kFrameCnt, -1); + for (int t = 0; t < kThreads; t++) { + if (!results[t]) continue; + got++; + int idx = ut20p_tx_frame_idx(results[t]); + if (idx_owner[idx] != -1) ownership_violations++; /* two threads, one slot */ + idx_owner[idx] = t; + } + if (got < kFrameCnt) spurious_failures += (kFrameCnt - got); + + /* release every claimed slot before the next trial */ + for (auto* f : results) { + if (f) { + ASSERT_EQ(ut20p_tx_put_frame_abort(ctx, f), 0); + } + } + } + + EXPECT_EQ(ownership_violations, 0) << "two producer threads claimed the same slot"; + EXPECT_EQ(spurious_failures, 0) + << "get_frame() reported \"no frame\" while a FREE slot still existed"; + + ut20p_tx_ctx_destroy(ctx); +} diff --git a/tests/unit/pipeline/st20p_tx_harness.c b/tests/unit/pipeline/st20p_tx_harness.c new file mode 100644 index 000000000..bcfa72e7b --- /dev/null +++ b/tests/unit/pipeline/st20p_tx_harness.c @@ -0,0 +1,178 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright(c) 2026 Intel Corporation + * + * C harness for ST20p (video) TX pipeline-layer concurrency unit tests. + * Includes the production translation unit so the file-local transport + * callbacks (tx_st20p_next_frame / tx_st20p_frame_done) are reachable, and + * hand-initialises the ctx in the derive path so create_transport is bypassed. + */ + +#include +#include + +#undef MTL_HAS_USDT +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wunused-variable" +#pragma GCC diagnostic ignored "-Wunused-but-set-variable" +#include "st2110/pipeline/st20_pipeline_tx.c" +#pragma GCC diagnostic pop + +#include "common/ut_common.h" + +struct ut20p_tx_ctx { + struct mtl_main_impl impl; + struct st20p_tx_ctx pipeline; + struct st20p_tx_frame* framebuffs; + int framebuff_cnt; + bool blocking; +}; + +#include "pipeline/st20p_tx_harness.h" + +int ut20p_tx_init(void) { + return ut_eal_init(); +} + +ut20p_tx_ctx* ut20p_tx_ctx_create(int framebuff_cnt) { + ut20p_tx_ctx* ctx = calloc(1, sizeof(*ctx)); + if (!ctx) return NULL; + + ctx->framebuff_cnt = framebuff_cnt; + ctx->impl.type = MT_HANDLE_MAIN; + + ctx->framebuffs = calloc(framebuff_cnt, sizeof(struct st20p_tx_frame)); + if (!ctx->framebuffs) { + free(ctx); + return NULL; + } + for (int i = 0; i < framebuff_cnt; i++) { + ctx->framebuffs[i].stat = ST20P_TX_FRAME_FREE; + ctx->framebuffs[i].idx = i; + /* derive path: tx_st20p_user_frame() returns &dst, so put_frame() recovers + * the framebuf via dst.priv. */ + ctx->framebuffs[i].dst.priv = &ctx->framebuffs[i]; + /* convert_frame.priv lets ut20p_tx_convert_frame_idx() recover the slot + * index from a st20_convert_frame_meta pointer. */ + ctx->framebuffs[i].convert_frame.src = &ctx->framebuffs[i].src; + ctx->framebuffs[i].convert_frame.dst = &ctx->framebuffs[i].dst; + ctx->framebuffs[i].convert_frame.priv = &ctx->framebuffs[i]; + } + + struct st20p_tx_ctx* p = &ctx->pipeline; + p->impl = &ctx->impl; + p->idx = 0; + p->socket_id = rte_socket_id(); + p->type = MT_ST20_HANDLE_PIPELINE_TX; + p->framebuff_cnt = framebuff_cnt; + p->framebuffs = ctx->framebuffs; + p->ready = true; + p->derive = true; /* input_fmt == transport_fmt: put_frame -> CONVERTED directly */ + p->internal_converter = NULL; + p->convert_impl = NULL; + p->transport = (st20_tx_handle)(uintptr_t)0x1; + p->block_get = false; + /* ops.flags left 0: no DROP_WHEN_LATE / USER_PACING, so tx_st20p_if_frame_late + * returns immediately and next_frame never touches the (absent) transport. */ + + return ctx; +} + +void ut20p_tx_ctx_destroy(ut20p_tx_ctx* ctx) { + if (!ctx) return; + if (ctx->blocking) { + mt_pthread_mutex_destroy(&ctx->pipeline.block_wake_mutex); + mt_pthread_cond_destroy(&ctx->pipeline.block_wake_cond); + } + free(ctx->framebuffs); + free(ctx); +} + +void ut20p_tx_ctx_enable_blocking(ut20p_tx_ctx* ctx, uint64_t timeout_ns) { + struct st20p_tx_ctx* p = &ctx->pipeline; + mt_pthread_mutex_init(&p->block_wake_mutex, NULL); + mt_pthread_cond_wait_init(&p->block_wake_cond); + p->block_timeout_ns = timeout_ns; + p->wake_on_destroy = (void (*)(void*))tx_st20p_block_wake; + p->block_get = true; + ctx->blocking = true; +} + +void ut20p_tx_wake_block(ut20p_tx_ctx* ctx) { + st20p_tx_wake_block(&ctx->pipeline); +} + +int ut20p_tx_framebuff_cnt(const ut20p_tx_ctx* ctx) { + return ctx->framebuff_cnt; +} + +struct st_frame* ut20p_tx_get_frame(ut20p_tx_ctx* ctx) { + return st20p_tx_get_frame(&ctx->pipeline); +} + +int ut20p_tx_put_frame(ut20p_tx_ctx* ctx, struct st_frame* frame) { + return st20p_tx_put_frame(&ctx->pipeline, frame); +} + +int ut20p_tx_put_frame_abort(ut20p_tx_ctx* ctx, struct st_frame* frame) { + return st20p_tx_put_frame_abort(&ctx->pipeline, frame); +} + +int ut20p_tx_next_frame(ut20p_tx_ctx* ctx, uint16_t* idx) { + struct st20_tx_frame_meta meta; + memset(&meta, 0, sizeof(meta)); + return tx_st20p_next_frame(&ctx->pipeline, idx, &meta); +} + +int ut20p_tx_frame_done(ut20p_tx_ctx* ctx, uint16_t idx) { + struct st20_tx_frame_meta meta; + memset(&meta, 0, sizeof(meta)); + return tx_st20p_frame_done(&ctx->pipeline, idx, &meta); +} + +void ut20p_tx_ctx_set_manual_release(ut20p_tx_ctx* ctx) { + ctx->pipeline.ops.flags |= ST20P_TX_FLAG_EXT_FRAME_MANUAL_RELEASE; +} + +int ut20p_tx_notify_ext_frame_free(ut20p_tx_ctx* ctx, uint16_t idx) { + /* derive path: the user-facing frame is &dst (dst.priv -> framebuff). */ + struct st_frame* frame = &ctx->framebuffs[idx].dst; + return st20p_tx_notify_ext_frame_free(&ctx->pipeline, frame); +} + +void ut20p_tx_set_frame_ready(ut20p_tx_ctx* ctx, int idx) { + __atomic_store_n(&ctx->framebuffs[idx].stat, ST20P_TX_FRAME_READY, __ATOMIC_RELEASE); +} + +struct st20_convert_frame_meta* ut20p_tx_convert_get_frame(ut20p_tx_ctx* ctx) { + return tx_st20p_convert_get_frame(&ctx->pipeline); +} + +int ut20p_tx_convert_put_frame(ut20p_tx_ctx* ctx, struct st20_convert_frame_meta* frame, + int result) { + return tx_st20p_convert_put_frame(&ctx->pipeline, frame, result); +} + +int ut20p_tx_convert_frame_idx(const struct st20_convert_frame_meta* meta) { + const struct st20p_tx_frame* framebuff = meta->priv; + return framebuff->idx; +} + +int ut20p_tx_frame_idx(const struct st_frame* frame) { + const struct st20p_tx_frame* framebuff = frame->priv; + return framebuff->idx; +} + +int ut20p_tx_all_free(const ut20p_tx_ctx* ctx) { + for (int i = 0; i < ctx->framebuff_cnt; i++) { + if (ctx->framebuffs[i].stat != ST20P_TX_FRAME_FREE) return 0; + } + return 1; +} + +int ut20p_tx_frame_stat(const ut20p_tx_ctx* ctx, int i) { + return (int)ctx->framebuffs[i].stat; +} + +uint64_t ut20p_tx_stat_frames_sent(const ut20p_tx_ctx* ctx) { + return ctx->pipeline.stat_frames_sent; +} diff --git a/tests/unit/pipeline/st20p_tx_harness.h b/tests/unit/pipeline/st20p_tx_harness.h new file mode 100644 index 000000000..ac943e75a --- /dev/null +++ b/tests/unit/pipeline/st20p_tx_harness.h @@ -0,0 +1,105 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright(c) 2026 Intel Corporation + * + * C header for ST20p (video) TX pipeline-layer concurrency unit tests. + * + * Exposes the two role-halves of the lock-free TX framebuffer ring so a + * gtest can drive them from independent threads: + * - producer (app): get_frame (FREE->IN_USER) / put_frame (->CONVERTED) + * - consumer (transport): next_frame (CONVERTED->IN_TRANSMITTING) / + * frame_done (->FREE) + * + * The ctx is hand-initialised in the derive (no-conversion) path so put_frame + * advances a frame straight to CONVERTED, the state next_frame consumes. This + * isolates the claim/lifecycle state machine so the test can hammer it with + * many threads and assert single-ownership + conservation + deadlock-freedom. + */ + +#ifndef _ST20P_TX_PIPELINE_HARNESS_H_ +#define _ST20P_TX_PIPELINE_HARNESS_H_ + +#include +#include + +#include "mtl_api.h" +#include "st_pipeline_api.h" + +#ifdef __cplusplus +extern "C" { +#endif + +typedef struct ut20p_tx_ctx ut20p_tx_ctx; + +int ut20p_tx_init(void); + +ut20p_tx_ctx* ut20p_tx_ctx_create(int framebuff_cnt); +void ut20p_tx_ctx_destroy(ut20p_tx_ctx* ctx); + +/** + * Turn on ST20P_TX_FLAG_BLOCK_GET behaviour: init the block cond/mutex, arm the + * wake_on_destroy hook, and set the blocking get_frame timeout. Call before any + * blocking get_frame. Mirrors the block setup st20p_tx_create() performs. + */ +void ut20p_tx_ctx_enable_blocking(ut20p_tx_ctx* ctx, uint64_t timeout_ns); + +/** Wake a blocking get_frame sleeper (wraps st20p_tx_wake_block). */ +void ut20p_tx_wake_block(ut20p_tx_ctx* ctx); + +int ut20p_tx_framebuff_cnt(const ut20p_tx_ctx* ctx); + +/* producer (app) side */ +struct st_frame* ut20p_tx_get_frame(ut20p_tx_ctx* ctx); +int ut20p_tx_put_frame(ut20p_tx_ctx* ctx, struct st_frame* frame); + +/* Cancel a got frame: IN_USER -> FREE (wraps st20p_tx_put_frame_abort). */ +int ut20p_tx_put_frame_abort(ut20p_tx_ctx* ctx, struct st_frame* frame); + +/* consumer (transport) side: returns 0 and sets *idx on success, -EBUSY when + * no CONVERTED frame is pending. */ +int ut20p_tx_next_frame(ut20p_tx_ctx* ctx, uint16_t* idx); +int ut20p_tx_frame_done(ut20p_tx_ctx* ctx, uint16_t idx); + +/** + * Enable the two-phase external-frame release lifecycle + * (ST20P_TX_FLAG_EXT_FRAME_MANUAL_RELEASE). frame_done then parks the frame in + * IN_USER instead of returning it straight to FREE; the app must call + * ut20p_tx_notify_ext_frame_free() to hand the slot back. Call before starting + * any worker thread. + */ +void ut20p_tx_ctx_set_manual_release(ut20p_tx_ctx* ctx); + +/** Wraps st20p_tx_notify_ext_frame_free(): IN_USER -> FREE. */ +int ut20p_tx_notify_ext_frame_free(ut20p_tx_ctx* ctx, uint16_t idx); + +/** + * Set framebuffer i to READY state (not yet converted), so that + * tx_st20p_convert_get_frame finds it in the concurrent-converter tests. + */ +void ut20p_tx_set_frame_ready(ut20p_tx_ctx* ctx, int idx); + +/** Wraps tx_st20p_convert_get_frame() — the external converter claim. */ +struct st20_convert_frame_meta* ut20p_tx_convert_get_frame(ut20p_tx_ctx* ctx); + +/** Wraps tx_st20p_convert_put_frame(). result 0 = success, <0 = fail. */ +int ut20p_tx_convert_put_frame(ut20p_tx_ctx* ctx, struct st20_convert_frame_meta* frame, + int result); + +/** Buffer index encoded in a convert-frame meta pointer. */ +int ut20p_tx_convert_frame_idx(const struct st20_convert_frame_meta* meta); + +/* Buffer index that a user-facing frame belongs to. */ +int ut20p_tx_frame_idx(const struct st_frame* frame); + +/* 1 if every framebuffer is back in the FREE state (no leak). */ +int ut20p_tx_all_free(const ut20p_tx_ctx* ctx); + +/* Raw stat value of framebuffer i (for diagnostics). */ +int ut20p_tx_frame_stat(const ut20p_tx_ctx* ctx, int i); + +uint64_t ut20p_tx_stat_frames_sent(const ut20p_tx_ctx* ctx); + +#ifdef __cplusplus +} +#endif + +#endif /* _ST20P_TX_PIPELINE_HARNESS_H_ */ diff --git a/tests/unit/pipeline/st22p_concurrency_stress_test.cpp b/tests/unit/pipeline/st22p_concurrency_stress_test.cpp new file mode 100644 index 000000000..83bd2a7cc --- /dev/null +++ b/tests/unit/pipeline/st22p_concurrency_stress_test.cpp @@ -0,0 +1,124 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright(c) 2026 Intel Corporation + * + * Adversarial ST22p (compressed video) TX pipeline concurrency test. + * + * TxConcurrentConsumersNoDoubleClaim drives tx_st22p_next_frame from many + * threads at once. The claim ENCODED -> IN_TRANSMITTING was a scan + * (newest_available) followed by a plain atomic store -- NOT a CAS -- so two + * consumers could both observe the same ENCODED slot and both claim it. FAILS + * before the claim is a CAS, PASSES after. Direct mirror of the st20p stress + * test that first exposed this class of bug. + */ + +#include +#include +#include +#include + +#include +#include +#include +#include + +#include "pipeline/st22p_tx_harness.h" + +namespace { + +constexpr auto kRunBudget = std::chrono::seconds(45); + +inline void dwell() { + for (volatile int i = 0; i < 64; i++) { + } +} + +inline void pin_worker(std::thread& t, int slot) { + long nproc = sysconf(_SC_NPROCESSORS_ONLN); + if (nproc <= 1) return; + cpu_set_t one; + CPU_ZERO(&one); + CPU_SET(1 + (slot % (int)(nproc - 1)), &one); + pthread_setaffinity_np(t.native_handle(), sizeof(one), &one); +} + +} // namespace + +TEST(St22PipelineConcurrencyStress, TxConcurrentConsumersNoDoubleClaim) { + ASSERT_EQ(ut22p_tx_init(), 0) << "EAL init failed"; + + constexpr int kFrameCnt = 8; + constexpr int kProducers = 3; + constexpr int kConsumers = 4; + constexpr int kTarget = 50000; + + ut22p_tx_ctx* ctx = ut22p_tx_ctx_create(kFrameCnt); + ASSERT_NE(ctx, nullptr); + + /* holder[idx]: 0 free/queued, >0 owning producer id, -1 a consumer. */ + std::vector> holder(kFrameCnt); + for (auto& h : holder) h.store(0); + + std::atomic to_produce{kTarget}; + std::atomic produced{0}; + std::atomic consumed{0}; + std::atomic stop{false}; + std::atomic ownership_violation{false}; + std::atomic api_error{false}; + + auto producer = [&](int id) { + while (!stop.load(std::memory_order_relaxed)) { + if (to_produce.fetch_sub(1, std::memory_order_relaxed) <= 0) break; + struct st_frame* f = nullptr; + while ((f = ut22p_tx_get_frame(ctx)) == nullptr) { + if (stop.load(std::memory_order_relaxed)) return; + } + int idx = ut22p_tx_frame_idx(f); + if (holder[idx].exchange(id) != 0) ownership_violation.store(true); + dwell(); + if (holder[idx].exchange(0) != id) ownership_violation.store(true); + if (ut22p_tx_put_frame(ctx, f) != 0) api_error.store(true); + produced.fetch_add(1, std::memory_order_relaxed); + } + }; + + auto consumer = [&]() { + while (consumed.load(std::memory_order_relaxed) < kTarget) { + if (stop.load(std::memory_order_relaxed)) break; + uint16_t idx = 0; + if (ut22p_tx_next_frame(ctx, &idx) != 0) continue; /* nothing ENCODED */ + /* If two consumers claimed the same slot, the second exchange sees -1. */ + if (holder[idx].exchange(-1) != 0) ownership_violation.store(true); + dwell(); + if (holder[idx].exchange(0) != -1) ownership_violation.store(true); + if (ut22p_tx_frame_done(ctx, idx) != 0) api_error.store(true); + consumed.fetch_add(1, std::memory_order_relaxed); + } + }; + + std::vector threads; + for (int c = 0; c < kConsumers; c++) threads.emplace_back(consumer); + for (int p = 1; p <= kProducers; p++) threads.emplace_back(producer, p); + for (size_t i = 0; i < threads.size(); i++) pin_worker(threads[i], (int)i); + + const auto start = std::chrono::steady_clock::now(); + bool timed_out = false; + while (consumed.load(std::memory_order_relaxed) < kTarget) { + if (ownership_violation.load() || api_error.load()) break; /* fail fast */ + if (std::chrono::steady_clock::now() - start > kRunBudget) { + timed_out = true; + break; + } + std::this_thread::sleep_for(std::chrono::milliseconds(2)); + } + stop.store(true); + for (auto& t : threads) t.join(); + + EXPECT_FALSE(timed_out) << "deadlock/livelock: consumed " << consumed.load() << "/" + << kTarget; + EXPECT_FALSE(ownership_violation.load()) + << "two consumer threads claimed the same framebuffer simultaneously"; + EXPECT_FALSE(api_error.load()) << "frame_done returned an error (double-claim)"; + EXPECT_TRUE(ut22p_tx_all_free(ctx)) << "framebuffer leaked (not back to FREE)"; + + ut22p_tx_ctx_destroy(ctx); +} diff --git a/tests/unit/pipeline/st22p_concurrency_test.cpp b/tests/unit/pipeline/st22p_concurrency_test.cpp new file mode 100644 index 000000000..e478388fd --- /dev/null +++ b/tests/unit/pipeline/st22p_concurrency_test.cpp @@ -0,0 +1,233 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright(c) 2026 Intel Corporation + * + * ST22p (compressed video) pipeline-layer concurrency tests for the lock-free + * framebuffer ring. Mirrors st20p_concurrency_test: many threads hammer the + * atomic claim (compare-exchange) and assert single-ownership, conservation + * and deadlock-freedom. + * + * Both halves run in the derive (no codec) path so the app/transport state + * machine is exercised in isolation: + * TX: N app producers (get_frame/put_frame -> ENCODED) + 1 transport consumer + * (next_frame/frame_done). + * RX: 1 transport producer (inject_frame -> DECODED) + N app consumers + * (get_frame/put_frame). + */ + +#include +#include +#include +#include + +#include +#include +#include +#include + +#include "pipeline/st22p_harness.h" /* RX role: ut22p_* */ +#include "pipeline/st22p_tx_harness.h" /* TX role: ut22p_tx_* */ + +namespace { + +/* Wall-clock budget for a whole run. With each worker pinned to its own core a + * correct lock-free design finishes in well under a second; only a genuine + * livelock/deadlock approaches this. */ +constexpr auto kRunBudget = std::chrono::seconds(45); + +/* Brief on-CPU dwell that widens the ownership window so a concurrent + * violation has time to be observed by a second actor. */ +inline void dwell() { + for (volatile int i = 0; i < 64; i++) { + } +} + +/* Pin a worker to its own core so the lock-free ring is exercised, not the + * scheduler. An unpinned busy-spin loop on a multi-socket NUMA host is migrated + * and co-located by the scheduler, collapsing throughput by ~600x and tripping + * the deadlock budget on a design that is in fact lock-free. + * + * Core 0 is skipped on purpose: ut_eal_init() starts DPDK with "-c1", which + * pins the calling (main) thread to core 0 -- so the process affinity mask is + * {0} after init and must NOT be used as the candidate set. We spread workers + * across cores [1, nproc) by slot instead. Best-effort: failure leaves the + * thread unpinned. */ +inline void pin_worker(std::thread& t, int slot) { + long nproc = sysconf(_SC_NPROCESSORS_ONLN); + if (nproc <= 1) return; + cpu_set_t one; + CPU_ZERO(&one); + CPU_SET(1 + (slot % (int)(nproc - 1)), &one); + pthread_setaffinity_np(t.native_handle(), sizeof(one), &one); +} + +} // namespace + +/* ------------------------------------------------------------------ TX ---- */ + +TEST(St22PipelineConcurrency, TxMultiProducerSingleConsumerNoDeadlock) { + ASSERT_EQ(ut22p_tx_init(), 0) << "EAL init failed"; + + constexpr int kFrameCnt = 8; + constexpr int kProducers = 4; + constexpr int kTarget = 50000; + + ut22p_tx_ctx* ctx = ut22p_tx_ctx_create(kFrameCnt); + ASSERT_NE(ctx, nullptr); + + /* holder[idx]: 0 = free/queued, >0 = owning producer id, -1 = consumer. */ + std::vector> holder(kFrameCnt); + for (auto& h : holder) h.store(0); + + std::atomic to_produce{kTarget}; /* remaining production slots */ + std::atomic produced{0}; /* frames handed to transport */ + std::atomic consumed{0}; /* frames completed by transport */ + std::atomic stop{false}; + std::atomic ownership_violation{false}; + std::atomic api_error{false}; /* worker threads cannot call gtest macros */ + + auto producer = [&](int id) { + while (!stop.load(std::memory_order_relaxed)) { + if (to_produce.fetch_sub(1, std::memory_order_relaxed) <= 0) break; + struct st_frame* f = nullptr; + while ((f = ut22p_tx_get_frame(ctx)) == nullptr) { + if (stop.load(std::memory_order_relaxed)) return; + } + int idx = ut22p_tx_frame_idx(f); + if (holder[idx].exchange(id) != 0) ownership_violation.store(true); + dwell(); + if (holder[idx].exchange(0) != id) ownership_violation.store(true); + if (ut22p_tx_put_frame(ctx, f) != 0) api_error.store(true); + produced.fetch_add(1, std::memory_order_relaxed); + } + }; + + auto consumer = [&]() { + while (consumed.load(std::memory_order_relaxed) < kTarget) { + if (stop.load(std::memory_order_relaxed)) break; + uint16_t idx = 0; + if (ut22p_tx_next_frame(ctx, &idx) != 0) continue; /* nothing ENCODED yet */ + if (holder[idx].exchange(-1) != 0) ownership_violation.store(true); + dwell(); + if (holder[idx].exchange(0) != -1) ownership_violation.store(true); + if (ut22p_tx_frame_done(ctx, idx) != 0) api_error.store(true); + consumed.fetch_add(1, std::memory_order_relaxed); + } + }; + + std::vector threads; + threads.emplace_back(consumer); + for (int p = 1; p <= kProducers; p++) threads.emplace_back(producer, p); + for (size_t i = 0; i < threads.size(); i++) pin_worker(threads[i], (int)i); + + const auto start = std::chrono::steady_clock::now(); + bool timed_out = false; + while (consumed.load(std::memory_order_relaxed) < kTarget) { + if (std::chrono::steady_clock::now() - start > kRunBudget) { + timed_out = true; + break; + } + std::this_thread::sleep_for(std::chrono::milliseconds(2)); + } + stop.store(true); + for (auto& t : threads) t.join(); + + if (timed_out) { + fprintf(stderr, "[TX timeout] produced=%d consumed=%d\n", produced.load(), + consumed.load()); + for (int i = 0; i < kFrameCnt; i++) + fprintf(stderr, " buf %d stat=%d holder=%d\n", i, ut22p_tx_frame_stat(ctx, i), + holder[i].load()); + } + + EXPECT_FALSE(timed_out) << "deadlock/livelock: consumed " << consumed.load() << "/" + << kTarget; + EXPECT_FALSE(ownership_violation.load()) << "two actors held the same framebuffer"; + EXPECT_FALSE(api_error.load()) << "put_frame/frame_done returned an error"; + EXPECT_EQ(produced.load(), kTarget); + EXPECT_EQ(consumed.load(), kTarget) << "frames lost or duplicated"; + EXPECT_TRUE(ut22p_tx_all_free(ctx)) << "framebuffer leaked (not back to FREE)"; + + ut22p_tx_ctx_destroy(ctx); +} + +/* ------------------------------------------------------------------ RX ---- */ + +TEST(St22PipelineConcurrency, RxSingleProducerMultiConsumerNoDeadlock) { + ASSERT_EQ(ut22p_init(), 0) << "EAL init failed"; + + constexpr int kFrameCnt = 8; + constexpr int kConsumers = 4; + constexpr int kTarget = 50000; + + ut22p_ctx* ctx = ut22p_ctx_create(kFrameCnt); + ASSERT_NE(ctx, nullptr); + + /* holder[idx]: 0 = free/queued, >0 = owning consumer id. */ + std::vector> holder(kFrameCnt); + for (auto& h : holder) h.store(0); + + std::atomic produced{0}; + std::atomic consumed{0}; + std::atomic stop{false}; + std::atomic ownership_violation{false}; + std::atomic api_error{false}; /* worker threads cannot call gtest macros */ + + auto producer = [&]() { + uint32_t ts = 1; + while (produced.load(std::memory_order_relaxed) < kTarget) { + if (stop.load(std::memory_order_relaxed)) break; + if (ut22p_inject_frame(ctx, ST_FRAME_STATUS_COMPLETE, ts++) == 0) + produced.fetch_add(1, std::memory_order_relaxed); + /* else -EBUSY: ring full, consumers will drain it; retry. */ + } + }; + + auto consumer = [&](int id) { + while (consumed.load(std::memory_order_relaxed) < kTarget) { + if (stop.load(std::memory_order_relaxed)) break; + struct st_frame* f = ut22p_get_frame(ctx); + if (!f) continue; /* nothing DECODED yet */ + int idx = ut22p_frame_idx(f); + if (holder[idx].exchange(id) != 0) ownership_violation.store(true); + dwell(); + if (holder[idx].exchange(0) != id) ownership_violation.store(true); + if (ut22p_put_frame(ctx, f) != 0) api_error.store(true); + consumed.fetch_add(1, std::memory_order_relaxed); + } + }; + + std::vector threads; + threads.emplace_back(producer); + for (int c = 1; c <= kConsumers; c++) threads.emplace_back(consumer, c); + for (size_t i = 0; i < threads.size(); i++) pin_worker(threads[i], (int)i); + + const auto start = std::chrono::steady_clock::now(); + bool timed_out = false; + while (consumed.load(std::memory_order_relaxed) < kTarget) { + if (std::chrono::steady_clock::now() - start > kRunBudget) { + timed_out = true; + break; + } + std::this_thread::sleep_for(std::chrono::milliseconds(2)); + } + stop.store(true); + for (auto& t : threads) t.join(); + + if (timed_out) { + fprintf(stderr, "[RX timeout] produced=%d consumed=%d\n", produced.load(), + consumed.load()); + for (int i = 0; i < kFrameCnt; i++) + fprintf(stderr, " buf %d stat=%d holder=%d\n", i, ut22p_frame_stat(ctx, i), + holder[i].load()); + } + + EXPECT_FALSE(timed_out) << "deadlock/livelock: consumed " << consumed.load() << "/" + << kTarget; + EXPECT_FALSE(ownership_violation.load()) << "two consumers held the same framebuffer"; + EXPECT_FALSE(api_error.load()) << "put_frame returned an error"; + EXPECT_EQ(produced.load(), kTarget); + EXPECT_EQ(consumed.load(), kTarget) << "frames lost or duplicated"; + EXPECT_TRUE(ut22p_framebuff_cnt(ctx) == kFrameCnt); + + ut22p_ctx_destroy(ctx); +} diff --git a/tests/unit/pipeline/st22p_harness.c b/tests/unit/pipeline/st22p_harness.c new file mode 100644 index 000000000..9e261e4ac --- /dev/null +++ b/tests/unit/pipeline/st22p_harness.c @@ -0,0 +1,116 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright(c) 2026 Intel Corporation + * + * C harness for ST22p (compressed video) RX pipeline-layer concurrency unit + * tests. Mirrors st30p_harness in spirit: drives rx_st22p_frame_ready() + * directly in the derive path and stubs the transport-side libmtl symbol that + * put_frame references. + */ + +#include +#include + +#undef MTL_HAS_USDT +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wunused-variable" +#include "st2110/pipeline/st22_pipeline_rx.c" +#pragma GCC diagnostic pop + +#include "common/ut_common.h" + +/* libmtl stub: put_frame returns the codestream buffer to the transport. */ + +int st22_rx_put_framebuff(st22_rx_handle handle, void* frame) { + (void)handle; + (void)frame; + return 0; +} + +struct ut22p_ctx { + struct mtl_main_impl impl; + struct st22p_rx_ctx pipeline; + struct st22p_rx_frame* framebuffs; + int framebuff_cnt; +}; + +#include "pipeline/st22p_harness.h" + +int ut22p_init(void) { + return ut_eal_init(); +} + +ut22p_ctx* ut22p_ctx_create(int framebuff_cnt) { + ut22p_ctx* ctx = calloc(1, sizeof(*ctx)); + if (!ctx) return NULL; + + ctx->framebuff_cnt = framebuff_cnt; + ctx->impl.type = MT_HANDLE_MAIN; + + ctx->framebuffs = calloc(framebuff_cnt, sizeof(struct st22p_rx_frame)); + if (!ctx->framebuffs) { + free(ctx); + return NULL; + } + for (int i = 0; i < framebuff_cnt; i++) { + ctx->framebuffs[i].stat = ST22P_RX_FRAME_FREE; + ctx->framebuffs[i].idx = i; + /* derive frame_ready() copies dst = src, so the user frame (&dst) inherits + * src.priv; seed src.priv so put_frame() recovers the framebuf. */ + ctx->framebuffs[i].src.priv = &ctx->framebuffs[i]; + ctx->framebuffs[i].dst.priv = &ctx->framebuffs[i]; + } + + struct st22p_rx_ctx* p = &ctx->pipeline; + p->impl = &ctx->impl; + p->idx = 0; + p->socket_id = rte_socket_id(); + p->type = MT_ST22_HANDLE_PIPELINE_RX; + p->framebuff_cnt = framebuff_cnt; + p->framebuffs = ctx->framebuffs; + p->ready = true; + p->derive = true; /* output_fmt == transport_fmt: frame_ready -> DECODED */ + p->ext_frame = false; + p->transport = (st22_rx_handle)(uintptr_t)0x1; + p->block_get = false; + + return ctx; +} + +void ut22p_ctx_destroy(ut22p_ctx* ctx) { + if (!ctx) return; + free(ctx->framebuffs); + free(ctx); +} + +int ut22p_framebuff_cnt(const ut22p_ctx* ctx) { + return ctx->framebuff_cnt; +} + +int ut22p_inject_frame(ut22p_ctx* ctx, enum st_frame_status status, uint32_t timestamp) { + struct st22_rx_frame_meta meta; + memset(&meta, 0, sizeof(meta)); + meta.timestamp = timestamp; + meta.rtp_timestamp = timestamp; + meta.frame_total_size = 1; + meta.status = status; + + static uint8_t dummy_frame_storage; + return rx_st22p_frame_ready(&ctx->pipeline, &dummy_frame_storage, &meta); +} + +struct st_frame* ut22p_get_frame(ut22p_ctx* ctx) { + return st22p_rx_get_frame(&ctx->pipeline); +} + +int ut22p_put_frame(ut22p_ctx* ctx, struct st_frame* frame) { + return st22p_rx_put_frame(&ctx->pipeline, frame); +} + +int ut22p_frame_idx(const struct st_frame* frame) { + const struct st22p_rx_frame* framebuff = frame->priv; + return framebuff->idx; +} + +int ut22p_frame_stat(const ut22p_ctx* ctx, int i) { + return (int)ctx->framebuffs[i].stat; +} diff --git a/tests/unit/pipeline/st22p_harness.h b/tests/unit/pipeline/st22p_harness.h new file mode 100644 index 000000000..2da45a451 --- /dev/null +++ b/tests/unit/pipeline/st22p_harness.h @@ -0,0 +1,58 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright(c) 2026 Intel Corporation + * + * C header for ST22p (compressed video) RX pipeline-layer concurrency unit + * tests. + * + * Exposes the two role-halves of the lock-free RX framebuffer ring so a + * gtest can drive them from independent threads: + * - producer (transport): inject_frame (FREE->DECODED via frame_ready) + * - consumer (app): get_frame (DECODED->IN_USER) / put_frame (->FREE) + * + * The ctx is hand-initialised in the derive (no-decoder) path so frame_ready + * advances a frame straight to DECODED, the state get_frame consumes. This + * isolates the claim/lifecycle state machine so the test can hammer it with + * many threads and assert single-ownership + conservation + deadlock-freedom. + */ + +#ifndef _ST22P_RX_PIPELINE_HARNESS_H_ +#define _ST22P_RX_PIPELINE_HARNESS_H_ + +#include +#include + +#include "mtl_api.h" +#include "st_pipeline_api.h" + +#ifdef __cplusplus +extern "C" { +#endif + +typedef struct ut22p_ctx ut22p_ctx; + +int ut22p_init(void); + +ut22p_ctx* ut22p_ctx_create(int framebuff_cnt); +void ut22p_ctx_destroy(ut22p_ctx* ctx); + +int ut22p_framebuff_cnt(const ut22p_ctx* ctx); + +/* producer (transport) side: drive one frame to DECODED. Returns 0 on success, + * -EBUSY when no FREE framebuffer is available. */ +int ut22p_inject_frame(ut22p_ctx* ctx, enum st_frame_status status, uint32_t timestamp); + +/* consumer (app) side */ +struct st_frame* ut22p_get_frame(ut22p_ctx* ctx); +int ut22p_put_frame(ut22p_ctx* ctx, struct st_frame* frame); + +/* Buffer index that a user-facing frame belongs to. */ +int ut22p_frame_idx(const struct st_frame* frame); + +/* Raw stat value of framebuffer i (for diagnostics). */ +int ut22p_frame_stat(const ut22p_ctx* ctx, int i); + +#ifdef __cplusplus +} +#endif + +#endif /* _ST22P_RX_PIPELINE_HARNESS_H_ */ diff --git a/tests/unit/pipeline/st22p_tx_harness.c b/tests/unit/pipeline/st22p_tx_harness.c new file mode 100644 index 000000000..dfc30f724 --- /dev/null +++ b/tests/unit/pipeline/st22p_tx_harness.c @@ -0,0 +1,120 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright(c) 2026 Intel Corporation + * + * C harness for ST22p (compressed video) TX pipeline-layer concurrency unit + * tests. Includes the production translation unit so the file-local transport + * callbacks (tx_st22p_next_frame / tx_st22p_frame_done) are reachable, and + * hand-initialises the ctx in the derive path so create_transport and the + * encoder plugin are bypassed. + */ + +#include +#include + +#undef MTL_HAS_USDT +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wunused-variable" +#pragma GCC diagnostic ignored "-Wunused-but-set-variable" +#include "st2110/pipeline/st22_pipeline_tx.c" +#pragma GCC diagnostic pop + +#include "common/ut_common.h" + +struct ut22p_tx_ctx { + struct mtl_main_impl impl; + struct st22p_tx_ctx pipeline; + struct st22p_tx_frame* framebuffs; + int framebuff_cnt; +}; + +#include "pipeline/st22p_tx_harness.h" + +int ut22p_tx_init(void) { + return ut_eal_init(); +} + +ut22p_tx_ctx* ut22p_tx_ctx_create(int framebuff_cnt) { + ut22p_tx_ctx* ctx = calloc(1, sizeof(*ctx)); + if (!ctx) return NULL; + + ctx->framebuff_cnt = framebuff_cnt; + ctx->impl.type = MT_HANDLE_MAIN; + + ctx->framebuffs = calloc(framebuff_cnt, sizeof(struct st22p_tx_frame)); + if (!ctx->framebuffs) { + free(ctx); + return NULL; + } + for (int i = 0; i < framebuff_cnt; i++) { + ctx->framebuffs[i].stat = ST22P_TX_FRAME_FREE; + ctx->framebuffs[i].idx = i; + /* derive path: tx_st22p_user_frame() returns &dst, so put_frame() recovers + * the framebuf via dst.priv. */ + ctx->framebuffs[i].dst.priv = &ctx->framebuffs[i]; + } + + struct st22p_tx_ctx* p = &ctx->pipeline; + p->impl = &ctx->impl; + p->idx = 0; + p->socket_id = rte_socket_id(); + p->type = MT_ST22_HANDLE_PIPELINE_TX; + p->framebuff_cnt = framebuff_cnt; + p->framebuffs = ctx->framebuffs; + p->ready = true; + p->derive = true; /* input_fmt == transport_fmt: put_frame -> ENCODED directly */ + p->ext_frame = false; + p->encode_impl = NULL; /* derive skips the encoder notify path */ + p->transport = (st22_tx_handle)(uintptr_t)0x1; + p->block_get = false; + p->encode_block_get = false; + /* ops.flags left 0: no DROP_WHEN_LATE / USER_PACING, so tx_st22p_if_frame_late + * returns immediately and next_frame never touches the (absent) transport. */ + + return ctx; +} + +void ut22p_tx_ctx_destroy(ut22p_tx_ctx* ctx) { + if (!ctx) return; + free(ctx->framebuffs); + free(ctx); +} + +int ut22p_tx_framebuff_cnt(const ut22p_tx_ctx* ctx) { + return ctx->framebuff_cnt; +} + +struct st_frame* ut22p_tx_get_frame(ut22p_tx_ctx* ctx) { + return st22p_tx_get_frame(&ctx->pipeline); +} + +int ut22p_tx_put_frame(ut22p_tx_ctx* ctx, struct st_frame* frame) { + return st22p_tx_put_frame(&ctx->pipeline, frame); +} + +int ut22p_tx_next_frame(ut22p_tx_ctx* ctx, uint16_t* idx) { + struct st22_tx_frame_meta meta; + memset(&meta, 0, sizeof(meta)); + return tx_st22p_next_frame(&ctx->pipeline, idx, &meta); +} + +int ut22p_tx_frame_done(ut22p_tx_ctx* ctx, uint16_t idx) { + struct st22_tx_frame_meta meta; + memset(&meta, 0, sizeof(meta)); + return tx_st22p_frame_done(&ctx->pipeline, idx, &meta); +} + +int ut22p_tx_frame_idx(const struct st_frame* frame) { + const struct st22p_tx_frame* framebuff = frame->priv; + return framebuff->idx; +} + +int ut22p_tx_all_free(const ut22p_tx_ctx* ctx) { + for (int i = 0; i < ctx->framebuff_cnt; i++) { + if (ctx->framebuffs[i].stat != ST22P_TX_FRAME_FREE) return 0; + } + return 1; +} + +int ut22p_tx_frame_stat(const ut22p_tx_ctx* ctx, int i) { + return (int)ctx->framebuffs[i].stat; +} diff --git a/tests/unit/pipeline/st22p_tx_harness.h b/tests/unit/pipeline/st22p_tx_harness.h new file mode 100644 index 000000000..902eeaf87 --- /dev/null +++ b/tests/unit/pipeline/st22p_tx_harness.h @@ -0,0 +1,63 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright(c) 2026 Intel Corporation + * + * C header for ST22p (compressed video) TX pipeline-layer concurrency unit + * tests. + * + * Exposes the two role-halves of the lock-free TX framebuffer ring so a + * gtest can drive them from independent threads: + * - producer (app): get_frame (FREE->IN_USER) / put_frame (->ENCODED) + * - consumer (transport): next_frame (ENCODED->IN_TRANSMITTING) / + * frame_done (->FREE) + * + * The ctx is hand-initialised in the derive (no-encoder) path so put_frame + * advances a frame straight to ENCODED, the state next_frame consumes. This + * isolates the claim/lifecycle state machine so the test can hammer it with + * many threads and assert single-ownership + conservation + deadlock-freedom. + */ + +#ifndef _ST22P_TX_PIPELINE_HARNESS_H_ +#define _ST22P_TX_PIPELINE_HARNESS_H_ + +#include +#include + +#include "mtl_api.h" +#include "st_pipeline_api.h" + +#ifdef __cplusplus +extern "C" { +#endif + +typedef struct ut22p_tx_ctx ut22p_tx_ctx; + +int ut22p_tx_init(void); + +ut22p_tx_ctx* ut22p_tx_ctx_create(int framebuff_cnt); +void ut22p_tx_ctx_destroy(ut22p_tx_ctx* ctx); + +int ut22p_tx_framebuff_cnt(const ut22p_tx_ctx* ctx); + +/* producer (app) side */ +struct st_frame* ut22p_tx_get_frame(ut22p_tx_ctx* ctx); +int ut22p_tx_put_frame(ut22p_tx_ctx* ctx, struct st_frame* frame); + +/* consumer (transport) side: returns 0 and sets *idx on success, -EBUSY when + * no ENCODED frame is pending. */ +int ut22p_tx_next_frame(ut22p_tx_ctx* ctx, uint16_t* idx); +int ut22p_tx_frame_done(ut22p_tx_ctx* ctx, uint16_t idx); + +/* Buffer index that a user-facing frame belongs to. */ +int ut22p_tx_frame_idx(const struct st_frame* frame); + +/* 1 if every framebuffer is back in the FREE state (no leak). */ +int ut22p_tx_all_free(const ut22p_tx_ctx* ctx); + +/* Raw stat value of framebuffer i (for diagnostics). */ +int ut22p_tx_frame_stat(const ut22p_tx_ctx* ctx, int i); + +#ifdef __cplusplus +} +#endif + +#endif /* _ST22P_TX_PIPELINE_HARNESS_H_ */ diff --git a/tests/unit/pipeline/st30p_concurrency_test.cpp b/tests/unit/pipeline/st30p_concurrency_test.cpp new file mode 100644 index 000000000..caa92a18f --- /dev/null +++ b/tests/unit/pipeline/st30p_concurrency_test.cpp @@ -0,0 +1,321 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright(c) 2026 Intel Corporation + * + * ST30p (audio) pipeline-layer concurrency tests for the lock-free framebuffer + * ring. The mutex that used to serialise the frame state machine has been + * removed; claiming a slot is now a single atomic compare-exchange. These + * tests hammer that claim from many threads and assert three properties: + * + * 1. single-ownership — no buffer is ever handed to two actors at once + * (a CAS regression to a plain store trips this). + * 2. conservation — every produced frame is consumed exactly once. + * 3. deadlock-freedom — the run completes within a wall-clock budget. + * + * Topologies (mirrors the two real deployments): + * TX: N app producers (get_frame/put_frame) + 1 transport consumer + * (next_frame/frame_done). + * RX: 1 transport producer (frame_ready) + N app consumers + * (get_frame/put_frame). + */ + +#include +#include +#include +#include + +#include +#include +#include +#include + +#include "pipeline/st30p_harness.h" /* RX role: ut30p_* */ +#include "pipeline/st30p_tx_harness.h" /* TX role: ut30p_tx_* */ + +namespace { + +/* Wall-clock budget for a whole run. With each worker pinned to its own core a + * correct lock-free design finishes in well under a second; only a genuine + * livelock/deadlock approaches this. The generous margin also covers a degraded + * unpinned fallback on a multi-socket host. */ +constexpr auto kRunBudget = std::chrono::seconds(45); + +/* Brief on-CPU dwell that widens the ownership window so a concurrent + * violation has time to be observed by a second actor. */ +inline void dwell() { + for (volatile int i = 0; i < 64; i++) { + } +} + +/* Pin a worker to its own core so the lock-free ring is exercised, not the + * scheduler. An unpinned busy-spin loop on a multi-socket NUMA host is migrated + * and co-located by the scheduler, collapsing throughput by ~600x and tripping + * the deadlock budget on a design that is in fact lock-free. + * + * Core 0 is skipped on purpose: ut_eal_init() starts DPDK with "-c1", which + * pins the calling (main) thread to core 0 -- so the process affinity mask is + * {0} after init and must NOT be used as the candidate set. We spread workers + * across cores [1, nproc) by slot instead. Best-effort: failure leaves the + * thread unpinned. */ +inline void pin_worker(std::thread& t, int slot) { + long nproc = sysconf(_SC_NPROCESSORS_ONLN); + if (nproc <= 1) return; + cpu_set_t one; + CPU_ZERO(&one); + CPU_SET(1 + (slot % (int)(nproc - 1)), &one); + pthread_setaffinity_np(t.native_handle(), sizeof(one), &one); +} + +} // namespace + +/* ------------------------------------------------------------------ TX ---- */ + +TEST(St30PipelineConcurrency, TxMultiProducerSingleConsumerNoDeadlock) { + ASSERT_EQ(ut30p_tx_init(), 0) << "EAL init failed"; + + constexpr int kFrameCnt = 8; + constexpr int kProducers = 4; + constexpr int kTarget = 50000; + + ut30p_tx_ctx* ctx = ut30p_tx_ctx_create(kFrameCnt); + ASSERT_NE(ctx, nullptr); + + /* holder[idx]: 0 = free/queued, >0 = owning producer id, -1 = consumer. */ + std::vector> holder(kFrameCnt); + for (auto& h : holder) h.store(0); + + std::atomic to_produce{kTarget}; /* remaining production slots */ + std::atomic produced{0}; /* frames handed to transport */ + std::atomic consumed{0}; /* frames completed by transport */ + std::atomic stop{false}; + std::atomic ownership_violation{false}; + std::atomic api_error{false}; /* worker threads cannot call gtest macros */ + + auto producer = [&](int id) { + while (!stop.load(std::memory_order_relaxed)) { + if (to_produce.fetch_sub(1, std::memory_order_relaxed) <= 0) break; + struct st30_frame* f = nullptr; + while ((f = ut30p_tx_get_frame(ctx)) == nullptr) { + if (stop.load(std::memory_order_relaxed)) return; + } + int idx = ut30p_tx_frame_idx(f); + if (holder[idx].exchange(id) != 0) ownership_violation.store(true); + dwell(); + if (holder[idx].exchange(0) != id) ownership_violation.store(true); + if (ut30p_tx_put_frame(ctx, f) != 0) api_error.store(true); + produced.fetch_add(1, std::memory_order_relaxed); + } + }; + + auto consumer = [&]() { + while (consumed.load(std::memory_order_relaxed) < kTarget) { + if (stop.load(std::memory_order_relaxed)) break; + uint16_t idx = 0; + if (ut30p_tx_next_frame(ctx, &idx) != 0) continue; /* nothing READY yet */ + if (holder[idx].exchange(-1) != 0) ownership_violation.store(true); + dwell(); + if (holder[idx].exchange(0) != -1) ownership_violation.store(true); + if (ut30p_tx_frame_done(ctx, idx) != 0) api_error.store(true); + consumed.fetch_add(1, std::memory_order_relaxed); + } + }; + + std::vector threads; + threads.emplace_back(consumer); + for (int p = 1; p <= kProducers; p++) threads.emplace_back(producer, p); + for (size_t i = 0; i < threads.size(); i++) pin_worker(threads[i], (int)i); + + const auto start = std::chrono::steady_clock::now(); + bool timed_out = false; + while (consumed.load(std::memory_order_relaxed) < kTarget) { + if (std::chrono::steady_clock::now() - start > kRunBudget) { + timed_out = true; + break; + } + std::this_thread::sleep_for(std::chrono::milliseconds(2)); + } + stop.store(true); + for (auto& t : threads) t.join(); + + if (timed_out) { + fprintf(stderr, "[TX timeout] produced=%d consumed=%d\n", produced.load(), + consumed.load()); + for (int i = 0; i < kFrameCnt; i++) + fprintf(stderr, " buf %d stat=%d holder=%d\n", i, ut30p_tx_frame_stat(ctx, i), + holder[i].load()); + } + + EXPECT_FALSE(timed_out) << "deadlock/livelock: consumed " << consumed.load() << "/" + << kTarget; + EXPECT_FALSE(ownership_violation.load()) << "two actors held the same framebuffer"; + EXPECT_FALSE(api_error.load()) << "put_frame/frame_done returned an error"; + EXPECT_EQ(produced.load(), kTarget); + EXPECT_EQ(consumed.load(), kTarget) << "frames lost or duplicated"; + EXPECT_EQ(ut30p_tx_stat_frames_sent(ctx), (uint64_t)kTarget); + EXPECT_TRUE(ut30p_tx_all_free(ctx)) << "framebuffer leaked (not back to FREE)"; + + ut30p_tx_ctx_destroy(ctx); +} + +/* ------------------------------------------------------------------ RX ---- */ + +TEST(St30PipelineConcurrency, RxSingleProducerMultiConsumerNoDeadlock) { + ASSERT_EQ(ut30p_init(), 0) << "EAL init failed"; + + constexpr int kFrameCnt = 8; + constexpr int kConsumers = 4; + constexpr int kTarget = 50000; + + ut30p_ctx* ctx = ut30p_ctx_create(kFrameCnt); + ASSERT_NE(ctx, nullptr); + + /* holder[idx]: 0 = free/queued, >0 = owning consumer id. */ + std::vector> holder(kFrameCnt); + for (auto& h : holder) h.store(0); + + std::atomic produced{0}; + std::atomic consumed{0}; + std::atomic stop{false}; + std::atomic ownership_violation{false}; + std::atomic api_error{false}; /* worker threads cannot call gtest macros */ + + auto producer = [&]() { + uint32_t ts = 1; + while (produced.load(std::memory_order_relaxed) < kTarget) { + if (stop.load(std::memory_order_relaxed)) break; + if (ut30p_inject_frame(ctx, ST_FRAME_STATUS_COMPLETE, ts++) == 0) + produced.fetch_add(1, std::memory_order_relaxed); + /* else -EBUSY: ring full, consumers will drain it; retry. */ + } + }; + + auto consumer = [&](int id) { + while (consumed.load(std::memory_order_relaxed) < kTarget) { + if (stop.load(std::memory_order_relaxed)) break; + struct st30_frame* f = ut30p_get_frame(ctx); + if (!f) continue; /* nothing READY yet */ + int idx = ut30p_frame_idx(f); + if (holder[idx].exchange(id) != 0) ownership_violation.store(true); + dwell(); + if (holder[idx].exchange(0) != id) ownership_violation.store(true); + if (ut30p_put_frame(ctx, f) != 0) api_error.store(true); + consumed.fetch_add(1, std::memory_order_relaxed); + } + }; + + std::vector threads; + threads.emplace_back(producer); + for (int c = 1; c <= kConsumers; c++) threads.emplace_back(consumer, c); + for (size_t i = 0; i < threads.size(); i++) pin_worker(threads[i], (int)i); + + const auto start = std::chrono::steady_clock::now(); + bool timed_out = false; + while (consumed.load(std::memory_order_relaxed) < kTarget) { + if (std::chrono::steady_clock::now() - start > kRunBudget) { + timed_out = true; + break; + } + std::this_thread::sleep_for(std::chrono::milliseconds(2)); + } + stop.store(true); + for (auto& t : threads) t.join(); + + if (timed_out) { + fprintf(stderr, "[RX timeout] produced=%d consumed=%d\n", produced.load(), + consumed.load()); + for (int i = 0; i < kFrameCnt; i++) + fprintf(stderr, " buf %d stat=%d holder=%d\n", i, ut30p_frame_stat(ctx, i), + holder[i].load()); + } + + EXPECT_FALSE(timed_out) << "deadlock/livelock: consumed " << consumed.load() << "/" + << kTarget; + EXPECT_FALSE(ownership_violation.load()) << "two consumers held the same framebuffer"; + EXPECT_FALSE(api_error.load()) << "put_frame returned an error"; + EXPECT_EQ(produced.load(), kTarget); + EXPECT_EQ(consumed.load(), kTarget) << "frames lost or duplicated"; + EXPECT_TRUE(ut30p_framebuff_cnt(ctx) == kFrameCnt); + + ut30p_ctx_destroy(ctx); +} + +/* ---- TX: concurrent transport consumers TOCTOU (READY->IN_TRANSMITTING) ---- */ + +/* tx_st30p_next_frame scans for a READY frame and writes IN_TRANSMITTING with a + * plain store — no CAS. Two concurrent transport threads can both observe READY + * on the same slot before either writes IN_TRANSMITTING, producing a double- + * claim. The fix is a CAS (READY -> IN_TRANSMITTING) in next_frame. + * + * Expected: FAIL before the CAS is added; PASS after. */ +TEST(St30PipelineConcurrency, TxConcurrentTransportNoDoubleClaim) { + ASSERT_EQ(ut30p_tx_init(), 0) << "EAL init failed"; + + constexpr int kFrameCnt = 8; + constexpr int kTransports = 4; + constexpr int kTarget = 50000; + + ut30p_tx_ctx* ctx = ut30p_tx_ctx_create(kFrameCnt); + ASSERT_NE(ctx, nullptr); + + /* holder[idx]: 0 = free/ready, >0 = owning transport id. */ + std::vector> holder(kFrameCnt); + for (auto& h : holder) h.store(0); + + std::atomic claimed{0}; + std::atomic stop{false}; + std::atomic ownership_violation{false}; + std::atomic api_error{false}; + + /* Refiller: cycles FREE slots back to READY so transports always have work. */ + auto refiller = [&]() { + while (!stop.load(std::memory_order_relaxed)) { + for (int i = 0; i < kFrameCnt; i++) { + if (ut30p_tx_frame_stat(ctx, i) == 0 /* FREE */) ut30p_tx_set_frame_ready(ctx, i); + } + } + }; + + auto transport = [&](int id) { + while (claimed.load(std::memory_order_relaxed) < kTarget) { + if (stop.load(std::memory_order_relaxed)) break; + uint16_t idx = 0; + if (ut30p_tx_next_frame(ctx, &idx) != 0) continue; + if (holder[idx].exchange(id) != 0) ownership_violation.store(true); + dwell(); + if (holder[idx].exchange(0) != id) ownership_violation.store(true); + if (ut30p_tx_frame_done(ctx, idx) != 0) api_error.store(true); + claimed.fetch_add(1, std::memory_order_relaxed); + } + }; + + std::vector threads; + threads.emplace_back(refiller); + for (int t = 1; t <= kTransports; t++) threads.emplace_back(transport, t); + for (size_t i = 0; i < threads.size(); i++) pin_worker(threads[i], (int)i); + + const auto start = std::chrono::steady_clock::now(); + bool timed_out = false; + while (claimed.load(std::memory_order_relaxed) < kTarget) { + if (std::chrono::steady_clock::now() - start > kRunBudget) { + timed_out = true; + break; + } + std::this_thread::sleep_for(std::chrono::milliseconds(2)); + } + stop.store(true); + for (auto& t : threads) t.join(); + + if (timed_out) { + fprintf(stderr, "[TX-transport timeout] claimed=%d\n", claimed.load()); + for (int i = 0; i < kFrameCnt; i++) + fprintf(stderr, " buf %d stat=%d holder=%d\n", i, ut30p_tx_frame_stat(ctx, i), + holder[i].load()); + } + + EXPECT_FALSE(timed_out) << "deadlock/livelock: claimed " << claimed.load() << "/" + << kTarget; + EXPECT_FALSE(ownership_violation.load()) + << "two transport threads held the same framebuffer simultaneously"; + EXPECT_FALSE(api_error.load()) << "frame_done returned an error"; + + ut30p_tx_ctx_destroy(ctx); +} diff --git a/tests/unit/pipeline/st30p_harness.c b/tests/unit/pipeline/st30p_harness.c index 6c1b67534..f8e1099ec 100644 --- a/tests/unit/pipeline/st30p_harness.c +++ b/tests/unit/pipeline/st30p_harness.c @@ -85,18 +85,11 @@ ut30p_ctx* ut30p_ctx_create(int framebuff_cnt) { p->ready = true; p->transport = (st30_rx_handle)(uintptr_t)0x1; - if (pthread_mutex_init(&p->lock, NULL) != 0) { - free(ctx->framebuffs); - free(ctx); - return NULL; - } - return ctx; } void ut30p_ctx_destroy(ut30p_ctx* ctx) { if (!ctx) return; - pthread_mutex_destroy(&ctx->pipeline.lock); free(ctx->framebuffs); free(ctx); } @@ -121,6 +114,19 @@ int ut30p_put_frame(ut30p_ctx* ctx, struct st30_frame* frame) { return st30p_rx_put_frame(&ctx->pipeline, frame); } +int ut30p_frame_idx(const struct st30_frame* frame) { + const struct st30p_rx_frame* framebuff = frame->priv; + return framebuff->idx; +} + +int ut30p_framebuff_cnt(const ut30p_ctx* ctx) { + return ctx->framebuff_cnt; +} + +int ut30p_frame_stat(const ut30p_ctx* ctx, int i) { + return (int)ctx->framebuffs[i].stat; +} + uint64_t ut30p_stat_frames_received(const ut30p_ctx* ctx) { return ctx->pipeline.stat_frames_received; } diff --git a/tests/unit/pipeline/st30p_harness.h b/tests/unit/pipeline/st30p_harness.h index 5f7f4dc88..28a032a56 100644 --- a/tests/unit/pipeline/st30p_harness.h +++ b/tests/unit/pipeline/st30p_harness.h @@ -43,6 +43,13 @@ int ut30p_inject_frame(ut30p_ctx* ctx, enum st_frame_status status, uint32_t tim struct st30_frame* ut30p_get_frame(ut30p_ctx* ctx); int ut30p_put_frame(ut30p_ctx* ctx, struct st30_frame* frame); +/* Buffer index that a user-facing frame belongs to (for ownership tracking). */ +int ut30p_frame_idx(const struct st30_frame* frame); +int ut30p_framebuff_cnt(const ut30p_ctx* ctx); + +/* Raw stat value of framebuffer i (for diagnostics). */ +int ut30p_frame_stat(const ut30p_ctx* ctx, int i); + uint64_t ut30p_stat_frames_received(const ut30p_ctx* ctx); uint64_t ut30p_stat_frames_dropped(const ut30p_ctx* ctx); uint64_t ut30p_stat_frames_corrupted(const ut30p_ctx* ctx); diff --git a/tests/unit/pipeline/st30p_tx_harness.c b/tests/unit/pipeline/st30p_tx_harness.c new file mode 100644 index 000000000..af536181f --- /dev/null +++ b/tests/unit/pipeline/st30p_tx_harness.c @@ -0,0 +1,138 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright(c) 2026 Intel Corporation + * + * C harness for ST30p (audio) TX pipeline-layer concurrency unit tests. + * Includes the production translation unit so the file-local transport + * callbacks (tx_st30p_next_frame / tx_st30p_frame_done) are reachable, and + * stubs the libmtl transport symbols the TU references but the test never + * calls (create_transport is bypassed). + */ + +#include +#include + +#undef MTL_HAS_USDT +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wunused-variable" +#pragma GCC diagnostic ignored "-Wunused-but-set-variable" +#include "st2110/pipeline/st30_pipeline_tx.c" +#pragma GCC diagnostic pop + +#include "common/ut_common.h" + +/* libmtl stubs: only referenced via create/free/update paths we never run. */ + +int st30_tx_get_session_stats(st30_tx_handle handle, struct st30_tx_user_stats* stats) { + (void)handle; + if (stats) memset(stats, 0, sizeof(*stats)); + return 0; +} + +int st30_tx_reset_session_stats(st30_tx_handle handle) { + (void)handle; + return 0; +} + +struct ut30p_tx_ctx { + struct mtl_main_impl impl; + struct st30p_tx_ctx pipeline; + struct st30p_tx_frame* framebuffs; + int framebuff_cnt; +}; + +#include "pipeline/st30p_tx_harness.h" + +int ut30p_tx_init(void) { + return ut_eal_init(); +} + +ut30p_tx_ctx* ut30p_tx_ctx_create(int framebuff_cnt) { + ut30p_tx_ctx* ctx = calloc(1, sizeof(*ctx)); + if (!ctx) return NULL; + + ctx->framebuff_cnt = framebuff_cnt; + ctx->impl.type = MT_HANDLE_MAIN; + + ctx->framebuffs = calloc(framebuff_cnt, sizeof(struct st30p_tx_frame)); + if (!ctx->framebuffs) { + free(ctx); + return NULL; + } + for (int i = 0; i < framebuff_cnt; i++) { + ctx->framebuffs[i].stat = ST30P_TX_FRAME_FREE; + ctx->framebuffs[i].idx = i; + /* mirrors production init: put_frame() recovers framebuf via frame->priv */ + ctx->framebuffs[i].frame.priv = &ctx->framebuffs[i]; + } + + struct st30p_tx_ctx* p = &ctx->pipeline; + p->impl = &ctx->impl; + p->idx = 0; + p->socket_id = rte_socket_id(); + p->type = MT_ST30_HANDLE_PIPELINE_TX; + p->framebuff_cnt = framebuff_cnt; + p->framebuffs = ctx->framebuffs; + p->ready = true; + p->transport = (st30_tx_handle)(uintptr_t)0x1; + p->block_get = false; + p->usdt_dump_fd = -1; /* keep usdt_dump_close() a no-op (avoid close(0)) */ + p->frames_per_sec = 1000; + /* ops.flags left 0: no DROP_WHEN_LATE / USER_PACING, so tx_st30p_if_frame_late + * returns immediately and next_frame never touches the (absent) transport. */ + + return ctx; +} + +void ut30p_tx_ctx_destroy(ut30p_tx_ctx* ctx) { + if (!ctx) return; + free(ctx->framebuffs); + free(ctx); +} + +int ut30p_tx_framebuff_cnt(const ut30p_tx_ctx* ctx) { + return ctx->framebuff_cnt; +} + +struct st30_frame* ut30p_tx_get_frame(ut30p_tx_ctx* ctx) { + return st30p_tx_get_frame(&ctx->pipeline); +} + +int ut30p_tx_put_frame(ut30p_tx_ctx* ctx, struct st30_frame* frame) { + return st30p_tx_put_frame(&ctx->pipeline, frame); +} + +int ut30p_tx_next_frame(ut30p_tx_ctx* ctx, uint16_t* idx) { + struct st30_tx_frame_meta meta; + memset(&meta, 0, sizeof(meta)); + return tx_st30p_next_frame(&ctx->pipeline, idx, &meta); +} + +int ut30p_tx_frame_done(ut30p_tx_ctx* ctx, uint16_t idx) { + struct st30_tx_frame_meta meta; + memset(&meta, 0, sizeof(meta)); + return tx_st30p_frame_done(&ctx->pipeline, idx, &meta); +} + +void ut30p_tx_set_frame_ready(ut30p_tx_ctx* ctx, int idx) { + __atomic_store_n(&ctx->framebuffs[idx].stat, ST30P_TX_FRAME_READY, __ATOMIC_RELEASE); +} + +int ut30p_tx_frame_idx(const struct st30_frame* frame) { + const struct st30p_tx_frame* framebuff = frame->priv; + return framebuff->idx; +} + +int ut30p_tx_all_free(const ut30p_tx_ctx* ctx) { + for (int i = 0; i < ctx->framebuff_cnt; i++) { + if (ctx->framebuffs[i].stat != ST30P_TX_FRAME_FREE) return 0; + } + return 1; +} + +int ut30p_tx_frame_stat(const ut30p_tx_ctx* ctx, int i) { + return (int)ctx->framebuffs[i].stat; +} + +uint64_t ut30p_tx_stat_frames_sent(const ut30p_tx_ctx* ctx) { + return ctx->pipeline.stat_frames_sent; +} diff --git a/tests/unit/pipeline/st30p_tx_harness.h b/tests/unit/pipeline/st30p_tx_harness.h new file mode 100644 index 000000000..eda10fce2 --- /dev/null +++ b/tests/unit/pipeline/st30p_tx_harness.h @@ -0,0 +1,71 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright(c) 2026 Intel Corporation + * + * C header for ST30p (audio) TX pipeline-layer concurrency unit tests. + * + * Exposes the two role-halves of the lock-free TX framebuffer ring so a + * gtest can drive them from independent threads: + * - producer (app): get_frame (FREE->IN_USER) / put_frame (->READY) + * - consumer (transport): next_frame (READY->IN_TRANSMITTING) / + * frame_done (->FREE) + * + * The harness bypasses create_transport: there is no real DPDK session, the + * framebuffers are plain heap memory and the ctx is hand-initialised into the + * "ready" state. This isolates the claim/lifecycle state machine so the test + * can hammer it with many threads and assert single-ownership + conservation + + * deadlock-freedom. + */ + +#ifndef _ST30P_TX_PIPELINE_HARNESS_H_ +#define _ST30P_TX_PIPELINE_HARNESS_H_ + +#include +#include + +#include "mtl_api.h" +#include "st30_pipeline_api.h" + +#ifdef __cplusplus +extern "C" { +#endif + +typedef struct ut30p_tx_ctx ut30p_tx_ctx; + +int ut30p_tx_init(void); + +ut30p_tx_ctx* ut30p_tx_ctx_create(int framebuff_cnt); +void ut30p_tx_ctx_destroy(ut30p_tx_ctx* ctx); + +int ut30p_tx_framebuff_cnt(const ut30p_tx_ctx* ctx); + +/* producer (app) side */ +struct st30_frame* ut30p_tx_get_frame(ut30p_tx_ctx* ctx); +int ut30p_tx_put_frame(ut30p_tx_ctx* ctx, struct st30_frame* frame); + +/* consumer (transport) side: returns 0 and sets *idx on success, -EBUSY when + * no READY frame is pending. */ +int ut30p_tx_next_frame(ut30p_tx_ctx* ctx, uint16_t* idx); +int ut30p_tx_frame_done(ut30p_tx_ctx* ctx, uint16_t idx); + +/** + * Set framebuffer i directly to READY state so concurrent-transport tests + * can prime slots without going through get_frame/put_frame. + */ +void ut30p_tx_set_frame_ready(ut30p_tx_ctx* ctx, int idx); + +/* Buffer index that a user-facing frame belongs to. */ +int ut30p_tx_frame_idx(const struct st30_frame* frame); + +/* 1 if every framebuffer is back in the FREE state (no leak). */ +int ut30p_tx_all_free(const ut30p_tx_ctx* ctx); + +/* Raw stat value of framebuffer i (for diagnostics). */ +int ut30p_tx_frame_stat(const ut30p_tx_ctx* ctx, int i); + +uint64_t ut30p_tx_stat_frames_sent(const ut30p_tx_ctx* ctx); + +#ifdef __cplusplus +} +#endif + +#endif /* _ST30P_TX_PIPELINE_HARNESS_H_ */ diff --git a/tests/unit/pipeline/st40p_concurrency_test.cpp b/tests/unit/pipeline/st40p_concurrency_test.cpp new file mode 100644 index 000000000..090b62abf --- /dev/null +++ b/tests/unit/pipeline/st40p_concurrency_test.cpp @@ -0,0 +1,314 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright(c) 2026 Intel Corporation + * + * ST40p (ancillary) pipeline-layer concurrency tests for the lock-free + * framebuffer ring. Mirrors st30p_concurrency_test: many threads hammer the + * atomic claim (compare-exchange) and assert single-ownership, conservation + * and deadlock-freedom. + * + * Topologies: + * TX: N app producers (get_frame/put_frame) + 1 transport consumer + * (next_frame/frame_done). + * RX: 1 transport producer (inject_frame) + N app consumers + * (get_frame/put_frame). + */ + +#include +#include +#include +#include + +#include +#include +#include +#include + +#include "pipeline/st40p_harness.h" /* RX role: ut40p_* */ +#include "pipeline/st40p_tx_harness.h" /* TX role: ut40p_tx_* */ + +namespace { + +/* Wall-clock budget for a whole run. With each worker pinned to its own core a + * correct lock-free design finishes in well under a second; only a genuine + * livelock/deadlock approaches this. */ +constexpr auto kRunBudget = std::chrono::seconds(45); + +/* Brief on-CPU dwell that widens the ownership window so a concurrent + * violation has time to be observed by a second actor. */ +inline void dwell() { + for (volatile int i = 0; i < 64; i++) { + } +} + +/* Pin a worker to its own core so the lock-free ring is exercised, not the + * scheduler. An unpinned busy-spin loop on a multi-socket NUMA host is migrated + * and co-located by the scheduler, collapsing throughput by ~600x and tripping + * the deadlock budget on a design that is in fact lock-free. + * + * Core 0 is skipped on purpose: ut_eal_init() starts DPDK with "-c1", which + * pins the calling (main) thread to core 0 -- so the process affinity mask is + * {0} after init and must NOT be used as the candidate set. We spread workers + * across cores [1, nproc) by slot instead. Best-effort: failure leaves the + * thread unpinned. */ +inline void pin_worker(std::thread& t, int slot) { + long nproc = sysconf(_SC_NPROCESSORS_ONLN); + if (nproc <= 1) return; + cpu_set_t one; + CPU_ZERO(&one); + CPU_SET(1 + (slot % (int)(nproc - 1)), &one); + pthread_setaffinity_np(t.native_handle(), sizeof(one), &one); +} + +} // namespace + +/* ------------------------------------------------------------------ TX ---- */ + +TEST(St40PipelineConcurrency, TxMultiProducerSingleConsumerNoDeadlock) { + ASSERT_EQ(ut40p_tx_init(), 0) << "EAL init failed"; + + constexpr int kFrameCnt = 8; + constexpr int kProducers = 4; + constexpr int kTarget = 50000; + + ut40p_tx_ctx* ctx = ut40p_tx_ctx_create(kFrameCnt); + ASSERT_NE(ctx, nullptr); + + /* holder[idx]: 0 = free/queued, >0 = owning producer id, -1 = consumer. */ + std::vector> holder(kFrameCnt); + for (auto& h : holder) h.store(0); + + std::atomic to_produce{kTarget}; /* remaining production slots */ + std::atomic produced{0}; /* frames handed to transport */ + std::atomic consumed{0}; /* frames completed by transport */ + std::atomic stop{false}; + std::atomic ownership_violation{false}; + std::atomic api_error{false}; /* worker threads cannot call gtest macros */ + + auto producer = [&](int id) { + while (!stop.load(std::memory_order_relaxed)) { + if (to_produce.fetch_sub(1, std::memory_order_relaxed) <= 0) break; + struct st40_frame_info* f = nullptr; + while ((f = ut40p_tx_get_frame(ctx)) == nullptr) { + if (stop.load(std::memory_order_relaxed)) return; + } + int idx = ut40p_tx_frame_idx(f); + if (holder[idx].exchange(id) != 0) ownership_violation.store(true); + dwell(); + if (holder[idx].exchange(0) != id) ownership_violation.store(true); + if (ut40p_tx_put_frame(ctx, f) != 0) api_error.store(true); + produced.fetch_add(1, std::memory_order_relaxed); + } + }; + + auto consumer = [&]() { + while (consumed.load(std::memory_order_relaxed) < kTarget) { + if (stop.load(std::memory_order_relaxed)) break; + uint16_t idx = 0; + if (ut40p_tx_next_frame(ctx, &idx) != 0) continue; /* nothing READY yet */ + if (holder[idx].exchange(-1) != 0) ownership_violation.store(true); + dwell(); + if (holder[idx].exchange(0) != -1) ownership_violation.store(true); + if (ut40p_tx_frame_done(ctx, idx) != 0) api_error.store(true); + consumed.fetch_add(1, std::memory_order_relaxed); + } + }; + + std::vector threads; + threads.emplace_back(consumer); + for (int p = 1; p <= kProducers; p++) threads.emplace_back(producer, p); + for (size_t i = 0; i < threads.size(); i++) pin_worker(threads[i], (int)i); + + const auto start = std::chrono::steady_clock::now(); + bool timed_out = false; + while (consumed.load(std::memory_order_relaxed) < kTarget) { + if (std::chrono::steady_clock::now() - start > kRunBudget) { + timed_out = true; + break; + } + std::this_thread::sleep_for(std::chrono::milliseconds(2)); + } + stop.store(true); + for (auto& t : threads) t.join(); + + if (timed_out) { + fprintf(stderr, "[TX timeout] produced=%d consumed=%d\n", produced.load(), + consumed.load()); + for (int i = 0; i < kFrameCnt; i++) + fprintf(stderr, " buf %d stat=%d holder=%d\n", i, ut40p_tx_frame_stat(ctx, i), + holder[i].load()); + } + + EXPECT_FALSE(timed_out) << "deadlock/livelock: consumed " << consumed.load() << "/" + << kTarget; + EXPECT_FALSE(ownership_violation.load()) << "two actors held the same framebuffer"; + EXPECT_FALSE(api_error.load()) << "put_frame/frame_done returned an error"; + EXPECT_EQ(produced.load(), kTarget); + EXPECT_EQ(consumed.load(), kTarget) << "frames lost or duplicated"; + EXPECT_EQ(ut40p_tx_stat_frames_sent(ctx), (uint64_t)kTarget); + EXPECT_TRUE(ut40p_tx_all_free(ctx)) << "framebuffer leaked (not back to FREE)"; + + ut40p_tx_ctx_destroy(ctx); +} + +/* ------------------------------------------------------------------ RX ---- */ + +TEST(St40PipelineConcurrency, RxSingleProducerMultiConsumerNoDeadlock) { + ASSERT_EQ(ut40p_init(), 0) << "EAL init failed"; + + constexpr int kFrameCnt = 8; + constexpr int kConsumers = 4; + constexpr int kTarget = 50000; + + ut40p_ctx* ctx = ut40p_ctx_create(kFrameCnt); + ASSERT_NE(ctx, nullptr); + + /* holder[idx]: 0 = free/queued, >0 = owning consumer id. */ + std::vector> holder(kFrameCnt); + for (auto& h : holder) h.store(0); + + std::atomic produced{0}; + std::atomic consumed{0}; + std::atomic stop{false}; + std::atomic ownership_violation{false}; + std::atomic api_error{false}; /* worker threads cannot call gtest macros */ + + auto producer = [&]() { + uint32_t ts = 1; + while (produced.load(std::memory_order_relaxed) < kTarget) { + if (stop.load(std::memory_order_relaxed)) break; + if (ut40p_inject_frame(ctx, (void*)(uintptr_t)0x1, ST_FRAME_STATUS_COMPLETE, false, + ts++) == 0) + produced.fetch_add(1, std::memory_order_relaxed); + /* else -EBUSY: ring full, consumers will drain it; retry. */ + } + }; + + auto consumer = [&](int id) { + while (consumed.load(std::memory_order_relaxed) < kTarget) { + if (stop.load(std::memory_order_relaxed)) break; + struct st40_frame_info* f = ut40p_get_frame(ctx); + if (!f) continue; /* nothing READY yet */ + int idx = ut40p_frame_idx(f); + if (holder[idx].exchange(id) != 0) ownership_violation.store(true); + dwell(); + if (holder[idx].exchange(0) != id) ownership_violation.store(true); + if (ut40p_put_frame(ctx, f) != 0) api_error.store(true); + consumed.fetch_add(1, std::memory_order_relaxed); + } + }; + + std::vector threads; + threads.emplace_back(producer); + for (int c = 1; c <= kConsumers; c++) threads.emplace_back(consumer, c); + for (size_t i = 0; i < threads.size(); i++) pin_worker(threads[i], (int)i); + + const auto start = std::chrono::steady_clock::now(); + bool timed_out = false; + while (consumed.load(std::memory_order_relaxed) < kTarget) { + if (std::chrono::steady_clock::now() - start > kRunBudget) { + timed_out = true; + break; + } + std::this_thread::sleep_for(std::chrono::milliseconds(2)); + } + stop.store(true); + for (auto& t : threads) t.join(); + + if (timed_out) { + fprintf(stderr, "[RX timeout] produced=%d consumed=%d\n", produced.load(), + consumed.load()); + for (int i = 0; i < kFrameCnt; i++) + fprintf(stderr, " buf %d stat=%d holder=%d\n", i, ut40p_frame_stat(ctx, i), + holder[i].load()); + } + + EXPECT_FALSE(timed_out) << "deadlock/livelock: consumed " << consumed.load() << "/" + << kTarget; + EXPECT_FALSE(ownership_violation.load()) << "two consumers held the same framebuffer"; + EXPECT_FALSE(api_error.load()) << "put_frame returned an error"; + EXPECT_EQ(produced.load(), kTarget); + EXPECT_EQ(consumed.load(), kTarget) << "frames lost or duplicated"; + EXPECT_TRUE(ut40p_framebuff_cnt(ctx) == kFrameCnt); + + ut40p_ctx_destroy(ctx); +} + +/* ---- TX: concurrent transport consumers TOCTOU (READY->IN_TRANSMITTING) ---- */ + +/* tx_st40p_next_frame scans for a READY frame and writes IN_TRANSMITTING with a + * plain store — no CAS. Two concurrent transport threads can both observe READY + * on the same slot before either writes IN_TRANSMITTING, producing a double- + * claim. The fix is a CAS (READY -> IN_TRANSMITTING) in next_frame. + * + * Expected: FAIL before the CAS is added; PASS after. */ +TEST(St40PipelineConcurrency, TxConcurrentTransportNoDoubleClaim) { + ASSERT_EQ(ut40p_tx_init(), 0) << "EAL init failed"; + + constexpr int kFrameCnt = 8; + constexpr int kTransports = 4; + constexpr int kTarget = 50000; + + ut40p_tx_ctx* ctx = ut40p_tx_ctx_create(kFrameCnt); + ASSERT_NE(ctx, nullptr); + + std::vector> holder(kFrameCnt); + for (auto& h : holder) h.store(0); + + std::atomic claimed{0}; + std::atomic stop{false}; + std::atomic ownership_violation{false}; + std::atomic api_error{false}; + + auto refiller = [&]() { + while (!stop.load(std::memory_order_relaxed)) { + for (int i = 0; i < kFrameCnt; i++) { + if (ut40p_tx_frame_stat(ctx, i) == 0 /* FREE */) ut40p_tx_set_frame_ready(ctx, i); + } + } + }; + + auto transport = [&](int id) { + while (claimed.load(std::memory_order_relaxed) < kTarget) { + if (stop.load(std::memory_order_relaxed)) break; + uint16_t idx = 0; + if (ut40p_tx_next_frame(ctx, &idx) != 0) continue; + if (holder[idx].exchange(id) != 0) ownership_violation.store(true); + dwell(); + if (holder[idx].exchange(0) != id) ownership_violation.store(true); + if (ut40p_tx_frame_done(ctx, idx) != 0) api_error.store(true); + claimed.fetch_add(1, std::memory_order_relaxed); + } + }; + + std::vector threads; + threads.emplace_back(refiller); + for (int t = 1; t <= kTransports; t++) threads.emplace_back(transport, t); + for (size_t i = 0; i < threads.size(); i++) pin_worker(threads[i], (int)i); + + const auto start = std::chrono::steady_clock::now(); + bool timed_out = false; + while (claimed.load(std::memory_order_relaxed) < kTarget) { + if (std::chrono::steady_clock::now() - start > kRunBudget) { + timed_out = true; + break; + } + std::this_thread::sleep_for(std::chrono::milliseconds(2)); + } + stop.store(true); + for (auto& t : threads) t.join(); + + if (timed_out) { + fprintf(stderr, "[TX-transport timeout] claimed=%d\n", claimed.load()); + for (int i = 0; i < kFrameCnt; i++) + fprintf(stderr, " buf %d stat=%d holder=%d\n", i, ut40p_tx_frame_stat(ctx, i), + holder[i].load()); + } + + EXPECT_FALSE(timed_out) << "deadlock/livelock: claimed " << claimed.load() << "/" + << kTarget; + EXPECT_FALSE(ownership_violation.load()) + << "two transport threads held the same ST40 framebuffer simultaneously"; + EXPECT_FALSE(api_error.load()) << "frame_done returned an error"; + + ut40p_tx_ctx_destroy(ctx); +} diff --git a/tests/unit/pipeline/st40p_harness.c b/tests/unit/pipeline/st40p_harness.c index 6af49d05b..839f2f225 100644 --- a/tests/unit/pipeline/st40p_harness.c +++ b/tests/unit/pipeline/st40p_harness.c @@ -101,12 +101,6 @@ ut40p_ctx* ut40p_ctx_create(int framebuff_cnt) { p->ready = true; p->transport = (st40_rx_handle)(uintptr_t)0x1; - if (mt_pthread_mutex_init(&p->lock, NULL) != 0) { - free(ctx->framebuffs); - free(ctx); - return NULL; - } - ut40p_put_framebuff_reset_spy(); return ctx; @@ -114,7 +108,6 @@ ut40p_ctx* ut40p_ctx_create(int framebuff_cnt) { void ut40p_ctx_destroy(ut40p_ctx* ctx) { if (!ctx) return; - pthread_mutex_destroy(&ctx->pipeline.lock); free(ctx->framebuffs); free(ctx); } @@ -145,6 +138,23 @@ int ut40p_put_frame_abort(ut40p_ctx* ctx, struct st40_frame_info* frame) { return st40p_rx_put_frame_abort(&ctx->pipeline, frame); } +int ut40p_frame_idx(const struct st40_frame_info* frame) { + const struct st40p_rx_frame* framebuff = frame->priv; + return framebuff->idx; +} + +int ut40p_framebuff_cnt(const ut40p_ctx* ctx) { + return ctx->framebuff_cnt; +} + +int ut40p_frame_stat(const ut40p_ctx* ctx, int i) { + return (int)ctx->framebuffs[i].stat; +} + +uint32_t ut40p_stat_drop_frame(const ut40p_ctx* ctx) { + return ctx->pipeline.stat_drop_frame; +} + uint64_t ut40p_stat_frames_received(const ut40p_ctx* ctx) { return ctx->pipeline.stat_frames_received; } diff --git a/tests/unit/pipeline/st40p_harness.h b/tests/unit/pipeline/st40p_harness.h index 8c1f2e2f7..96b5399f7 100644 --- a/tests/unit/pipeline/st40p_harness.h +++ b/tests/unit/pipeline/st40p_harness.h @@ -44,6 +44,18 @@ struct st40_frame_info* ut40p_get_frame(ut40p_ctx* ctx); int ut40p_put_frame(ut40p_ctx* ctx, struct st40_frame_info* frame); int ut40p_put_frame_abort(ut40p_ctx* ctx, struct st40_frame_info* frame); +/* ── concurrency-test helpers ─────────────────────────────────────────── */ + +/** Buffer index that a user-facing frame belongs to. */ +int ut40p_frame_idx(const struct st40_frame_info* frame); + +/** Total framebuffer count. */ +int ut40p_framebuff_cnt(const ut40p_ctx* ctx); + +/** Raw stat value of framebuffer i (for diagnostics). */ +int ut40p_frame_stat(const ut40p_ctx* ctx, int i); + +uint32_t ut40p_stat_drop_frame(const ut40p_ctx* ctx); uint64_t ut40p_stat_frames_received(const ut40p_ctx* ctx); uint64_t ut40p_stat_frames_dropped(const ut40p_ctx* ctx); uint64_t ut40p_stat_frames_corrupted(const ut40p_ctx* ctx); diff --git a/tests/unit/pipeline/st40p_tx_harness.c b/tests/unit/pipeline/st40p_tx_harness.c new file mode 100644 index 000000000..2c326a1d2 --- /dev/null +++ b/tests/unit/pipeline/st40p_tx_harness.c @@ -0,0 +1,130 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright(c) 2026 Intel Corporation + * + * C harness for ST40p (ancillary) TX pipeline-layer concurrency unit tests. + * Includes the production translation unit so the file-local transport + * callbacks (tx_st40p_next_frame / tx_st40p_frame_done) are reachable, and + * hand-initialises the ctx so create_transport is bypassed. + */ + +#include +#include + +#undef MTL_HAS_USDT +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wunused-variable" +#pragma GCC diagnostic ignored "-Wunused-but-set-variable" +#include "st2110/pipeline/st40_pipeline_tx.c" +#pragma GCC diagnostic pop + +#include "common/ut_common.h" + +struct ut40p_tx_ctx { + struct mtl_main_impl impl; + struct st40p_tx_ctx pipeline; + struct st40p_tx_frame* framebuffs; + struct st40_frame* anc_frames; /* put_frame writes anc_frame->meta_num/data_size */ + int framebuff_cnt; +}; + +#include "pipeline/st40p_tx_harness.h" + +int ut40p_tx_init(void) { + return ut_eal_init(); +} + +ut40p_tx_ctx* ut40p_tx_ctx_create(int framebuff_cnt) { + ut40p_tx_ctx* ctx = calloc(1, sizeof(*ctx)); + if (!ctx) return NULL; + + ctx->framebuff_cnt = framebuff_cnt; + ctx->impl.type = MT_HANDLE_MAIN; + + ctx->framebuffs = calloc(framebuff_cnt, sizeof(struct st40p_tx_frame)); + ctx->anc_frames = calloc(framebuff_cnt, sizeof(struct st40_frame)); + if (!ctx->framebuffs || !ctx->anc_frames) { + free(ctx->anc_frames); + free(ctx->framebuffs); + free(ctx); + return NULL; + } + for (int i = 0; i < framebuff_cnt; i++) { + ctx->framebuffs[i].stat = ST40P_TX_FRAME_FREE; + ctx->framebuffs[i].idx = i; + /* mirrors production init: put_frame() recovers framebuf via frame_info->priv */ + ctx->framebuffs[i].frame_info.priv = &ctx->framebuffs[i]; + /* put_frame() writes meta_num/data_size into the assigned anc frame */ + ctx->framebuffs[i].anc_frame = &ctx->anc_frames[i]; + } + + struct st40p_tx_ctx* p = &ctx->pipeline; + p->impl = &ctx->impl; + p->idx = 0; + p->socket_id = rte_socket_id(); + p->type = MT_ST40_HANDLE_PIPELINE_TX; + p->framebuff_cnt = framebuff_cnt; + p->framebuffs = ctx->framebuffs; + p->ready = true; + p->transport = (st40_tx_handle)(uintptr_t)0x1; + p->block_get = false; + p->frames_per_sec = 1000; + /* ops.flags left 0: no DROP_WHEN_LATE / USER_PACING, so tx_st40p_if_frame_late + * returns immediately and next_frame never touches the (absent) transport. */ + + return ctx; +} + +void ut40p_tx_ctx_destroy(ut40p_tx_ctx* ctx) { + if (!ctx) return; + free(ctx->anc_frames); + free(ctx->framebuffs); + free(ctx); +} + +int ut40p_tx_framebuff_cnt(const ut40p_tx_ctx* ctx) { + return ctx->framebuff_cnt; +} + +struct st40_frame_info* ut40p_tx_get_frame(ut40p_tx_ctx* ctx) { + return st40p_tx_get_frame(&ctx->pipeline); +} + +int ut40p_tx_put_frame(ut40p_tx_ctx* ctx, struct st40_frame_info* frame) { + return st40p_tx_put_frame(&ctx->pipeline, frame); +} + +int ut40p_tx_next_frame(ut40p_tx_ctx* ctx, uint16_t* idx) { + struct st40_tx_frame_meta meta; + memset(&meta, 0, sizeof(meta)); + return tx_st40p_next_frame(&ctx->pipeline, idx, &meta); +} + +int ut40p_tx_frame_done(ut40p_tx_ctx* ctx, uint16_t idx) { + struct st40_tx_frame_meta meta; + memset(&meta, 0, sizeof(meta)); + return tx_st40p_frame_done(&ctx->pipeline, idx, &meta); +} + +void ut40p_tx_set_frame_ready(ut40p_tx_ctx* ctx, int idx) { + __atomic_store_n(&ctx->framebuffs[idx].stat, ST40P_TX_FRAME_READY, __ATOMIC_RELEASE); +} + +int ut40p_tx_frame_idx(const struct st40_frame_info* frame) { + const struct st40p_tx_frame* framebuff = frame->priv; + return framebuff->idx; +} + +int ut40p_tx_all_free(const ut40p_tx_ctx* ctx) { + for (int i = 0; i < ctx->framebuff_cnt; i++) { + if (ctx->framebuffs[i].stat != ST40P_TX_FRAME_FREE) return 0; + } + return 1; +} + +int ut40p_tx_frame_stat(const ut40p_tx_ctx* ctx, int i) { + return (int)ctx->framebuffs[i].stat; +} + +uint64_t ut40p_tx_stat_frames_sent(const ut40p_tx_ctx* ctx) { + return ctx->pipeline.stat_frames_sent; +} diff --git a/tests/unit/pipeline/st40p_tx_harness.h b/tests/unit/pipeline/st40p_tx_harness.h new file mode 100644 index 000000000..8d14ed416 --- /dev/null +++ b/tests/unit/pipeline/st40p_tx_harness.h @@ -0,0 +1,71 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright(c) 2026 Intel Corporation + * + * C header for ST40p (ancillary) TX pipeline-layer concurrency unit tests. + * + * Exposes the two role-halves of the lock-free TX framebuffer ring so a + * gtest can drive them from independent threads: + * - producer (app): get_frame (FREE->IN_USER) / put_frame (->READY) + * - consumer (transport): next_frame (READY->IN_TRANSMITTING) / + * frame_done (->FREE) + * + * The harness bypasses create_transport: there is no real DPDK session, the + * framebuffers are plain heap memory and the ctx is hand-initialised into the + * "ready" state. This isolates the claim/lifecycle state machine so the test + * can hammer it with many threads and assert single-ownership + conservation + + * deadlock-freedom. + */ + +#ifndef _ST40P_TX_PIPELINE_HARNESS_H_ +#define _ST40P_TX_PIPELINE_HARNESS_H_ + +#include +#include + +#include "mtl_api.h" +#include "st40_pipeline_api.h" + +#ifdef __cplusplus +extern "C" { +#endif + +typedef struct ut40p_tx_ctx ut40p_tx_ctx; + +int ut40p_tx_init(void); + +ut40p_tx_ctx* ut40p_tx_ctx_create(int framebuff_cnt); +void ut40p_tx_ctx_destroy(ut40p_tx_ctx* ctx); + +int ut40p_tx_framebuff_cnt(const ut40p_tx_ctx* ctx); + +/* producer (app) side */ +struct st40_frame_info* ut40p_tx_get_frame(ut40p_tx_ctx* ctx); +int ut40p_tx_put_frame(ut40p_tx_ctx* ctx, struct st40_frame_info* frame); + +/* consumer (transport) side: returns 0 and sets *idx on success, -EBUSY when + * no READY frame is pending. */ +int ut40p_tx_next_frame(ut40p_tx_ctx* ctx, uint16_t* idx); +int ut40p_tx_frame_done(ut40p_tx_ctx* ctx, uint16_t idx); + +/** + * Set framebuffer i directly to READY state so concurrent-transport tests + * can prime slots without going through get_frame/put_frame. + */ +void ut40p_tx_set_frame_ready(ut40p_tx_ctx* ctx, int idx); + +/* Buffer index that a user-facing frame belongs to. */ +int ut40p_tx_frame_idx(const struct st40_frame_info* frame); + +/* 1 if every framebuffer is back in the FREE state (no leak). */ +int ut40p_tx_all_free(const ut40p_tx_ctx* ctx); + +/* Raw stat value of framebuffer i (for diagnostics). */ +int ut40p_tx_frame_stat(const ut40p_tx_ctx* ctx, int i); + +uint64_t ut40p_tx_stat_frames_sent(const ut40p_tx_ctx* ctx); + +#ifdef __cplusplus +} +#endif + +#endif /* _ST40P_TX_PIPELINE_HARNESS_H_ */