Skip to content

Conversation

@sielicki
Copy link
Contributor

@sielicki sielicki commented Jan 2, 2026

just use an atomic counter, the only thing this lock protects is a single uint32.

just use an atomic counter, the only thing this lock protects is a
single uint32.

Signed-off-by: Nicholas Sielicki <[email protected]>
Copilot AI review requested due to automatic review settings January 2, 2026 15:15
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR optimizes the progress counter implementation by replacing a lock-protected integer field with atomic operations. The change eliminates unnecessary lock overhead for a single uint32_t counter that tracks the number of operations needing progress.

Key changes:

  • Replaced progress_count_lock spinlock with lock-free atomic operations
  • Converted progress_count from int to ofi_atomic32_t type
  • Updated all access functions to use atomic increment, decrement, and get operations

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
prov/cxi/include/cxip.h Replaced lock-protected counter field with atomic type and updated inline helper functions to use atomic operations
prov/cxi/src/cxip_cntr.c Removed lock initialization/destruction, added atomic counter initialization, and fixed spelling typo in comment

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

@j-xiong j-xiong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I'll let provider owner(s) to double check.

@j-xiong j-xiong requested review from jswaro and swelch January 5, 2026 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants