Skip to content

Enable IPC API for FSDAX #878

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Nov 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 1 addition & 1 deletion src/provider/provider_devdax_memory.c
Original file line number Diff line number Diff line change
Expand Up @@ -493,7 +493,7 @@ static umf_result_t devdax_open_ipc_handle(void *provider,
LOG_DEBUG("devdax mapped (path: %s, size: %zu, protection: %i, fd: %i, "
"offset: %zu) to address %p",
devdax_ipc_data->path, length_aligned,
devdax_ipc_data->protection, fd, offset_aligned, addr);
devdax_ipc_data->protection, fd, offset_aligned, (void *)addr);

*ptr = addr;

Expand Down
71 changes: 53 additions & 18 deletions src/provider/provider_file_memory.c
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,15 @@ umf_memory_provider_ops_t *umfFileMemoryProviderOps(void) {
#include "utils_concurrency.h"
#include "utils_log.h"

#define FSDAX_PAGE_SIZE_2MB ((size_t)(2 * 1024 * 1024)) // == 2 MB

#define TLS_MSG_BUF_LEN 1024

typedef struct file_memory_provider_t {
utils_mutex_t lock; // lock for file parameters (size and offsets)

char path[PATH_MAX]; // a path to the file
bool is_fsdax; // true if file is located on FSDAX
int fd; // file descriptor for memory mapping
size_t size_fd; // size of the file used for memory mappings
size_t offset_fd; // offset in the file used for memory mappings
Expand Down Expand Up @@ -130,8 +133,6 @@ static umf_result_t file_initialize(void *params, void **provider) {
umf_file_memory_provider_params_t *in_params =
(umf_file_memory_provider_params_t *)params;

size_t page_size = utils_get_page_size();

if (in_params->path == NULL) {
LOG_ERR("file path is missing");
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
Expand All @@ -145,8 +146,6 @@ static umf_result_t file_initialize(void *params, void **provider) {

memset(file_provider, 0, sizeof(*file_provider));

file_provider->page_size = page_size;

ret = file_translate_params(in_params, file_provider);
if (ret != UMF_RESULT_SUCCESS) {
goto err_free_file_provider;
Expand All @@ -163,17 +162,34 @@ static umf_result_t file_initialize(void *params, void **provider) {
goto err_free_file_provider;
}

if (utils_set_file_size(file_provider->fd, page_size)) {
if (utils_set_file_size(file_provider->fd, FSDAX_PAGE_SIZE_2MB)) {
LOG_ERR("cannot set size of the file: %s", in_params->path);
ret = UMF_RESULT_ERROR_UNKNOWN;
goto err_close_fd;
}

file_provider->size_fd = page_size;
file_provider->size_fd = FSDAX_PAGE_SIZE_2MB;

LOG_DEBUG("size of the file %s is: %zu", in_params->path,
file_provider->size_fd);

if (!(in_params->visibility & UMF_MEM_MAP_PRIVATE)) {
// check if file is located on FSDAX
void *addr = utils_mmap_file(
NULL, file_provider->size_fd, file_provider->protection,
file_provider->visibility, file_provider->fd, 0,
&file_provider->is_fsdax);
if (addr) {
utils_munmap(addr, file_provider->size_fd);
}
}

if (file_provider->is_fsdax) {
file_provider->page_size = FSDAX_PAGE_SIZE_2MB;
} else {
file_provider->page_size = utils_get_page_size();
}

if (utils_mutex_init(&file_provider->lock) == NULL) {
LOG_ERR("lock init failed");
ret = UMF_RESULT_ERROR_UNKNOWN;
Expand Down Expand Up @@ -480,7 +496,8 @@ static umf_result_t file_get_recommended_page_size(void *provider, size_t size,
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
}

*page_size = utils_get_page_size();
file_memory_provider_t *file_provider = (file_memory_provider_t *)provider;
*page_size = file_provider->page_size;

return UMF_RESULT_SUCCESS;
}
Expand Down Expand Up @@ -669,30 +686,41 @@ static umf_result_t file_open_ipc_handle(void *provider, void *providerIpcData,
umf_result_t ret = UMF_RESULT_SUCCESS;
int fd;

size_t offset_aligned = file_ipc_data->offset_fd;
size_t size_aligned = file_ipc_data->size;

if (file_provider->is_fsdax) {
// It is just a workaround for case when
// file_alloc() was called with the size argument
// that is not a multiplier of FSDAX_PAGE_SIZE_2MB.
utils_align_ptr_down_size_up((void **)&offset_aligned, &size_aligned,
FSDAX_PAGE_SIZE_2MB);
}

fd = utils_file_open(file_ipc_data->path);
if (fd == -1) {
LOG_PERR("opening the file to be mapped (%s) failed",
file_ipc_data->path);
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
}

char *addr = utils_mmap_file(
NULL, file_ipc_data->size, file_ipc_data->protection,
file_ipc_data->visibility, fd, file_ipc_data->offset_fd, NULL);
char *addr =
utils_mmap_file(NULL, size_aligned, file_ipc_data->protection,
file_ipc_data->visibility, fd, offset_aligned, NULL);
(void)utils_close_fd(fd);
if (addr == NULL) {
file_store_last_native_error(UMF_FILE_RESULT_ERROR_ALLOC_FAILED, errno);
LOG_PERR("file mapping failed (path: %s, size: %zu, protection: %i, "
"fd: %i, offset: %zu)",
file_ipc_data->path, file_ipc_data->size,
file_ipc_data->protection, fd, file_ipc_data->offset_fd);
LOG_PERR("file mapping failed (path: %s, size: %zu, protection: %u, "
"visibility: %u, fd: %i, offset: %zu)",
file_ipc_data->path, size_aligned, file_ipc_data->protection,
file_ipc_data->visibility, fd, offset_aligned);
return UMF_RESULT_ERROR_MEMORY_PROVIDER_SPECIFIC;
}

LOG_DEBUG("file mapped (path: %s, size: %zu, protection: %i, fd: %i, "
"offset: %zu) at address %p",
file_ipc_data->path, file_ipc_data->size,
file_ipc_data->protection, fd, file_ipc_data->offset_fd, addr);
LOG_DEBUG("file mapped (path: %s, size: %zu, protection: %u, visibility: "
"%u, fd: %i, offset: %zu) at address %p",
file_ipc_data->path, size_aligned, file_ipc_data->protection,
file_ipc_data->visibility, fd, offset_aligned, (void *)addr);

*ptr = addr;

Expand All @@ -711,6 +739,13 @@ static umf_result_t file_close_ipc_handle(void *provider, void *ptr,
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
}

if (file_provider->is_fsdax) {
// It is just a workaround for case when
// file_alloc() was called with the size argument
// that is not a multiplier of FSDAX_PAGE_SIZE_2MB.
utils_align_ptr_down_size_up(&ptr, &size, FSDAX_PAGE_SIZE_2MB);
}

errno = 0;
int ret = utils_munmap(ptr, size);
// ignore error when size == 0
Expand Down
39 changes: 39 additions & 0 deletions test/provider_file_memory_ipc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,16 @@ umf_file_memory_provider_params_t get_file_params_shared(char *path) {
umf_file_memory_provider_params_t file_params_shared =
get_file_params_shared(FILE_PATH);

umf_file_memory_provider_params_t get_file_params_fsdax(char *path) {
umf_file_memory_provider_params_t file_params =
umfFileMemoryProviderParamsDefault(path);
file_params.visibility = UMF_MEM_MAP_SHARED;
return file_params;
}

umf_file_memory_provider_params_t file_params_fsdax =
get_file_params_fsdax(getenv("UMF_TESTS_FSDAX_PATH"));

HostMemoryAccessor hostAccessor;

static std::vector<ipcTestParams> ipcManyPoolsTestParamsList = {
Expand All @@ -43,7 +53,36 @@ static std::vector<ipcTestParams> ipcManyPoolsTestParamsList = {
#endif
};

static std::vector<ipcTestParams> getIpcFsDaxTestParamsList(void) {
std::vector<ipcTestParams> ipcFsDaxTestParamsList = {};

char *path = getenv("UMF_TESTS_FSDAX_PATH");
if (path == nullptr || path[0] == 0) {
// skipping the test, UMF_TESTS_FSDAX_PATH is not set
return ipcFsDaxTestParamsList;
}

ipcFsDaxTestParamsList = {
// TODO: enable it when sizes of allocations in ipcFixtures.hpp are fixed
// {umfProxyPoolOps(), nullptr, umfFileMemoryProviderOps(),
// &file_params_fsdax, &hostAccessor, true},
#ifdef UMF_POOL_JEMALLOC_ENABLED
{umfJemallocPoolOps(), nullptr, umfFileMemoryProviderOps(),
&file_params_fsdax, &hostAccessor, false},
#endif
#ifdef UMF_POOL_SCALABLE_ENABLED
{umfScalablePoolOps(), nullptr, umfFileMemoryProviderOps(),
&file_params_fsdax, &hostAccessor, false},
#endif
};

return ipcFsDaxTestParamsList;
}

GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(umfIpcTest);

INSTANTIATE_TEST_SUITE_P(FileProviderDifferentPoolsTest, umfIpcTest,
::testing::ValuesIn(ipcManyPoolsTestParamsList));

INSTANTIATE_TEST_SUITE_P(FileProviderDifferentPoolsFSDAXTest, umfIpcTest,
::testing::ValuesIn(getIpcFsDaxTestParamsList()));
16 changes: 16 additions & 0 deletions test/supp/helgrind-umf_test-ipc.supp
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{
False-positive race in critnib_insert (lack of instrumentation)
Helgrind:Race
fun:store
fun:critnib_insert
...
}

{
False-positive race in critnib_find (lack of instrumentation)
Helgrind:Race
fun:find_predecessor
fun:find_le
fun:critnib_find
...
}
17 changes: 17 additions & 0 deletions test/supp/helgrind-umf_test-provider_file_memory_ipc.supp
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,20 @@
fun:umfOpenIPCHandle
...
}

{
False-positive race in critnib_insert (lack of instrumentation)
Helgrind:Race
fun:store
fun:critnib_insert
...
}

{
False-positive race in critnib_find (lack of instrumentation)
Helgrind:Race
fun:find_predecessor
fun:find_le
fun:critnib_find
...
}
Loading