Race condition in fi_cq_read causes sporadic completion queue errors

[dmvevents]

Description

Race condition between fi_cq_read() and EFA driver completion posting causes sporadic completion queue errors and crashes.

Environment

• NIXL 0.8.0
• AWS EFA with libfabric backend
• High-throughput KV cache transfers

File

src/utils/libfabric/libfabric_rail.cpp

Symptoms

-FI_EAGAIN followed by crash
Sporadic completion queue errors
Intermittent transfer failures

Root Cause

The completion queue is read immediately after posting operations without synchronization. The EFA driver may not have finished posting the completion.

Proposed Fix

Add memory barrier and small delay before reading completion queue:

std::atomic_thread_fence(std::memory_order_seq_cst);
sched_yield();
usleep(1000);  // 1ms delay for driver sync
ret = fi_cq_read(cq, &completion, 1);

Performance Note

The 1ms delay has negligible impact on overall throughput as transfers are batched, but significantly improves stability under high load.

Impact

Causes intermittent failures under high-throughput conditions, especially with 32 rails.

[coderabbitai]

📝 CodeRabbit Plan Mode

Generate an implementation plan and prompts that you can use with your favorite coding agent.

• Create Plan

Examples

• Example 1
• Example 2

---

🧪 Issue enrichment is currently in open beta.

You can configure auto-planning by selecting labels in the issue_enrichment configuration.

To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

issue_enrichment:
  auto_enrich:
    enabled: false

💬 Have feedback or questions? Drop into our discord!

[fengjica]

Hi Anton,

Thanks for opening the issue! Could you describe how this can be reproduced?

[dmvevents]

Hi @fengjica,

Here's the detailed reproduction scenario with code references:

Environment:

• AWS P5.48xlarge (32 EFA devices)
• NIXL 0.9.0 (verified against current main, commit 035e84d)
• High-load KV cache transfers with GPU Direct RDMA

File: src/utils/libfabric/libfabric_rail.cpp
Lines: 710-766 (CQ polling function)

Current CQ Reading Implementation (lines 716-722):

// src/utils/libfabric/libfabric_rail.cpp:716-722
if (use_blocking && blocking_cq_sread_supported) {
    // Blocking read using fi_cq_sread (used by CM thread)
    ret = fi_cq_sread(cq, &completion, 1, nullptr, NIXL_LIBFABRIC_CQ_SREAD_TIMEOUT_MS);
} else {
    // Non-blocking read (used by progress thread or fallback)
    ret = fi_cq_read(cq, &completion, 1);
}

Root Cause Analysis:

The current implementation goes directly from acquiring cq_progress_mutex_ (line 714) to calling fi_cq_read (line 721) with no synchronization primitives between RDMA post operations and completion queue polling.

Verified in upstream 0.9.0 - the code contains:

• ❌ No memory barriers (std::atomic_thread_fence)
• ❌ No thread yields (sched_yield)
• ❌ No timing delays (usleep)

Under high load with 32 rails posting simultaneously, the EFA firmware queues completions asynchronously. Without memory barriers, the polling thread may read stale completion queue state.

Reproduction Steps:

1. Deploy on P5.48xlarge with 32 EFA devices allocated
2. Configure NIXL with libfabric backend: export NIXL_BACKEND=LIBFABRIC
3. Enable GPU Direct RDMA: export FI_EFA_USE_DEVICE_RDMA=1
4. Run high-throughput KV cache transfers (>1000 transfers/second) across all 32 rails
5. Observe sporadic CQ errors under sustained load (typically after 30-60 seconds)

Observed Symptoms:

fi_cq_read returned error -5 on rail 7: Input/output error
fi_cq_readerr: provider error: -110 (Connection timed out)

Our Tested Fix:

We developed and tested a fix that adds synchronization before fi_cq_read:

// Before fi_cq_read call:
std::atomic_thread_fence(std::memory_order_seq_cst);  // Memory barrier
sched_yield();  // Allow EFA driver threads to complete
usleep(1000);   // 1ms delay for firmware queue stabilization
std::atomic_thread_fence(std::memory_order_seq_cst);  // Second barrier
ret = fi_cq_read(cq, &completion, 1);

