Skip to content

Commit

Permalink
UCP/MMAP: apply review comments
Browse files Browse the repository at this point in the history
 - Fix description and simplify check erroneous cases of
   ucp_mem_map_check_and_adjust_params function
 - Fix description of UCP_MEM_MAP_ALLOCATE flag
 - Refactoring of test_ucp_memheap allocation part
  • Loading branch information
evgeny-leksikov committed Feb 16, 2017
1 parent bbd4fb0 commit ef88b59
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 48 deletions.
2 changes: 1 addition & 1 deletion src/ucp/api/ucp.h
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ enum {
UCP_MEM_MAP_ALLOCATE = UCS_BIT(2) /**< Identify requirement for allocation,
if passed address is not a null-pointer
then it will be used as a hint or direct
address for allocation depend */
address for allocation. */
};


Expand Down
33 changes: 12 additions & 21 deletions src/ucp/core/ucp_mm.c
Original file line number Diff line number Diff line change
Expand Up @@ -196,15 +196,16 @@ static inline unsigned uct_flags(ucp_mem_map_params_t *params)
}

/* Matrix of behavior
* |-------------------------------------------------------------------------------------------|
* | parameter | value |
* |-----------|-------------------------------------------------------------------------------|
* | ALLOCATE | 0 | 1 | 0 | 0 | 1 | 1 | 0 | 1 |
* | FIXED | 0 | 0 | 1 | 0 | 1 | 0 | 1 | 1 |
* | addr | 0 | 0 | 0 | 1 | 0 | 1 | 1 | 1 |
* |-----------|-------|----------|-------|-----|-------|---------------|-----|----------------|
* | result | error | allocate | error | reg | error | allocate/hint | reg | allocate/fixed |
* |-------------------------------------------------------------------------------------------|
* |--------------------------------------------------------------------------|
* | parameter | value |
* |-----------|--------------------------------------------------------------|
* | ALLOCATE | 0 | 1 | 0 | 0 | 1 | 1 | 0 | 1 |
* | FIXED | 0 | 0 | 1 | 0 | 1 | 0 | 1 | 1 |
* | addr | 0 | 0 | 0 | 1 | 0 | 1 | 1 | 1 |
* |-----------|-----|----------|-----|-----|-----|----------|-----|----------|
* | result | err | allocate | err | reg | err | allocate | err | allocate |
* | | | | | | | (hint) | | (fixed) |
* |--------------------------------------------------------------------------|
*/
static inline ucs_status_t ucp_mem_map_check_and_adjust_params(ucp_mem_map_params_t *params)
{
Expand Down Expand Up @@ -235,19 +236,9 @@ static inline ucs_status_t ucp_mem_map_check_and_adjust_params(ucp_mem_map_param
return UCS_ERR_INVALID_PARAM;
}

/* Now, lets check all erroneous cases from the matrix */
/* Now, lets check the rest of erroneous cases from the matrix */
if (params->address == NULL) {
if (params->flags == 0) {
return UCS_ERR_INVALID_PARAM;
}

if (!(params->flags & UCP_MEM_MAP_ALLOCATE) &&
(params->flags & UCP_MEM_MAP_FIXED)) {
return UCS_ERR_INVALID_PARAM;
}

if ((params->flags & UCP_MEM_MAP_ALLOCATE) &&
(params->flags & UCP_MEM_MAP_FIXED)) {
if (!(params->flags & UCP_MEM_MAP_ALLOCATE)) {
return UCS_ERR_INVALID_PARAM;
}
} else if (!(params->flags & UCP_MEM_MAP_ALLOCATE) &&
Expand Down
41 changes: 15 additions & 26 deletions test/gtest/ucp/test_ucp_memheap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,29 +50,23 @@ void test_ucp_memheap::test_nonblocking_implicit_stream_xfer(nonblocking_send_fu
receiver().connect(&sender());
}

ucp_mem_h memh;
void *memheap;

if (malloc_allocate) {
memheap = malloc(memheap_size);
} else {
memheap = NULL;
}

params.field_mask = UCP_MEM_MAP_PARAM_FIELD_ADDRESS |
UCP_MEM_MAP_PARAM_FIELD_LENGTH |
UCP_MEM_MAP_PARAM_FIELD_FLAGS;
params.address = memheap;
params.length = memheap_size;
params.flags = GetParam().variant;
if (!malloc_allocate) {
params.flags = UCP_MEM_MAP_ALLOCATE;
if (malloc_allocate) {
params.address = malloc(memheap_size);
} else {
params.address = NULL;
params.flags |= UCP_MEM_MAP_ALLOCATE;
}

ucp_mem_h memh;
status = ucp_mem_map(receiver().ucph(), &params, &memh);
ASSERT_UCS_OK(status);

memheap = params.address;
void *memheap = params.address;
memset(memheap, 0, memheap_size);

void *rkey_buffer;
Expand Down Expand Up @@ -154,29 +148,24 @@ void test_ucp_memheap::test_blocking_xfer(blocking_send_func_t send,
}

ucp_mem_h memh;
void *memheap;

if (malloc_allocate) {
memheap = malloc(memheap_size);
} else {
memheap = NULL;
}
void *memheap = NULL;

params.field_mask = UCP_MEM_MAP_PARAM_FIELD_ADDRESS |
UCP_MEM_MAP_PARAM_FIELD_LENGTH |
UCP_MEM_MAP_PARAM_FIELD_FLAGS;
params.address = memheap;
params.length = memheap_size;
params.flags = GetParam().variant;
if (malloc_allocate) {
params.address = malloc(memheap_size);
} else {
params.address = NULL;
params.flags |= UCP_MEM_MAP_ALLOCATE;
}

if (params.flags & UCP_MEM_MAP_FIXED) {
goto out;
}

if (!malloc_allocate) {
params.flags = UCP_MEM_MAP_ALLOCATE;
}

status = ucp_mem_map(receiver().ucph(), &params, &memh);
ASSERT_UCS_OK(status);

Expand Down Expand Up @@ -241,7 +230,7 @@ void test_ucp_memheap::test_blocking_xfer(blocking_send_func_t send,
disconnect(sender());
disconnect(receiver());

if (malloc_allocate) {
if (malloc_allocate && memheap) {
free(memheap);
}
}

0 comments on commit ef88b59

Please sign in to comment.