Disabling put_scalar when disable-ofi-inject is used

[wrrobin]

No description provided.

[davidozog]

Looks like a good patch. IIUC, the only substantial change here is the #ifndef DISABLE_OFI_INJECT change in put_scalar. All other diffs are from reordering existing routines?

[davidozog]

@wrrobin - We can ignore the UCX over pmi-mpi failure, I'm removing it in #1106. Do you think we should add at least one test with --disable-ofi-inject here?

[wrrobin]

Thanks @davidozog. You are right about the diffs. I will add a test in CI. We can merge this after #1106 is merged.

[davidozog]

This looks good to merge when ready @wrrobin

[wrrobin]

@davidozog - Yes, this is good to go. One thing I should mention, we are not disabling OFI atomic inject with the same config flag. We might need to do this, but I am thinking to create a separate PR for that with a new flag option -disable-ofi-atomic-inject. Thoughts?

[davidozog]

@davidozog - Yes, this is good to go. One thing I should mention, we are not disabling OFI atomic inject with the same config flag. We might need to do this, but I am thinking to create a separate PR for that with a new flag option -disable-ofi-atomic-inject. Thoughts?

That makes sense to me, thanks for the note.

It's slightly unfortunately the DISABLE_OFI_INJECT macro is outside/above the transport_ofi layer, but I do think this is the simplest way to do it. Do you think it's possible to keep it within the transport layer? Something to think about when adding "disable-ofi-atomic-inject".

It's not the first time we've done something similar to this though - see here:
https://github.com/Sandia-OpenSHMEM/SOS/blob/v1.5.2/src/init.c#L206

[wrrobin]

Yeah.. I thought about it too. And, initially, I made changes only on the transport layer for a particular function, but it requires more changes.
If it is ok, lets move forward with this. I am keeping a issue created for this and will handle separately.