ch4/ofi: fix distribution of NICs in NUMA systems

[mthyoung-amzn]

On systems with non-uniform attachment of NICs to CPU cores, we are considering closeness in NIC assignment, but assigning them by round-robin based on local rank. In a two-NUMA node system (using -map-by numa) this results in only half of the NICs being used (the even-indexed ones on NUMA node 0 and odd-indexed on NUMA node 1).

This fixes this by explicitly sharing the close NICs across all local ranks, and selecting NICs based on index within each "sharing set" of local ranks.

Pull Request Description

Author Checklist

• Provide Description
  Particularly focus on why, not what. Reference background, issues, test failures, xfail entries, etc.
• Commits Follow Good Practice
  Commits are self-contained and do not do two things at once.
  Commit message is of the form: module: short description
  Commit message explains what's in the commit.
• Passes All Tests
  Whitespace checker. Warnings test. Additional tests via comments.
• Contribution Agreement
  For non-Argonne authors, check contribution agreement.
  If necessary, request an explicit comment from your companies PR approval manager.

[hzhou]

Looks good in general! Curious on how it works in your testing with 32 NICs.

[hzhou]

With MPI sessions, users can create a comm_world after a sub-comm already created. So we will have a situation that fabric_initialized are not uniform. Since we can't reorder nics after some of the process already initialized, every processes (in the node_comm) needs to skip multi-nic ordering.

Thus:

if (fabric_initialized) {
    all_need_init = false;
}

My original design was to "allreduce" !fabric_initialized -> all_need_init.

PS: on second thought, I think it is perfectly okay for any not-yet-initialized process to locally re-order nics to select its global closest rank. It may not be perfect since it may collide with those process that already initialized, but that won't be any worse than without re-ordering. Thus, it may be okay to skip all_need_init check altogether.

[mthyoung-amzn]

I can make this more robust

[mthyoung-amzn]

I agree that this is correct/safe as-is, though far from optimal. I have rolled back the all_need_init, since in the current implementation it's just kind of confusing. Added a comment to the effect that the Session model needs some work.

I think the right way to do it would be to maintain a shared-memory claim table for all local NICs, so that subsequent communicator creation can see which NICs are already in use by other communicators. That's probably too much for this PR, though.

[mthyoung-amzn]

Curious on how it works in your testing with 32 NICs.

This works well! One thing that I want to do (this was in my earlier PR) is to distribute NIC assignments based on more detailed bus topology; we can't actually push full line rate out of all NICs at the same time, but can do better if we distribute NICs in use across host bridges. Getting this right is tricky, so figured it should go in its own PR.

Also, getting GPU/NIC affinity right is quite a bit more important on these systems, so I will be focusing on that next.

[hzhou]

An alternative solution is to rely on asynchronous shared memory table using inter-process mutex. The shared memory slab is created here:

mpich/src/mpid/ch4/shm/posix/posix_init.c

Lines 287 to 300 in c2e6701

	slab = MPL_initshm_open(MPIDI_POSIX_global.shm_name, slab_size, &is_root);
	MPIR_ERR_CHKANDJUMP(!slab, mpi_errno, MPI_ERR_OTHER, "**nomem");
	if (is_root) {
	memset(slab, 0, sizeof(MPIDI_POSIX_shm_t));
	MPL_atomic_relaxed_store_uint64(&slab->shm_limit_counter, 0);
	MPL_atomic_store_int(&slab->num_shared, 1);
	MPL_atomic_store_int(&slab->num_shared_vci, 0);
	MPL_atomic_store_int(&slab->shm_ready, MPIDI_POSIX_READY_FLAG);
	} else {
	while (MPL_atomic_load_int(&slab->shm_ready) != MPIDI_POSIX_READY_FLAG) {
	MPID_Thread_yield();
	}
	MPL_atomic_fetch_add_int(&slab->num_shared, 1);

We can add a mutex and nic usage table to the slab, then each process can select its nic based on previous nic claims by other processes.

My original suggestion is to let every process proceed to MPIDI_OFI_order_multi_nic_global and perform the allgather. The allgather can collect close-nic-bitmaps plus the fabric_initialized flag, then each process can make its nic choice locally.

If we want the algorithm to work for sessions, we can expand the "bitmap" into double purpose:

1. designate a single bit as special flag
   • if true: the process already selected a preferred nic, and the bitmap marks the selected nic
   • if false: the process have not selected a preferred nic yet, the bitmap marks all the close nics
2. a selection algorithm that avoids already selected nic and the nics that would be selected by the ranks that has the precedence.

[mthyoung-amzn]

I think we are on the same page regarding the shared memory approach; I would like to do something like that as future work.

As for the current PR, I see what you mean; I'll make the fabric_initialized change.

[mthyoung-amzn]

Okay I removed the added allreduce of fabric_initialized, added a dryrun argument for MPIDI_OFI_order_multi_nic_global and friends. If a process has already initialized the fabric, it will participate in the close-NIC allgather, but skip any reordering of it's NICs

[hzhou]

How about keep the same name as "fabric_initialized"? It takes an extra AI token to understand the logic of "dryrun" :)

[mthyoung-amzn]

Ha! What a fascinating modern age we live in. Sure thing

[mthyoung-amzn]

moved the fabric_initialized from file-static to the OFI globals, since that's a more uniform place for it in the first place. removed argument entirely.

[hzhou]

Looks good! Will merge after some additional CI testing.

[hzhou]

test:mpich/ch4/ofi
test:mpich/ch4/ofi/more

Some failures that I don't think are related to this PR. Potentially due to now we force FI_PROVIDER=sockets (to save the init time). Will monitor.