Potential issue in osc/sm

[janjust]

During onesided debugging we observed strange behavior with ompi-tests/ibm/onesided/c_win_shared_noncontig_put when we're running ppn > 1.

The issue happens when ranks call Win_shared_query() on windows of size 0 and check the return base_ptr.

 MPI_Info_create(&alloc_shared_info);
 MPI_Info_set(alloc_shared_info, "alloc_shared_noncontig", "true");

 MPI_Win_allocate_shared(my_size, sizeof(int), alloc_shared_info,
                             shm_comm, &my_base, &shm_win);

 MPI_Win_shared_query(shm_win, i, &size, &disp_unit, &base);

assert(base == NULL);

Interestingly, it seems that in the osc/sm code, the following calls may impact module->noncontig variable which is used in a conditional statement that sets base_ptr

ompi/ompi/mca/osc/sm/osc_sm_component.c

Line 222 in 9d94a14

ret = opal_infosubscribe_subscribe(&(win->super), "alloc_shared_noncontig", "false", component_set_alloc_shared_noncontig_info);

ompi/ompi/mca/osc/sm/osc_sm_component.c

Lines 276 to 280 in 9d94a14

	module->noncontig = false;
	if (OMPI_SUCCESS != opal_info_get_bool(info, "alloc_shared_noncontig",
	&module->noncontig, &flag)) {
	goto error;
	}

After 277 the noncontig will be false, which will evaluate to true, and the base_ptr is set.

ompi/ompi/mca/osc/sm/osc_sm_component.c

Lines 369 to 374 in 9d94a14

	if (module->sizes[i] || !module->noncontig) {
	module->bases[i] = ((char *) module->segment_base) + total;
	total += rbuf[i];
	} else {
	module->bases[i] = NULL;
	}

However, if we call opal_info_get_bool() before opal_infosubscribe_subscribe(), the noncontig flag will remain true, and the base_ptr will not be set. To be clear, we're not removing the original opal_info_get_bool() on line 277, only adding an extra call prior to the subscribe call, and the flag will remain unchanged.

[bwbarrett]

This looks more like a question about the info subsystem than osc, and I know nothing about the info subsystem, so I'm not the right person to own this ticket. @hjelmn and @jsquyres may be able to help.

It looks like (ignoring the info questions), the SM OSC component is working properly. Unless the SM component detects that the info key "alloc_shared_noncontig" is "true", the component will allocate one shared memory segment of size SUM(rank_sizes), and the baseptr for every process will be non-NULL, because the algorithm for assigning base pointers is essentially:

buffer = malloc(size);
offset = 0;
for (i = 0 ; i < window_size ; ++i) {
  base_ptr[i] = buffer + offset;
  offset += win_sizes[i];
}

However, when the non-contig case is true, the SM component is rightly not setting all baseptrs to non-null, because every rank is locally allocating memory (and calling malloc(0) generally causes unhappiness).

So this really comes down to a usage behavior in info retrieval.

[devreal]

The registered callback component_set_alloc_shared_noncontig_info does not actually change the noncontig value in the module. Instead, it returns "false" (because module->noncontig is false by default), which is then set as the value in the info object. Any further querying of the info key will return false. If, however, you move the the call to opal_info_get_bool up (and thus set module->noncontig to the value of the info key) the component_set_alloc_shared_noncontig_info callback will return true and the result is what you would expect. This is a bug in coll/sm, because component_set_alloc_shared_noncontig_info forces the value to be false.

The correct way in this case is to check the info key first and then subscribe to it, e.g., like in osc/ucx: https://github.com/open-mpi/ompi/blob/main/ompi/mca/osc/ucx/osc_ucx_component.c#L404 followed by the info subscription in https://github.com/open-mpi/ompi/blob/main/ompi/mca/osc/ucx/osc_ucx_component.c#L542

[bwbarrett]

@devreal is there actually any reason to subscribe to an info key like "alloc_shared_noncontig"? The behavior in question can't change once WIN_ALLOCATE_SHARED returns, since the memory is allocated at that point. Can't we simplify by just checking the value, doing our allocations, and moving on? Is there anything we should be doing to reject an attempt to set an info key after the window is created in this case?

[devreal]

Good question. Looking at the infosubscriber code, it seems like any attempt to update the key is ignored if there are no subscribers. Dropping the subscription in osc/sm should be safe then, the key won't be changed.

[awlauria]

Pr's merged - I believe this can be closed?