Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
oshmem: sshmem: code cleaunp
The commit removes unused code and interface function, moves
common code to the base.

Signed-off-by: Alex Mikheev <[email protected]>
  • Loading branch information
alex-mikheev committed Feb 22, 2017
commit e038e3f9e0aa95083297d3d479abb7f29cedeee2
21 changes: 3 additions & 18 deletions oshmem/mca/sshmem/base/base.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,6 @@ mca_sshmem_segment_create(map_segment_t *ds_buf,
const char *file_name,
size_t size);

OSHMEM_DECLSPEC int
mca_sshmem_ds_copy(const map_segment_t *from,
map_segment_t *to);

OSHMEM_DECLSPEC void *
mca_sshmem_segment_attach(map_segment_t *ds_buf, sshmem_mkey_t *mkey);

Expand Down Expand Up @@ -148,23 +144,12 @@ OSHMEM_DECLSPEC extern mca_base_framework_t oshmem_sshmem_base_framework;
"Warning %s:%d - %s()", __SSHMEM_FILE__, __LINE__, __func__, __VA_ARGS__)


OSHMEM_DECLSPEC extern void shmem_ds_reset(map_segment_t *ds_buf);

/*
* Get unique file name
*/
static inline char * oshmem_get_unique_file_name(uint64_t pe)
{
char *file_name = NULL;

assert(mca_sshmem_base_backing_file_dir);

if (NULL == (file_name = calloc(OPAL_PATH_MAX, sizeof(char)))) {
return NULL;
}

snprintf(file_name, OPAL_PATH_MAX, "%s/shmem_job_%u_pe_%llu", mca_sshmem_base_backing_file_dir, ORTE_PROC_MY_NAME->jobid, (unsigned long long)pe);

return file_name;
}
OSHMEM_DECLSPEC extern char * oshmem_get_unique_file_name(uint64_t pe);

END_C_DECLS

Expand Down
44 changes: 28 additions & 16 deletions oshmem/mca/sshmem/base/sshmem_base_wrappers.c
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
#include "oshmem/mca/sshmem/sshmem.h"
#include "oshmem/mca/sshmem/base/base.h"

/* ////////////////////////////////////////////////////////////////////////// */
int
mca_sshmem_segment_create(map_segment_t *ds_buf,
const char *file_name,
Expand All @@ -28,19 +27,6 @@ mca_sshmem_segment_create(map_segment_t *ds_buf,
return mca_sshmem_base_module->segment_create(ds_buf, file_name, size);
}

/* ////////////////////////////////////////////////////////////////////////// */
int
mca_sshmem_ds_copy(const map_segment_t *from,
map_segment_t *to)
{
if (!mca_sshmem_base_selected) {
return OSHMEM_ERROR;
}

return mca_sshmem_base_module->ds_copy(from, to);
}

/* ////////////////////////////////////////////////////////////////////////// */
void *
mca_sshmem_segment_attach(map_segment_t *ds_buf, sshmem_mkey_t *mkey)
{
Expand All @@ -51,7 +37,6 @@ mca_sshmem_segment_attach(map_segment_t *ds_buf, sshmem_mkey_t *mkey)
return mca_sshmem_base_module->segment_attach(ds_buf, mkey);
}

/* ////////////////////////////////////////////////////////////////////////// */
int
mca_sshmem_segment_detach(map_segment_t *ds_buf, sshmem_mkey_t *mkey)
{
Expand All @@ -62,7 +47,6 @@ mca_sshmem_segment_detach(map_segment_t *ds_buf, sshmem_mkey_t *mkey)
return mca_sshmem_base_module->segment_detach(ds_buf, mkey);
}

/* ////////////////////////////////////////////////////////////////////////// */
int
mca_sshmem_unlink(map_segment_t *ds_buf)
{
Expand All @@ -73,3 +57,31 @@ mca_sshmem_unlink(map_segment_t *ds_buf)
return mca_sshmem_base_module->unlink(ds_buf);
}


char * oshmem_get_unique_file_name(uint64_t pe)
{
char *file_name = NULL;

assert(mca_sshmem_base_backing_file_dir);

if (NULL == (file_name = calloc(OPAL_PATH_MAX, sizeof(char)))) {
return NULL;
}

snprintf(file_name, OPAL_PATH_MAX, "%s/shmem_job_%u_pe_%llu", mca_sshmem_base_backing_file_dir, ORTE_PROC_MY_NAME->jobid, (unsigned long long)pe);

return file_name;
}


void
shmem_ds_reset(map_segment_t *ds_buf)
{
MAP_SEGMENT_RESET_FLAGS(ds_buf);
ds_buf->seg_id = MAP_SEGMENT_SHM_INVALID;
ds_buf->super.va_base = 0;
ds_buf->super.va_end = 0;
ds_buf->seg_size = 0;
ds_buf->type = MAP_SEGMENT_UNKNOWN;
}

75 changes: 8 additions & 67 deletions oshmem/mca/sshmem/mmap/sshmem_mmap_module.c
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,6 @@ segment_create(map_segment_t *ds_buf,
const char *file_name,
size_t size);

static int
ds_copy(const map_segment_t *from,
map_segment_t *to);

static void *
segment_attach(map_segment_t *ds_buf, sshmem_mkey_t *mkey);

Expand All @@ -88,44 +84,13 @@ mca_sshmem_mmap_module_t mca_sshmem_mmap_module = {
{
module_init,
segment_create,
ds_copy,
segment_attach,
segment_detach,
segment_unlink,
module_finalize
}
};

/* ////////////////////////////////////////////////////////////////////////// */
/* private utility functions */
/* ////////////////////////////////////////////////////////////////////////// */

/* ////////////////////////////////////////////////////////////////////////// */
/**
* completely resets the contents of *ds_buf
*/
static inline void
shmem_ds_reset(map_segment_t *ds_buf)
{
OPAL_OUTPUT_VERBOSE(
(70, oshmem_sshmem_base_framework.framework_output,
"%s: %s: shmem_ds_resetting "
"(id: %d, size: %lu, name: %s)\n",
mca_sshmem_mmap_component.super.base_version.mca_type_name,
mca_sshmem_mmap_component.super.base_version.mca_component_name,
ds_buf->seg_id, (unsigned long)ds_buf->seg_size, ds_buf->seg_name)
);

MAP_SEGMENT_RESET_FLAGS(ds_buf);
ds_buf->seg_id = MAP_SEGMENT_SHM_INVALID;
ds_buf->super.va_base = 0;
ds_buf->super.va_end = 0;
ds_buf->seg_size = 0;
ds_buf->type = MAP_SEGMENT_UNKNOWN;
unlink(ds_buf->seg_name);
memset(ds_buf->seg_name, '\0', sizeof(ds_buf->seg_name));
}

/* ////////////////////////////////////////////////////////////////////////// */
static int
module_init(void)
Expand All @@ -142,30 +107,6 @@ module_finalize(void)
return OSHMEM_SUCCESS;
}

