Skip to content

Commit 199e754

Browse files
ldorauldorau
committed
Remove the UMF_MEM_MAP_SYNC flag
Remove the UMF_MEM_MAP_SYNC flag, UMF_MEM_MAP_SHARED should be used instead. Verify if /dev/dax was mapped with MAP_SYNC. Signed-off-by: Lukasz Dorau <[email protected]>
1 parent cace4ec commit 199e754

12 files changed

+56
-81
lines changed

‎README.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -207,10 +207,10 @@ so it should be used with a pool manager that will take over
207207
the managing of the provided memory - for example the jemalloc pool
208208
with the `disable_provider_free` parameter set to true.
209209

210-
IPC API requires the `UMF_MEM_MAP_SHARED` or `UMF_MEM_MAP_SYNC` memory `visibility` mode
210+
IPC API requires the `UMF_MEM_MAP_SHARED` memory `visibility` mode
211211
(`UMF_RESULT_ERROR_INVALID_ARGUMENT` is returned otherwise).
212212

213-
The memory visibility mode parameter must be set to `UMF_MEM_MAP_SYNC` in case of FSDAX.
213+
The memory visibility mode parameter must be set to `UMF_MEM_MAP_SHARED` in case of FSDAX.
214214

215215
##### Requirements
216216

‎examples/dram_and_fsdax/dram_and_fsdax.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,8 @@ static umf_memory_pool_handle_t create_fsdax_pool(const char *path) {
5050

5151
umf_file_memory_provider_params_t params_fsdax =
5252
umfFileMemoryProviderParamsDefault(path);
53-
// FSDAX requires mapping the UMF_MEM_MAP_SYNC flag
54-
params_fsdax.visibility = UMF_MEM_MAP_SYNC;
53+
// FSDAX requires mapping the UMF_MEM_MAP_SHARED flag
54+
params_fsdax.visibility = UMF_MEM_MAP_SHARED;
5555

5656
umf_result = umfMemoryProviderCreate(umfFileMemoryProviderOps(),
5757
&params_fsdax, &provider_fsdax);

‎include/umf/memory_provider.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ extern "C" {
2121
typedef enum umf_memory_visibility_t {
2222
UMF_MEM_MAP_PRIVATE = 1, ///< private memory mapping
2323
UMF_MEM_MAP_SHARED, ///< shared memory mapping (Linux only)
24-
UMF_MEM_MAP_SYNC, ///< direct mapping of persistent memory (supported only for files supporting DAX, Linux only)
2524
} umf_memory_visibility_t;
2625

2726
/// @brief Protection of the memory allocations

‎src/provider/provider_devdax_memory.c‎

Lines changed: 36 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -139,21 +139,31 @@ static umf_result_t devdax_initialize(void *params, void **provider) {
139139
goto err_free_devdax_provider;
140140
}
141141

142-
unsigned map_sync_flag = 0;
143-
utils_translate_mem_visibility_flag(UMF_MEM_MAP_SYNC, &map_sync_flag);
142+
bool is_dax = false;
144143

145-
// mmap /dev/dax with the MAP_SYNC xor MAP_SHARED flag (if MAP_SYNC fails)
144+
// mmap /dev/dax with the MAP_SYNC
146145
devdax_provider->base = utils_mmap_file(
147-
NULL, devdax_provider->size, devdax_provider->protection, map_sync_flag,
148-
fd, 0 /* offset */, NULL);
146+
NULL, devdax_provider->size, devdax_provider->protection, 0 /* flags */,
147+
fd, 0 /* offset */, &is_dax);
149148
utils_close_fd(fd);
150149
if (devdax_provider->base == NULL) {
151-
LOG_PDEBUG("devdax memory mapping failed (path=%s, size=%zu)",
150+
LOG_PDEBUG("mapping the devdax failed (path=%s, size=%zu)",
152151
in_params->path, devdax_provider->size);
153152
ret = UMF_RESULT_ERROR_UNKNOWN;
154153
goto err_free_devdax_provider;
155154
}
156155

156+
if (!is_dax) {
157+
LOG_ERR("mapping the devdax with MAP_SYNC failed: %s", in_params->path);
158+
ret = UMF_RESULT_ERROR_UNKNOWN;
159+
160+
if (devdax_provider->base) {
161+
utils_munmap(devdax_provider->base, devdax_provider->size);
162+
}
163+
164+
goto err_free_devdax_provider;
165+
}
166+
157167
LOG_DEBUG("devdax memory mapped (path=%s, size=%zu, addr=%p)",
158168
in_params->path, devdax_provider->size, devdax_provider->base);
159169

@@ -433,6 +443,8 @@ static umf_result_t devdax_open_ipc_handle(void *provider,
433443
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
434444
}
435445

446+
*ptr = NULL;
447+
436448
devdax_ipc_data_t *devdax_ipc_data = (devdax_ipc_data_t *)providerIpcData;
437449

438450
int fd = utils_devdax_open(devdax_ipc_data->path);
@@ -441,9 +453,6 @@ static umf_result_t devdax_open_ipc_handle(void *provider,
441453
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
442454
}
443455

444-
unsigned map_sync_flag = 0;
445-
utils_translate_mem_visibility_flag(UMF_MEM_MAP_SYNC, &map_sync_flag);
446-
447456
// It is just a workaround for case when
448457
// devdax_alloc() was called with the size argument
449458
// that is not a multiplier of DEVDAX_PAGE_SIZE_2MB.
@@ -452,34 +461,42 @@ static umf_result_t devdax_open_ipc_handle(void *provider,
452461
utils_align_ptr_down_size_up((void **)&offset_aligned, &length_aligned,
453462
DEVDAX_PAGE_SIZE_2MB);
454463

455-
// mmap /dev/dax with the MAP_SYNC xor MAP_SHARED flag (if MAP_SYNC fails)
464+
bool is_dax = false;
465+
466+
// mmap /dev/dax with the MAP_SYNC
456467
char *addr =
457468
utils_mmap_file(NULL, length_aligned, devdax_ipc_data->protection,
458-
map_sync_flag, fd, offset_aligned, NULL);
469+
0 /* flags */, fd, offset_aligned, &is_dax);
470+
(void)utils_close_fd(fd);
459471
if (addr == NULL) {
460-
devdax_store_last_native_error(UMF_DEVDAX_RESULT_ERROR_ALLOC_FAILED,
461-
errno);
462-
463472
LOG_PERR("devdax mapping failed (path: %s, size: %zu, protection: %i, "
464473
"fd: %i, offset: %zu)",
465474
devdax_ipc_data->path, length_aligned,
466475
devdax_ipc_data->protection, fd, offset_aligned);
467476

468-
*ptr = NULL;
469-
(void)utils_close_fd(fd);
470-
477+
devdax_store_last_native_error(UMF_DEVDAX_RESULT_ERROR_ALLOC_FAILED,
478+
errno);
471479
return UMF_RESULT_ERROR_MEMORY_PROVIDER_SPECIFIC;
472480
}
473481

482+
if (!is_dax) {
483+
LOG_ERR("mapping the devdax with MAP_SYNC failed: %s",
484+
devdax_ipc_data->path);
485+
486+
if (addr) {
487+
utils_munmap(addr, length_aligned);
488+
}
489+
490+
return UMF_RESULT_ERROR_UNKNOWN;
491+
}
492+
474493
LOG_DEBUG("devdax mapped (path: %s, size: %zu, protection: %i, fd: %i, "
475494
"offset: %zu) to address %p",
476495
devdax_ipc_data->path, length_aligned,
477496
devdax_ipc_data->protection, fd, offset_aligned, addr);
478497

479498
*ptr = addr;
480499

481-
(void)utils_close_fd(fd);
482-
483500
return UMF_RESULT_SUCCESS;
484501
}
485502

‎src/provider/provider_file_memory.c‎

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ typedef struct file_memory_provider_t {
5151
unsigned visibility; // memory visibility mode
5252
size_t page_size; // minimum page size
5353

54-
// IPC is enabled only for UMF_MEM_MAP_SHARED or UMF_MEM_MAP_SYNC visibility
54+
// IPC is enabled only for the UMF_MEM_MAP_SHARED visibility
5555
bool IPC_enabled;
5656

5757
critnib *mmaps; // a critnib map storing mmap mappings (addr, size)
@@ -114,9 +114,8 @@ file_translate_params(umf_file_memory_provider_params_t *in_params,
114114
return result;
115115
}
116116

117-
// IPC is enabled only for UMF_MEM_MAP_SHARED or UMF_MEM_MAP_SYNC visibility
118-
provider->IPC_enabled = (in_params->visibility == UMF_MEM_MAP_SHARED ||
119-
in_params->visibility == UMF_MEM_MAP_SYNC);
117+
// IPC is enabled only for the UMF_MEM_MAP_SHARED visibility
118+
provider->IPC_enabled = (in_params->visibility == UMF_MEM_MAP_SHARED);
120119

121120
return UMF_RESULT_SUCCESS;
122121
}
@@ -594,8 +593,7 @@ static umf_result_t file_get_ipc_handle_size(void *provider, size_t *size) {
594593

595594
file_memory_provider_t *file_provider = (file_memory_provider_t *)provider;
596595
if (!file_provider->IPC_enabled) {
597-
LOG_ERR("memory visibility mode is not UMF_MEM_MAP_SHARED nor "
598-
"UMF_MEM_MAP_SYNC")
596+
LOG_ERR("memory visibility mode is not UMF_MEM_MAP_SHARED")
599597
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
600598
}
601599

@@ -612,8 +610,7 @@ static umf_result_t file_get_ipc_handle(void *provider, const void *ptr,
612610

613611
file_memory_provider_t *file_provider = (file_memory_provider_t *)provider;
614612
if (!file_provider->IPC_enabled) {
615-
LOG_ERR("memory visibility mode is not UMF_MEM_MAP_SHARED nor "
616-
"UMF_MEM_MAP_SYNC")
613+
LOG_ERR("memory visibility mode is not UMF_MEM_MAP_SHARED")
617614
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
618615
}
619616

@@ -643,8 +640,7 @@ static umf_result_t file_put_ipc_handle(void *provider, void *providerIpcData) {
643640

644641
file_memory_provider_t *file_provider = (file_memory_provider_t *)provider;
645642
if (!file_provider->IPC_enabled) {
646-
LOG_ERR("memory visibility mode is not UMF_MEM_MAP_SHARED nor "
647-
"UMF_MEM_MAP_SYNC")
643+
LOG_ERR("memory visibility mode is not UMF_MEM_MAP_SHARED")
648644
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
649645
}
650646

@@ -665,8 +661,7 @@ static umf_result_t file_open_ipc_handle(void *provider, void *providerIpcData,
665661

666662
file_memory_provider_t *file_provider = (file_memory_provider_t *)provider;
667663
if (!file_provider->IPC_enabled) {
668-
LOG_ERR("memory visibility mode is not UMF_MEM_MAP_SHARED nor "
669-
"UMF_MEM_MAP_SYNC")
664+
LOG_ERR("memory visibility mode is not UMF_MEM_MAP_SHARED")
670665
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
671666
}
672667

@@ -712,8 +707,7 @@ static umf_result_t file_close_ipc_handle(void *provider, void *ptr,
712707

713708
file_memory_provider_t *file_provider = (file_memory_provider_t *)provider;
714709
if (!file_provider->IPC_enabled) {
715-
LOG_ERR("memory visibility mode is not UMF_MEM_MAP_SHARED nor "
716-
"UMF_MEM_MAP_SYNC")
710+
LOG_ERR("memory visibility mode is not UMF_MEM_MAP_SHARED")
717711
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
718712
}
719713

‎src/utils/utils_linux_common.c‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,6 @@ utils_translate_mem_visibility_flag(umf_memory_visibility_t in_flag,
3232
case UMF_MEM_MAP_SHARED:
3333
*out_flag = MAP_SHARED;
3434
return UMF_RESULT_SUCCESS;
35-
case UMF_MEM_MAP_SYNC:
36-
*out_flag = MAP_SYNC;
37-
return UMF_RESULT_SUCCESS;
3835
}
3936
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
4037
}
@@ -95,7 +92,7 @@ void *utils_mmap_file(void *hint_addr, size_t length, int prot, int flags,
9592

9693
/* try to mmap with MAP_SHARED flag (without MAP_SYNC) */
9794
if (errno == EINVAL || errno == ENOTSUP || errno == EOPNOTSUPP) {
98-
const int shared_flags = (flags & (~MAP_SYNC)) | MAP_SHARED;
95+
const int shared_flags = flags | MAP_SHARED;
9996
addr = utils_mmap(hint_addr, length, prot, shared_flags, fd, fd_offset);
10097
if (addr) {
10198
LOG_DEBUG("file mapped with the MAP_SHARED flag (fd=%i, "

‎src/utils/utils_macosx_common.c‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,6 @@ utils_translate_mem_visibility_flag(umf_memory_visibility_t in_flag,
2424
return UMF_RESULT_SUCCESS;
2525
case UMF_MEM_MAP_SHARED:
2626
return UMF_RESULT_ERROR_NOT_SUPPORTED; // not supported on MacOSX
27-
case UMF_MEM_MAP_SYNC:
28-
return UMF_RESULT_ERROR_NOT_SUPPORTED; // not supported on MacOSX
2927
}
3028
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
3129
}

‎src/utils/utils_windows_common.c‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,6 @@ utils_translate_mem_visibility_flag(umf_memory_visibility_t in_flag,
9696
return UMF_RESULT_SUCCESS;
9797
case UMF_MEM_MAP_SHARED:
9898
return UMF_RESULT_ERROR_NOT_SUPPORTED; // not supported on Windows yet
99-
case UMF_MEM_MAP_SYNC:
100-
return UMF_RESULT_ERROR_NOT_SUPPORTED; // not supported on Windows yet
10199
}
102100
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
103101
}

‎test/ipc_file_prov_consumer.c‎

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -18,30 +18,16 @@
1818

1919
int main(int argc, char *argv[]) {
2020
if (argc < 3) {
21-
fprintf(stderr, "usage: %s <port> <file_name> <fsdax>\n", argv[0]);
22-
fprintf(stderr, " <fsdax> should be \"FSDAX\" or \"fsdax\" if "
23-
"<file_name> is located on FSDAX \n");
21+
fprintf(stderr, "usage: %s <port> <file_name>\n", argv[0]);
2422
return -1;
2523
}
2624

2725
int port = atoi(argv[1]);
2826
char *file_name = argv[2];
29-
bool is_fsdax = false;
30-
31-
if (argc >= 4) {
32-
if (strncasecmp(argv[3], "FSDAX", strlen("FSDAX")) == 0) {
33-
is_fsdax = true;
34-
}
35-
}
3627

3728
umf_file_memory_provider_params_t file_params;
38-
3929
file_params = umfFileMemoryProviderParamsDefault(file_name);
40-
if (is_fsdax) {
41-
file_params.visibility = UMF_MEM_MAP_SYNC;
42-
} else {
43-
file_params.visibility = UMF_MEM_MAP_SHARED;
44-
}
30+
file_params.visibility = UMF_MEM_MAP_SHARED;
4531

4632
void *pool_params = NULL;
4733

‎test/ipc_file_prov_fsdax.sh‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,13 @@ UMF_LOG_VAL="level:debug;flush:debug;output:stderr;pid:yes"
3131
rm -f ${FILE_NAME}
3232

3333
echo "Starting ipc_file_prov_fsdax CONSUMER on port $PORT ..."
34-
UMF_LOG=$UMF_LOG_VAL ./umf_test-ipc_file_prov_consumer $PORT $FILE_NAME "FSDAX" &
34+
UMF_LOG=$UMF_LOG_VAL ./umf_test-ipc_file_prov_consumer $PORT $FILE_NAME &
3535

3636
echo "Waiting 1 sec ..."
3737
sleep 1
3838

3939
echo "Starting ipc_file_prov_fsdax PRODUCER on port $PORT ..."
40-
UMF_LOG=$UMF_LOG_VAL ./umf_test-ipc_file_prov_producer $PORT $FILE_NAME_2 "FSDAX"
40+
UMF_LOG=$UMF_LOG_VAL ./umf_test-ipc_file_prov_producer $PORT $FILE_NAME_2
4141

4242
# remove the SHM file
4343
rm -f ${FILE_NAME}

0 commit comments

Comments
 (0)