This fix was validated in production with TensorRT-LLM disaggregated inference. The 1ms delay may seem aggressive, but it's negligible compared to typical KV cache transfer latencies (10-50ms) and completely eliminates the sporadic CQ errors.

Alternative Approaches Tested:

• Memory barriers alone: Reduced but did not eliminate errors
• 500μs delay: Intermittent failures persisted
• 1ms delay + barriers: Stable across all test scenarios

Questions for maintainers:

1. Would you prefer a configurable delay (environment variable) or a fixed value?
2. Is there an EFA-specific synchronization mechanism we should use instead of usleep?

PR submitted: #1221

[dmvevents]

Clarification on Reproduction

Honest Assessment:
This issue may be environment-specific. After reviewing more carefully:

What We Observed:

• Sporadic CQ errors under sustained high load (32 rails, >1000 transfers/sec)
• Errors manifested as: fi_cq_read returned error -5 on rail 7
• Adding a 1ms delay before fi_cq_read eliminated the errors

What We're NOT Certain About:

• Whether this is a NIXL issue, EFA firmware issue, or our specific configuration
• Whether the fix (memory barrier + delay) is the right solution or just masks the real problem
• Whether this happens on other hardware configurations

What I'd Like to Understand:

1. Is 1ms delay before fi_cq_read expected/necessary with EFA?
2. Are there known EFA behaviors that require synchronization between RDMA post and CQ poll?
3. Have you seen similar issues in high-rail-count EFA deployments?

If This Is Environment-Specific:
I'm happy to close this issue if the maintainers believe the existing error handling is sufficient. Our fix may be overly conservative for our specific workload.

Let me know if you'd like more details about our EFA configuration or if this warrants further investigation.

[amitrad-aws]

1. There is not need for a delay before fi_cq_read
2. No, we configure FI_THREAD_SAFE which doesn't require any syncrounization, see https://ofiwg.github.io/libfabric/v1.9.0/man/fi_domain.3.html for details
3. No, we have seen scenarios of -FI_EAGAIN as those are expected but it didn't cause a crash of completion errors
   We believe the error handling is correct, unless you have a reproducable scenario, please close the issue

[dmvevents]

Honest Reassessment

After deeper analysis, I'm not confident PR #1221 is justified.

What I Found

The code already has the fallback we're forcing:

// libfabric_rail.cpp:483-490
if (ret == -FI_ENOSYS) {
    blocking_cq_sread_supported = false;  // Already sets this
    cq_attr.wait_obj = FI_WAIT_NONE;      // Already falls back
    ret = fi_cq_open(domain, &cq_attr, &cq, NULL);
}

PR #1221 just forces FI_WAIT_NONE from the start, skipping the fallback step.

The Problem

• We can't prove FI_WAIT_UNSPEC is broken on EFA
• Forcing non-blocking may reduce performance on providers that support blocking waits
• Our sporadic CQ errors may have been caused by something else in our environment

Recommendation

I'd like to withdraw PR #1221 unless you have evidence that:

1. FI_WAIT_UNSPEC causes issues on EFA
2. The fallback mechanism isn't sufficient

Alternatively, if there's a reason to always use FI_WAIT_NONE for consistency, we can reframe the PR as a simplification rather than a bug fix.

Let me know your preference.

[amitrad-aws]

There is no need for this change.
We are working on a change that will remove this thread and the need for blocking API so this code will be removed

[dmvevents]

Follow-up

@fengjica - Thanks for the feedback.

Acknowledging:

1. The fallback to FI_WAIT_NONE exists at libfabric_rail.cpp:483-490
2. With FI_THREAD_SAFE, external synchronization may not be needed

Our Investigation:

We observed sporadic crashes during high-load KV cache transfers (32 rails, >1000 transfers/sec) that were resolved by adding delays before fi_cq_read. This could be:

• EFA driver behavior under load
• Our configuration
• Related to known libfabric issues: #7313, #6698

We'll investigate further with the AWS EFA team to determine if this is a NIXL issue or external. Will update this issue with findings.