Commit 42b5fcf1 authored by Xin Zhao's avatar Xin Zhao
Browse files

Use function hook instead of function pointer for win_free.



The original implementation of win_free is not correct. The
problem is described as follows:

It uses a function pointer which is initially set to the CH3
implementation, and can be overridden by the channel layer if
the channel provides an specific implementation.  In the CH3
win_free implementation, it first checks if all RMA
communication is finished and epoch states is reset, then
performs a global barrier, then frees the window resources
that are allocated in CH3, and finally returns. In the Nemesis
win_free implementation, it directly frees the window resources
that are allocated in Nemesis, and calls the CH3 win_free at last.
This makes no sense because we free the window resources before
checking if the RMA communication is completed.

To fix this issue, we add a function hook for channel layer
to free its own resources, the the function hook is called from
the CH3 win_free.
Signed-off-by: Pavan Balaji's avatarPavan Balaji <balaji@anl.gov>
parent 9dbcae0c
......@@ -70,20 +70,14 @@ int MPIDI_CH3_SHM_Win_shared_query(MPID_Win * win_ptr, int target_rank, MPI_Aint
int MPIDI_CH3_SHM_Win_free(MPID_Win ** win_ptr)
{
int mpi_errno = MPI_SUCCESS;
mpir_errflag_t errflag = MPIR_ERR_NONE;
MPIDI_STATE_DECL(MPID_STATE_MPIDI_CH3_SHM_WIN_FREE);
MPIDI_RMA_FUNC_ENTER(MPID_STATE_MPIDI_CH3_SHM_WIN_FREE);
if ((*win_ptr)->comm_ptr->node_comm == NULL) {
mpi_errno = MPIDI_Win_free(win_ptr);
goto fn_exit;
}
mpi_errno = MPIR_Barrier_impl((*win_ptr)->comm_ptr, &errflag);
if (mpi_errno)
MPIU_ERR_POP(mpi_errno);
/* Free shared memory region */
if ((*win_ptr)->shm_allocated) {
/* free shm_base_addrs that's only used for shared memory windows */
......@@ -153,11 +147,6 @@ int MPIDI_CH3_SHM_Win_free(MPID_Win ** win_ptr)
MPIDI_CH3I_SHM_Wins_unlink(&shm_wins_list, (*win_ptr));
}
mpi_errno = MPIDI_Win_free(win_ptr);
if (mpi_errno != MPI_SUCCESS) {
MPIU_ERR_POP(mpi_errno);
}
fn_exit:
MPIDI_RMA_FUNC_EXIT(MPID_STATE_MPIDI_CH3_SHM_WIN_FREE);
return mpi_errno;
......
......@@ -64,6 +64,7 @@ int MPIDI_CH3_Win_hooks_init(MPIDI_CH3U_Win_hooks_t * win_hooks)
if (MPIDI_CH3I_Shm_supported()) {
win_hooks->win_init = MPIDI_CH3I_Win_init;
win_hooks->win_free = MPIDI_CH3_SHM_Win_free;
}
MPIDI_RMA_FUNC_EXIT(MPID_STATE_MPIDI_CH3_WIN_HOOKS_INIT);
......@@ -294,7 +295,6 @@ static int MPIDI_CH3I_Win_detect_shm(MPID_Win ** win_ptr)
/* TODO: should we use the same mutex or create a new one ?
* It causes unnecessary synchronization.*/
(*win_ptr)->shm_mutex = shm_win_ptr->shm_mutex;
(*win_ptr)->RMAFns.Win_free = MPIDI_CH3_SHM_Win_free;
fn_exit:
MPIU_CHKLMEM_FREEALL();
......@@ -441,8 +441,6 @@ static int MPIDI_CH3I_Win_gather_info(void *base, MPI_Aint size, int disp_unit,
if (mpi_errno != MPI_SUCCESS)
MPIU_ERR_POP(mpi_errno);
(*win_ptr)->RMAFns.Win_free = MPIDI_CH3_SHM_Win_free;
fn_exit:
MPIU_CHKLMEM_FREEALL();
MPIDI_RMA_FUNC_EXIT(MPID_STATE_MPIDI_CH3I_WIN_GATHER_INFO);
......@@ -718,7 +716,6 @@ static int MPIDI_CH3I_Win_allocate_shm(MPI_Aint size, int disp_unit, MPID_Info *
/* Provide operation overrides for this window flavor */
(*win_ptr)->RMAFns.Win_shared_query = MPIDI_CH3_SHM_Win_shared_query;
(*win_ptr)->RMAFns.Win_free = MPIDI_CH3_SHM_Win_free;
/* Cache SHM windows */
MPIDI_CH3I_SHM_Wins_append(&shm_wins_list, (*win_ptr));
......
......@@ -1132,6 +1132,7 @@ extern MPIDI_CH3U_Win_fns_t MPIDI_CH3U_Win_fns;
typedef struct {
int (*win_init)(MPI_Aint, int, int, int, MPID_Info *, MPID_Comm *, MPID_Win **);
int (*win_free)(MPID_Win **);
} MPIDI_CH3U_Win_hooks_t;
extern MPIDI_CH3U_Win_hooks_t MPIDI_CH3U_Win_hooks;
......
......@@ -213,11 +213,14 @@ int MPIDI_Win_free(MPID_Win ** win_ptr)
MPIU_ERR_POP(mpi_errno);
}
if (!(*win_ptr)->shm_allocated) {
/* when SHM is allocated, we already did a global barrier in
* MPIDI_CH3_SHM_Win_free, so we do not need to do it again here. */
mpi_errno = MPIR_Barrier_impl((*win_ptr)->comm_ptr, &errflag);
if (mpi_errno)
mpi_errno = MPIR_Barrier_impl((*win_ptr)->comm_ptr, &errflag);
if (mpi_errno)
MPIU_ERR_POP(mpi_errno);
/* Free window resources in lower layer. */
if (MPIDI_CH3U_Win_hooks.win_free != NULL) {
mpi_errno = MPIDI_CH3U_Win_hooks.win_free(win_ptr);
if (mpi_errno != MPI_SUCCESS)
MPIU_ERR_POP(mpi_errno);
}
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment