|WARNING| eal: remove atomic flag operations

dpdklab at iol.unh.edu dpdklab at iol.unh.edu
Fri Sep 18 00:03:08 CEST 2026


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

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-09-17

# DPDK Patch Review

## Patch Summary
This patch removes the `rte_atomic_flag_test_and_set_explicit` and `rte_atomic_flag_clear_explicit` macros from the EAL atomic API, claiming they are not portable and have no in-tree users.

---

## ERRORS

None identified. The patch appears correct for its stated purpose.

---

## WARNINGS

### 1. Removal timing and deprecation process
This patch removes API without following the standard deprecation process (announce in one release, remove in next). While the commit message states "Removed without a deprecation notice as the API cannot be used as specified," this should be verified by maintainers. The claim that the API "cannot be used portably" should be validated - if any out-of-tree code uses these macros (even incorrectly), this is still an ABI break.

**Suggested action**: Maintainers should confirm that skipping deprecation is acceptable given the portability issues described.

### 2. Release notes formatting - migration path clarity
The release notes explain what was removed and why, but the migration suggestion could be more explicit with a code example showing the before/after pattern.

**Suggested improvement**: Add a brief code snippet showing the replacement pattern:
```rst
Applications using these macros should migrate to:

.. code-block:: c

   /* Old (removed): */
   rte_atomic_flag_test_and_set_explicit(&flag, order);
   
   /* New: */
   RTE_ATOMIC(bool) flag;
   rte_atomic_exchange_explicit(&flag, true, order);
```

---

## INFO

### 1. Justification strength
The patch's rationale is well-documented. The claims that:
- No in-tree users exist (verifiable by grep)
- The API cannot be used portably (operand type differs between implementations)
- C11 atomic_flag is opaque with no load operation

...provide strong justification for the removal.

### 2. Code organization
The release notes entry is appropriately placed in the "Removed Items" section and follows the existing format consistently.

### 3. Documentation completeness
The release notes provide:
- What was removed
- Why it was removed
- What to use instead

This is thorough and helpful for anyone affected by the change.

---

## SUMMARY

**Overall assessment**: This patch appears correct and well-justified. The code changes cleanly remove the two macros from both the C11 stdatomic and compiler builtin branches. The release notes are comprehensive.

**Primary concern**: The lack of deprecation process should be validated by maintainers, though the technical justification (API is not usable portably) may warrant the exception.

**Recommendation**: Acceptable pending maintainer confirmation on the deprecation process exemption.


More information about the test-report mailing list