Skip to content

Conversation

@joe-explr
Copy link

@joe-explr joe-explr commented Nov 4, 2025

Add notification support for put/get operations:

Adds notification support to the OSC SM component by
implementing the put_with_notify, get_with_notify, rput_with_notify,
and rget_with_notify functions. These functions perform the same
operations as their non-notify counterparts but also increment
notification counters after the data transfer completes.

The changes include:

  • Added function pointer types for notify variants in osc.h
  • Added function prototypes in osc_sm.h
  • Implemented the notify functions in osc_sm_comm.c
  • Updated the module template to register the new functions
  • Removed TODO comments that have been addressed

@github-actions
Copy link

github-actions bot commented Nov 4, 2025

Hello! The Git Commit Checker CI bot found a few problems with this PR:

58c4c26: Notified osc SM

  • check_signed_off: does not contain a valid Signed-off-by line

Please fix these problems and, if necessary, force-push new commits back up to the PR branch. Thanks!

This commit adds notification support to the OSC SM component by
implementing the put_with_notify, get_with_notify, rput_with_notify,
and rget_with_notify functions. These functions perform the same
operations as their non-notify counterparts but also increment
notification counters after the data transfer completes.

The changes include:
- Added function pointer types for notify variants in osc.h
- Added function prototypes in osc_sm.h
- Implemented the notify functions in osc_sm_comm.c
- Updated the module template to register the new functions
- Removed TODO comments that have been addressed

Signed-off-by: Joseph Antony <jajoseph.antony18@gmail.com>
Copy link
Owner

@devreal devreal left a comment

Choose a reason for hiding this comment

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

Looks good, please fix the indentation and add the sign-off

int notify,
struct ompi_win_t *win)
{
int ret;
Copy link
Owner

Choose a reason for hiding this comment

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

Nitpick: indentation is off in this function

origin_addr, origin_count, origin_dt);
// TODO: do the same for put_with_notify
if (OMPI_SUCCESS != ret) {
return ret;
Copy link
Owner

Choose a reason for hiding this comment

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

Nitpick: indentation is off here


return OMPI_SUCCESS;;
}
}
Copy link
Owner

Choose a reason for hiding this comment

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

Please revert the change removing the newline at the end

Signed-off-by: Joseph Antony <jajoseph.antony18@gmail.com>
@joe-explr joe-explr requested a review from devreal November 4, 2025 17:05
Copy link
Owner

@devreal devreal left a comment

Choose a reason for hiding this comment

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

LGTM!

@devreal devreal merged commit 4be483e into devreal:notified-rma Nov 5, 2025
17 checks passed
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