/* ////////////////////////////////////////////////////////////////////////// */
static int
ds_copy(const map_segment_t *from,
map_segment_t *to)
{
memcpy(to, from, sizeof(map_segment_t));

OPAL_OUTPUT_VERBOSE(
(70, oshmem_sshmem_base_framework.framework_output,
"%s: %s: ds_copy complete "
"from: (id: %d, size: %lu, "
"name: %s flags: 0x%02x) "
"to: (id: %d, size: %lu, "
"name: %s flags: 0x%02x)\n",
mca_sshmem_mmap_component.super.base_version.mca_type_name,
mca_sshmem_mmap_component.super.base_version.mca_component_name,
from->seg_id, (unsigned long)from->seg_size, from->seg_name,
from->flags, to->seg_id, (unsigned long)to->seg_size, to->seg_name,
to->flags)
);

return OSHMEM_SUCCESS;
}
/* ////////////////////////////////////////////////////////////////////////// */

static int
segment_create(map_segment_t *ds_buf,
Expand Down Expand Up @@ -225,11 +166,11 @@ segment_create(map_segment_t *ds_buf,
OPAL_OUTPUT_VERBOSE(
(70, oshmem_sshmem_base_framework.framework_output,
"%s: %s: create %s "
"(id: %d, addr: %p size: %lu, name: %s)\n",
"(id: %d, addr: %p size: %lu)\n",
mca_sshmem_mmap_component.super.base_version.mca_type_name,
mca_sshmem_mmap_component.super.base_version.mca_component_name,
(rc ? "failure" : "successful"),
ds_buf->seg_id, ds_buf->super.va_base, (unsigned long)ds_buf->seg_size, ds_buf->seg_name)
ds_buf->seg_id, ds_buf->super.va_base, (unsigned long)ds_buf->seg_size)
);

return rc;
Expand Down Expand Up @@ -316,10 +257,10 @@ segment_attach(map_segment_t *ds_buf, sshmem_mkey_t *mkey)
OPAL_OUTPUT_VERBOSE(
(70, oshmem_sshmem_base_framework.framework_output,
"%s: %s: attach successful "
"(id: %d, addr: %p size: %lu, name: %s | va_base: 0x%p len: %d key %llx)\n",
"(id: %d, addr: %p size: %lu | va_base: 0x%p len: %d key %llx)\n",
mca_sshmem_mmap_component.super.base_version.mca_type_name,
mca_sshmem_mmap_component.super.base_version.mca_component_name,
ds_buf->seg_id, ds_buf->super.va_base, (unsigned long)ds_buf->seg_size, ds_buf->seg_name,
ds_buf->seg_id, ds_buf->super.va_base, (unsigned long)ds_buf->seg_size,
mkey->va_base, mkey->len, (unsigned long long)mkey->u.key)
);

Expand All @@ -338,10 +279,10 @@ segment_detach(map_segment_t *ds_buf, sshmem_mkey_t *mkey)
OPAL_OUTPUT_VERBOSE(
(70, oshmem_sshmem_base_framework.framework_output,
"%s: %s: detaching "
"(id: %d, addr: %p size: %lu, name: %s)\n",
"(id: %d, addr: %p size: %lu)\n",
mca_sshmem_mmap_component.super.base_version.mca_type_name,
mca_sshmem_mmap_component.super.base_version.mca_component_name,
ds_buf->seg_id, ds_buf->super.va_base, (unsigned long)ds_buf->seg_size, ds_buf->seg_name)
ds_buf->seg_id, ds_buf->super.va_base, (unsigned long)ds_buf->seg_size)
);

