Skip to content

Commit ab94622

Browse files
ldorauldorau
committed
Deprecate the UMF_MEM_MAP_SYNC flag
The UMF_MEM_MAP_SYNC flag is DEPRECATED, (UMF_MEM_MAP_SHARED should be used instead). UMF_MEM_MAP_SYNC will be removed in the next release. Verify if /dev/dax was mapped with MAP_SYNC. Signed-off-by: Lukasz Dorau <[email protected]>
1 parent 37a4b71 commit ab94622

12 files changed

+58
-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: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ 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)
24+
UMF_MEM_MAP_SYNC =
25+
UMF_MEM_MAP_SHARED, ///< DEPRECATED (use UMF_MEM_MAP_SHARED instead), will be removed in the next release
2526
} umf_memory_visibility_t;
2627

2728
/// @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

@@ -437,6 +447,8 @@ static umf_result_t devdax_open_ipc_handle(void *provider,
437447
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
438448
}
439449

450+
*ptr = NULL;
451+
440452
devdax_ipc_data_t *devdax_ipc_data = (devdax_ipc_data_t *)providerIpcData;
441453

442454
int fd = utils_devdax_open(devdax_ipc_data->path);
@@ -445,9 +457,6 @@ static umf_result_t devdax_open_ipc_handle(void *provider,
445457
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
446458
}
447459

448-
unsigned map_sync_flag = 0;
449-
utils_translate_mem_visibility_flag(UMF_MEM_MAP_SYNC, &map_sync_flag);
450-
451460
// It is just a workaround for case when
452461
// devdax_alloc() was called with the size argument
453462
// that is not a multiplier of DEVDAX_PAGE_SIZE_2MB.
@@ -456,34 +465,42 @@ static umf_result_t devdax_open_ipc_handle(void *provider,
456465
utils_align_ptr_down_size_up((void **)&offset_aligned, &length_aligned,
457466
DEVDAX_PAGE_SIZE_2MB);
458467

459-
// mmap /dev/dax with the MAP_SYNC xor MAP_SHARED flag (if MAP_SYNC fails)
468+
bool is_dax = false;
469+
470+
// mmap /dev/dax with the MAP_SYNC
460471
char *addr =
461472
utils_mmap_file(NULL, length_aligned, devdax_ipc_data->protection,
462-
map_sync_flag, fd, offset_aligned, NULL);
473+
0 /* flags */, fd, offset_aligned, &is_dax);
474+
(void)utils_close_fd(fd);
463475
if (addr == NULL) {
464-
devdax_store_last_native_error(UMF_DEVDAX_RESULT_ERROR_ALLOC_FAILED,
465-
errno);
466-
467476
LOG_PERR("devdax mapping failed (path: %s, size: %zu, protection: %i, "
468477
"fd: %i, offset: %zu)",
469478
devdax_ipc_data->path, length_aligned,
470479
devdax_ipc_data->protection, fd, offset_aligned);
471480

472-
*ptr = NULL;
473-
(void)utils_close_fd(fd);
474-
481+
devdax_store_last_native_error(UMF_DEVDAX_RESULT_ERROR_ALLOC_FAILED,
482+
errno);
475483
return UMF_RESULT_ERROR_MEMORY_PROVIDER_SPECIFIC;
476484
}
477485

486+
if (!is_dax) {
487+
LOG_ERR("mapping the devdax with MAP_SYNC failed: %s",
488+
devdax_ipc_data->path);
489+
490+
if (addr) {
491+
utils_munmap(addr, length_aligned);
492+
}
493+
494+
return UMF_RESULT_ERROR_UNKNOWN;
495+
}
496+
478497
LOG_DEBUG("devdax mapped (path: %s, size: %zu, protection: %i, fd: %i, "
479498
"offset: %zu) to address %p",
480499
devdax_ipc_data->path, length_aligned,
481500
devdax_ipc_data->protection, fd, offset_aligned, addr);
482501

483502
*ptr = addr;
484503

485-
(void)utils_close_fd(fd);
486-
487504
return UMF_RESULT_SUCCESS;
488505
}
489506

‎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}

‎test/ipc_file_prov_producer.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/provider_file_memory.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ TEST_F(test, test_if_mapped_with_MAP_SYNC) {
137137
}
138138

139139
auto params = umfFileMemoryProviderParamsDefault(path);
140-
params.visibility = UMF_MEM_MAP_SYNC;
140+
params.visibility = UMF_MEM_MAP_SHARED;
141141

142142
umf_result = umfMemoryProviderCreate(umfFileMemoryProviderOps(), &params,
143143
&hProvider);

0 commit comments

Comments
 (0)