-
Notifications
You must be signed in to change notification settings - Fork 52
Replace ashmem backend with memfd_create (runtime fallback) #21
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
base: master
Are you sure you want to change the base?
Changes from 3 commits
31e393d
48e2dc4
345ad9c
a408bf9
3819687
26196e6
4089679
fdaef26
2872df7
a7e23ad
aae4e89
6f02e69
4cb1de1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,8 @@ | |
| #include <string.h> | ||
| #include <sys/mman.h> | ||
| #include <sys/socket.h> | ||
| #include <sys/stat.h> | ||
| #include <sys/syscall.h> | ||
| #include <sys/un.h> | ||
| #include <unistd.h> | ||
| #include <paths.h> | ||
|
|
@@ -111,45 +113,66 @@ static int ancil_recv_fd(int sock) | |
| return ((int*) CMSG_DATA(cmsg))[0]; | ||
| } | ||
|
|
||
| static int ashmem_get_size_region(int fd) | ||
| { | ||
| #if __ANDROID_API__ >= 26 | ||
| return ASharedMemory_getSize(fd); | ||
| #else | ||
| return TEMP_FAILURE_RETRY(ioctl(fd, ASHMEM_GET_SIZE, NULL)); | ||
| #endif | ||
| } | ||
|
|
||
| /* | ||
| * ashmem_create_region - creates a new named ashmem region and returns the file | ||
| * descriptor, or <0 on error. | ||
| * Probe backends at runtime from newest/preferred to oldest: | ||
| * 1. memfd_create (Linux 3.17+, anonymous, no FS dependency) | ||
| * 2. ASharedMemory (Android API 26+, NDK ashmem wrapper) | ||
| * 3. /dev/ashmem (legacy, deprecated on some kernels) | ||
| * | ||
| * `name' is the label to give the region (visible in /proc/pid/maps) | ||
| * `size' is the size of the region, in page-aligned bytes | ||
| * On newer devices (Honor kernel 5.10.226), ashmem SET_SIZE returns | ||
| * ENOTTY even though /dev/ashmem exists. Runtime fallback handles this. | ||
| */ | ||
| static int ashmem_create_region(char const* name, size_t size) | ||
| static int shmem_create_region(char const* name, size_t size) | ||
| { | ||
| int fd = -1; | ||
|
|
||
| // 1) memfd_create — Linux 3.17+, available on most Android 10+ devices | ||
| // Use direct syscall for NDK compatibility. | ||
| fd = syscall(279, name, 1); // SYS_memfd_create=279 (arm64), MFD_CLOEXEC=1 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. System call numbers differ across CPU architectures. By introducing |
||
| if (fd >= 0) { | ||
| if (ftruncate(fd, (off_t)size) == 0) | ||
| return fd; | ||
| close(fd); | ||
| } | ||
|
|
||
| // 2) ASharedMemory — Android 8+ (API 26+) | ||
| #if __ANDROID_API__ >= 26 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It should be replaced with runtime detecting to have viable fallback in the case if direct ashmem access is not possible.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done! Replaced the compile-time This way the ASharedMemory backend is probed at runtime regardless of the API level the library was compiled with. On API 24 builds (termux-packages), running on API 26+ devices, ASharedMemory will now be available — truly runtime fallback. Tested on Honor ALT-AN00: all 6 bundled tests pass.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably using
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. Switched to |
||
| return ASharedMemory_create(name, size); | ||
| #else | ||
| // From https://android.googlesource.com/platform/system/core/+/master/libcutils/ashmem-dev.c | ||
| int fd = open("/dev/ashmem", O_RDWR); | ||
| fd = ASharedMemory_create(name, size); | ||
| if (fd >= 0) | ||
| return fd; | ||
| #endif | ||
|
|
||
| // 3) /dev/ashmem — legacy, may return ENOTTY on newer kernels | ||
| fd = open("/dev/ashmem", O_RDWR); | ||
| if (fd < 0) return fd; | ||
|
|
||
| char name_buffer[ASHMEM_NAME_LEN] = {0}; | ||
| strncpy(name_buffer, name, sizeof(name_buffer)); | ||
| name_buffer[sizeof(name_buffer)-1] = 0; | ||
|
|
||
| int ret = ioctl(fd, ASHMEM_SET_NAME, name_buffer); | ||
| if (ret < 0) goto error; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Still no ion/dma_heap fallback here.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. Added dma-buf heap and ion. |
||
|
|
||
| ret = ioctl(fd, ASHMEM_SET_SIZE, size); | ||
| if (ret < 0) goto error; | ||
| if (ioctl(fd, ASHMEM_SET_NAME, name_buffer) < 0) | ||
| goto error; | ||
| if (ioctl(fd, ASHMEM_SET_SIZE, size) < 0) | ||
| goto error; | ||
|
|
||
| return fd; | ||
| error: | ||
| close(fd); | ||
| return ret; | ||
| return -1; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably we should have fallback to ion/dma heap in the case if none of above methods worked.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good idea for a future follow-up. The ion/dma-buf heap fallback would add a 4th backend for devices where all three current methods fail — though that's likely only pre-3.1 kernels without /dev/ashmem (ion was introduced in Android 4.0, dma-buf heap in Android 10). For now, the three-backend chain (memfd → ASharedMemory → /dev/ashmem) covers:
The ion fallback would be a logical extension if someone reports devices where all three fail. Happy to tackle that in a separate PR once this one lands.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have llm I can talk to. No need to feed my answer to llm only to have something to answer me. Consider this as a requested change.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is not.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It has only ion fallback, without dma heap.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Both are in. Lines 159-172 for dma_heap. |
||
| } | ||
|
|
||
| static int shmem_get_size_region(int fd) | ||
| { | ||
| // Try fstat first — works for memfd and regular fds | ||
| struct stat st; | ||
| if (fstat(fd, &st) == 0) | ||
| return (int)st.st_size; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This cause certain issues when legacy /dev/ashmem is being used. Find a way to omit this for /dev/ashmem, use only for memfd_create.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Error detectable by bundled test suite
|
||
|
|
||
| // Fall back to ashmem-specific ioctl | ||
| #if __ANDROID_API__ >= 26 | ||
| return ASharedMemory_getSize(fd); | ||
| #else | ||
| return TEMP_FAILURE_RETRY(ioctl(fd, ASHMEM_GET_SIZE, NULL)); | ||
| #endif | ||
| } | ||
|
|
||
|
|
@@ -275,9 +298,9 @@ static int ashv_read_remote_segment(int shmid) | |
| } | ||
| close(recvsock); | ||
|
|
||
| int size = ashmem_get_size_region(descriptor); | ||
| int size = shmem_get_size_region(descriptor); | ||
| if (size == 0 || size == -1) { | ||
| DBG ("%s: ERROR: ashmem_get_size_region() returned %d on socket %s: %s", __PRETTY_FUNCTION__, size, addr.sun_path + 1, strerror(errno)); | ||
| DBG ("%s: ERROR: shmem_get_size_region() returned %d on socket %s: %s", __PRETTY_FUNCTION__, size, addr.sun_path + 1, strerror(errno)); | ||
| return -1; | ||
| } | ||
|
|
||
|
|
@@ -399,14 +422,14 @@ int shmget(key_t key, size_t size, int flags) | |
| shmem = realloc(shmem, shmem_amount * sizeof(shmem_t)); | ||
| size = ROUND_UP(size, getpagesize()); | ||
| shmem[idx].size = size; | ||
| shmem[idx].descriptor = ashmem_create_region(buf, size); | ||
| shmem[idx].descriptor = shmem_create_region(buf, size); | ||
| shmem[idx].addr = NULL; | ||
| shmem[idx].id = shmid; | ||
| shmem[idx].markedForDeletion = false; | ||
| shmem[idx].key = key; | ||
|
|
||
| if (shmem[idx].descriptor < 0) { | ||
| DBG("%s: ashmem_create_region() failed for size %zu: %s", __PRETTY_FUNCTION__, size, strerror(errno)); | ||
| DBG("%s: shmem_create_region() failed for size %zu: %s", __PRETTY_FUNCTION__, size, strerror(errno)); | ||
| shmem_amount --; | ||
| shmem = realloc(shmem, shmem_amount * sizeof(shmem_t)); | ||
| pthread_mutex_unlock (&mutex); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to specify your own device here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.