Bug report : osc_ucx_component.c openmpi 5.0.x : dynamic windows attach/detach

[ymeur]

Background information

What version of Open MPI are you using? (e.g., v4.1.6, v5.0.1, git branch name and hash, etc.)

v5.0.5 head, (0b7c77c)

Details of the problem

In osc_ucx_component.c, ompi_osc_ucx_win_attach function : when adding new memory zone attachment to window, the reference counter is not correctly initialized to zero, leading in some case, to take the value of reference counter of an other memory zone previously attached. The code lines doing it (from lines 1042, ompi/mca/osc/ucx/osc_ucx_component.c ) :

    if (module->state.dynamic_win_count > 0) {
        contain_index = ompi_osc_find_attached_region_position((ompi_osc_dynamic_win_info_t *)module->state.dynamic_wins,
                                                               0, (int)module->state.dynamic_win_count - 1,
                                                               (uint64_t)base, len, &insert_index);
        if (contain_index >= 0) {
            module->local_dynamic_win_info[contain_index].refcnt++;
            ret = ompi_osc_ucx_dynamic_unlock(module, ompi_comm_rank(module->comm));
            return ret;
        }

        assert(insert_index >= 0 && (uint64_t)insert_index <= module->state.dynamic_win_count);

        memmove((void *)&module->local_dynamic_win_info[insert_index+1],
                (void *)&module->local_dynamic_win_info[insert_index],
                (OMPI_OSC_UCX_ATTACH_MAX - (insert_index + 1)) * sizeof(ompi_osc_local_dynamic_win_info_t));
        memmove((void *)&module->state.dynamic_wins[insert_index+1],
                (void *)&module->state.dynamic_wins[insert_index],
                (OMPI_OSC_UCX_ATTACH_MAX - (insert_index + 1)) * sizeof(ompi_osc_dynamic_win_info_t));
        module->local_dynamic_win_info[insert_index].refcnt = 0 ;

    } else {
        insert_index = 0;
    }

Concretely, when inserting a new memory zone in "module->local_dynamic_win_info" array at insert_index, a copy is made from (insert_index : OMPI_OSC_UCX_ATTACH_MAX -2) to (insert_index+1 : OMPI_OSC_UCX_ATTACH_MAX -1), which is correct, but the new entry created at insert_index is not correctly reinitialized for "refcnt" data member of local_dynamic_win_info structure.

A possible fix (and that seems to work well) would be to set refcnt to 0 after moving memory :

  if (module->state.dynamic_win_count > 0) {
        contain_index = ompi_osc_find_attached_region_position((ompi_osc_dynamic_win_info_t *)module->state.dynamic_wins,
                                                               0, (int)module->state.dynamic_win_count - 1,
                                                               (uint64_t)base, len, &insert_index);
        if (contain_index >= 0) {
            module->local_dynamic_win_info[contain_index].refcnt++;
            ret = ompi_osc_ucx_dynamic_unlock(module, ompi_comm_rank(module->comm));
            return ret;
        }

        assert(insert_index >= 0 && (uint64_t)insert_index <= module->state.dynamic_win_count);

        memmove((void *)&module->local_dynamic_win_info[insert_index+1],
                (void *)&module->local_dynamic_win_info[insert_index],
                (OMPI_OSC_UCX_ATTACH_MAX - (insert_index + 1)) * sizeof(ompi_osc_local_dynamic_win_info_t));
        memmove((void *)&module->state.dynamic_wins[insert_index+1],
                (void *)&module->state.dynamic_wins[insert_index],
                (OMPI_OSC_UCX_ATTACH_MAX - (insert_index + 1)) * sizeof(ompi_osc_dynamic_win_info_t));
        module->local_dynamic_win_info[insert_index].refcnt = 0 ;  /* <------ HERE */
    } else {
        insert_index = 0;
    } 

This bug may lead to not correctly detach the memory zone, since the refcnt is not reaching 0 as it might, which could make issue when attempting to attach new memory zone to window.

[ymeur]

Hello,
Some reaction about this report ? How can I help ? Should I make a patch or a pull request ?
Since I am not very familiar with openmpi development processus, could you indicate me the good way to make an efficient reporting or fix.
Regards,
YM

[devreal]

@janjust Mind taking a look at this?

[ymeur]

Many thanks !

[janjust]

@MamziB ^