munmap((void *)ds_buf->super.va_base, ds_buf->seg_size);
Expand All @@ -363,10 +304,10 @@ segment_unlink(map_segment_t *ds_buf)
OPAL_OUTPUT_VERBOSE(
(70, oshmem_sshmem_base_framework.framework_output,
"%s: %s: unlinking "
"(id: %d, addr: %p size: %lu, name: %s)\n",
"(id: %d, addr: %p size: %lu)\n",
mca_sshmem_mmap_component.super.base_version.mca_type_name,
mca_sshmem_mmap_component.super.base_version.mca_component_name,
ds_buf->seg_id, ds_buf->super.va_base, (unsigned long)ds_buf->seg_size, ds_buf->seg_name)
ds_buf->seg_id, ds_buf->super.va_base, (unsigned long)ds_buf->seg_size)
);

/* don't completely reset. in particular, only reset
Expand Down
16 changes: 0 additions & 16 deletions oshmem/mca/sshmem/sshmem.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
*
* - module_init
* - segment_create
* - ds_copy
* - segment_attach
* - segment_detach
* - unlink
Expand Down Expand Up @@ -74,20 +73,6 @@ typedef struct mca_sshmem_base_component_2_0_0_t mca_sshmem_base_component_t;
typedef int
(*mca_sshmem_base_module_init_fn_t)(void);

/**
* copy shmem data structure information pointed to by from to the structure
* pointed to by to.
*
* @param from source pointer (IN).
*
* @param to destination pointer (OUT).
*
* @return OSHMEM_SUCCESS on success.
*/
typedef int
(*mca_sshmem_base_ds_copy_fn_t)(const map_segment_t *from,
map_segment_t *to);

/**
* create a new shared memory segment and initialize members in structure
* pointed to by ds_buf.
Expand Down Expand Up @@ -153,7 +138,6 @@ typedef int (*mca_sshmem_base_module_finalize_fn_t)(void);
struct mca_sshmem_base_module_2_0_0_t {
mca_sshmem_base_module_init_fn_t module_init;
mca_sshmem_base_module_segment_create_fn_t segment_create;
mca_sshmem_base_ds_copy_fn_t ds_copy;
mca_sshmem_base_module_segment_attach_fn_t segment_attach;
mca_sshmem_base_module_segment_detach_fn_t segment_detach;
mca_sshmem_base_module_unlink_fn_t unlink;
Expand Down
4 changes: 3 additions & 1 deletion oshmem/mca/sshmem/sshmem_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ typedef enum {
MAP_SEGMENT_ALLOC_SHM,
MAP_SEGMENT_ALLOC_IBV,
MAP_SEGMENT_ALLOC_IBV_NOSHMR,
MAP_SEGMENT_ALLOC_UCX,
MAP_SEGMENT_UNKNOWN
} segment_type_t;

Expand Down Expand Up @@ -112,9 +113,10 @@ typedef struct map_segment {
sshmem_mkey_t *mkeys; /* includes local segment bases in va_base */
segment_flag_t flags; /* enable/disable flag */
int seg_id;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alex-mikheev do you believe that seg_name is needless? I see that original code has unlink(ds_buf->seg_name) in shmem_ds_reset(). I do not remember if it is something useful but pay attention to avoid related issues.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see unlink() and memset two zero. However I do not see where seg_name is set to something and I do not see where it is used. (except in debug prints)

char seg_name[OPAL_PATH_MAX];
size_t seg_size; /* length of the segment */
segment_type_t type; /* type of the segment */
void *context; /* allocator can use this field to store
its own private data */
} map_segment_t;

END_C_DECLS
Expand Down
Loading