Skip to content

Commit 508162e

Browse files
ldorauldorau
authored
Merge pull request #887 from ldorau/Deprecate_the_UMF_MEM_MAP_SYNC_flag
Remove the `UMF_MEM_MAP_SYNC` flag
2 parents a567d1b + 199e754 commit 508162e

13 files changed

+102
-119
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: 37 additions & 20 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)
146-
devdax_provider->base = utils_mmap_file(NULL, devdax_provider->size,
147-
devdax_provider->protection,
148-
map_sync_flag, fd, 0 /* offset */);
144+
// mmap /dev/dax with the MAP_SYNC
145+
devdax_provider->base = utils_mmap_file(
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);
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: 11 additions & 17 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
}
@@ -293,8 +292,8 @@ static umf_result_t file_mmap_aligned(file_memory_provider_t *file_provider,
293292
ASSERT_IS_ALIGNED(extended_size, page_size);
294293
ASSERT_IS_ALIGNED(aligned_offset_fd, page_size);
295294

296-
void *ptr =
297-
utils_mmap_file(NULL, extended_size, prot, flag, fd, aligned_offset_fd);
295+
void *ptr = utils_mmap_file(NULL, extended_size, prot, flag, fd,
296+
aligned_offset_fd, NULL);
298297
if (ptr == NULL) {
299298
LOG_PERR("memory mapping failed");
300299
return UMF_RESULT_ERROR_MEMORY_PROVIDER_SPECIFIC;
@@ -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

@@ -683,7 +678,7 @@ static umf_result_t file_open_ipc_handle(void *provider, void *providerIpcData,
683678

684679
char *addr = utils_mmap_file(
685680
NULL, file_ipc_data->size, file_ipc_data->protection,
686-
file_ipc_data->visibility, fd, file_ipc_data->offset_fd);
681+
file_ipc_data->visibility, fd, file_ipc_data->offset_fd, NULL);
687682
(void)utils_close_fd(fd);
688683
if (addr == NULL) {
689684
file_store_last_native_error(UMF_FILE_RESULT_ERROR_ALLOC_FAILED, errno);
@@ -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_common.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#define UMF_COMMON_H 1
1212

1313
#include <assert.h>
14+
#include <stdbool.h>
1415
#include <stddef.h>
1516
#include <stdint.h>
1617

@@ -136,7 +137,7 @@ void *utils_mmap(void *hint_addr, size_t length, int prot, int flag, int fd,
136137
size_t fd_offset);
137138

138139
void *utils_mmap_file(void *hint_addr, size_t length, int prot, int flags,
139-
int fd, size_t fd_offset);
140+
int fd, size_t fd_offset, bool *map_sync);
140141

141142
int utils_munmap(void *addr, size_t length);
142143

‎src/utils/utils_linux_common.c

Lines changed: 36 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
#include <errno.h>
1111
#include <fcntl.h>
12+
#include <stdbool.h>
1213
#include <sys/mman.h>
1314
#include <sys/stat.h>
1415
#include <sys/syscall.h>
@@ -31,78 +32,78 @@ utils_translate_mem_visibility_flag(umf_memory_visibility_t in_flag,
3132
case UMF_MEM_MAP_SHARED:
3233
*out_flag = MAP_SHARED;
3334
return UMF_RESULT_SUCCESS;
34-
case UMF_MEM_MAP_SYNC:
35-
*out_flag = MAP_SYNC;
36-
return UMF_RESULT_SUCCESS;
3735
}
3836
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
3937
}
4038

4139
/*
4240
* Map given file into memory.
43-
* If (flags & MAP_PRIVATE) it uses just mmap. Otherwise, if (flags & MAP_SYNC)
44-
* it tries to mmap with (flags | MAP_SHARED_VALIDATE | MAP_SYNC)
45-
* which allows flushing from the user-space. If MAP_SYNC fails and the user
46-
* did not specify it by himself it tries to mmap with (flags | MAP_SHARED).
41+
* If (flags & MAP_PRIVATE) it uses just mmap. Otherwise, it tries to mmap
42+
* with (flags | MAP_SHARED_VALIDATE | MAP_SYNC) which allows flushing
43+
* from the user-space. If MAP_SYNC fails and if the user did not specify
44+
* this flag by himself, it falls back to the mmap with (flags | MAP_SHARED).
4745
*/
4846
void *utils_mmap_file(void *hint_addr, size_t length, int prot, int flags,
49-
int fd, size_t fd_offset) {
47+
int fd, size_t fd_offset, bool *map_sync) {
5048
void *addr;
5149

50+
if (map_sync) {
51+
*map_sync = false;
52+
}
53+
5254
/*
5355
* MAP_PRIVATE and MAP_SHARED are mutually exclusive,
5456
* therefore mmap with MAP_PRIVATE is executed separately.
5557
*/
5658
if (flags & MAP_PRIVATE) {
5759
addr = utils_mmap(hint_addr, length, prot, flags, fd, fd_offset);
58-
if (addr == MAP_FAILED) {
60+
if (addr == NULL) {
5961
LOG_PERR("mapping file with the MAP_PRIVATE flag failed (fd=%i, "
60-
"offset=%zu, length=%zu)",
61-
fd, fd_offset, length);
62+
"offset=%zu, length=%zu, flags=%i)",
63+
fd, fd_offset, length, flags);
6264
return NULL;
6365
}
6466

6567
LOG_DEBUG("file mapped with the MAP_PRIVATE flag (fd=%i, offset=%zu, "
66-
"length=%zu)",
67-
fd, fd_offset, length);
68+
"length=%zu, flags=%i)",
69+
fd, fd_offset, length, flags);
6870

6971
return addr;
7072
}
7173

7274
errno = 0;
7375

74-
if (flags & MAP_SYNC) {
75-
/* try to mmap with MAP_SYNC flag */
76-
const int sync_flags = MAP_SHARED_VALIDATE | MAP_SYNC;
77-
addr = utils_mmap(hint_addr, length, prot, flags | sync_flags, fd,
78-
fd_offset);
79-
if (addr) {
80-
LOG_DEBUG("file mapped with the MAP_SYNC flag (fd=%i, offset=%zu, "
81-
"length=%zu)",
82-
fd, fd_offset, length);
83-
return addr;
76+
/* try to mmap with MAP_SYNC flag */
77+
const int sync_flags = flags | MAP_SHARED_VALIDATE | MAP_SYNC;
78+
addr = utils_mmap(hint_addr, length, prot, sync_flags, fd, fd_offset);
79+
if (addr) {
80+
LOG_DEBUG("file mapped with the MAP_SYNC flag (fd=%i, offset=%zu, "
81+
"length=%zu, flags=%i)",
82+
fd, fd_offset, length, sync_flags);
83+
if (map_sync) {
84+
*map_sync = true;
8485
}
85-
86-
LOG_PERR("mapping file with the MAP_SYNC flag failed (fd=%i, "
87-
"offset=%zu, length=%zu)",
88-
fd, fd_offset, length);
86+
return addr;
8987
}
9088

91-
if ((!(flags & MAP_SYNC)) || errno == EINVAL || errno == ENOTSUP ||
92-
errno == EOPNOTSUPP) {
93-
/* try to mmap with MAP_SHARED flag (without MAP_SYNC) */
94-
const int shared_flags = (flags & (~MAP_SYNC)) | MAP_SHARED;
89+
LOG_PERR("mapping file with the MAP_SYNC flag failed (fd=%i, offset=%zu, "
90+
"length=%zu, flags=%i)",
91+
fd, fd_offset, length, sync_flags);
92+
93+
/* try to mmap with MAP_SHARED flag (without MAP_SYNC) */
94+
if (errno == EINVAL || errno == ENOTSUP || errno == EOPNOTSUPP) {
95+
const int shared_flags = flags | MAP_SHARED;
9596
addr = utils_mmap(hint_addr, length, prot, shared_flags, fd, fd_offset);
9697
if (addr) {
9798
LOG_DEBUG("file mapped with the MAP_SHARED flag (fd=%i, "
98-
"offset=%zu, length=%zu)",
99-
fd, fd_offset, length);
99+
"offset=%zu, length=%zu, flags=%i)",
100+
fd, fd_offset, length, shared_flags);
100101
return addr;
101102
}
102103

103104
LOG_PERR("mapping file with the MAP_SHARED flag failed (fd=%i, "
104-
"offset=%zu, length=%zu)",
105-
fd, fd_offset, length);
105+
"offset=%zu, length=%zu, flags=%i)",
106+
fd, fd_offset, length, shared_flags);
106107
}
107108

108109
return NULL;

0 commit comments

Comments
 (0)