-
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 11 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 |
|---|---|---|
| @@ -1,24 +1,29 @@ | ||
| #define __ANDROID_UNAVAILABLE_SYMBOLS_ARE_WEAK__ 1 | ||
| #include <android/log.h> | ||
| #if __ANDROID_API__ >= 26 | ||
| #include <android/sharedmem.h> | ||
| #endif | ||
| #include <errno.h> | ||
| #include <pthread.h> | ||
| #include <stdbool.h> | ||
| #include <stdint.h> | ||
| #include <stdio.h> | ||
| #include <stdlib.h> | ||
| #include <string.h> | ||
| #define _GNU_SOURCE | ||
| #include <sys/mman.h> | ||
| #include <sys/socket.h> | ||
| #include <sys/stat.h> | ||
| #include <sys/syscall.h> | ||
| #include <linux/memfd.h> | ||
| #include <sys/un.h> | ||
| #include <unistd.h> | ||
| #include <paths.h> | ||
|
|
||
| #if __ANDROID_API__ < 26 | ||
| #ifndef __NR_memfd_create | ||
| #error "__NR_memfd_create not defined" | ||
| #endif | ||
|
|
||
| #define __u32 uint32_t | ||
| #include <linux/ashmem.h> | ||
| #endif | ||
|
|
||
| #include "shm.h" | ||
|
|
||
|
|
@@ -111,46 +116,105 @@ 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. | ||
| * | ||
| * `name' is the label to give the region (visible in /proc/pid/maps) | ||
| * `size' is the size of the region, in page-aligned bytes | ||
| * Probe backends at runtime from newest/preferred to oldest: | ||
| * 1. memfd_create (Linux 3.17+) | ||
| * 2. ASharedMemory (Android API 26+) | ||
| * 3. /dev/ashmem (legacy) | ||
| * 4. dma-buf heap (Android 10+, /dev/dma_heap/system) | ||
| * 5. ion (Android 4.0–9, /dev/ion) | ||
| */ | ||
| static int ashmem_create_region(char const* name, size_t size) | ||
| static int shmem_create_region(char const* name, size_t size) | ||
| { | ||
| #if __ANDROID_API__ >= 26 | ||
| 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); | ||
| if (fd < 0) return fd; | ||
| int fd = -1; | ||
|
|
||
| // 1) memfd_create — Linux 3.17+, available on most Android 10+ devices | ||
| // Use direct syscall for NDK compatibility. | ||
| fd = syscall(__NR_memfd_create, name, MFD_CLOEXEC); | ||
| if (fd >= 0) { | ||
| if (ftruncate(fd, (off_t)size) == 0) | ||
| return fd; | ||
| close(fd); | ||
| } | ||
|
|
||
| char name_buffer[ASHMEM_NAME_LEN] = {0}; | ||
| strncpy(name_buffer, name, sizeof(name_buffer)); | ||
| name_buffer[sizeof(name_buffer)-1] = 0; | ||
| // 2) ASharedMemory — Android 8+ (API 26+), weak symbol | ||
| if (ASharedMemory_create) { | ||
| fd = ASharedMemory_create(name, size); | ||
| if (fd >= 0) return fd; | ||
| } | ||
|
|
||
| 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. |
||
| // 3) /dev/ashmem — legacy, may return ENOTTY on newer kernels | ||
| fd = open("/dev/ashmem", O_RDWR); | ||
| if (fd >= 0) { | ||
| char name_buffer[ASHMEM_NAME_LEN] = {0}; | ||
| strncpy(name_buffer, name, sizeof(name_buffer)); | ||
| name_buffer[sizeof(name_buffer)-1] = 0; | ||
|
|
||
| if (ioctl(fd, ASHMEM_SET_NAME, name_buffer) == 0 && | ||
| ioctl(fd, ASHMEM_SET_SIZE, size) == 0) | ||
| return fd; | ||
| close(fd); | ||
| } | ||
|
|
||
| ret = ioctl(fd, ASHMEM_SET_SIZE, size); | ||
| if (ret < 0) goto error; | ||
| // 4) dma-buf heap — Android 10+ (/dev/dma_heap/system) | ||
| { | ||
| int heap_fd = open("/dev/dma_heap/system", O_RDWR | O_CLOEXEC); | ||
| if (heap_fd >= 0) { | ||
| struct dma_heap_allocation_data { | ||
| __u64 len; | ||
| __u32 fd; | ||
| __u32 fd_flags; | ||
| __u64 heap_flags; | ||
| } data = { .len = size, .fd_flags = O_RDWR | O_CLOEXEC }; | ||
| int ret = ioctl(heap_fd, _IOWR(0x48, 0x00, struct dma_heap_allocation_data), &data); | ||
| close(heap_fd); | ||
| if (ret == 0) return (int)data.fd; | ||
| } | ||
| } | ||
|
|
||
| return fd; | ||
| error: | ||
| close(fd); | ||
| return ret; | ||
| #endif | ||
| // 5) ion — Android 4.0–9 (/dev/ion) | ||
| { | ||
| int ion_fd = open("/dev/ion", O_RDWR); | ||
| if (ion_fd >= 0) { | ||
| struct ion_allocation_data { | ||
| __u64 len; | ||
| __u32 heap_id_mask; | ||
| __u32 flags; | ||
| __u32 handle; | ||
| __u32 unused; | ||
| } alloc = { .len = size, .heap_id_mask = 1 /*ION_HEAP_SYSTEM*/ }; | ||
| if (ioctl(ion_fd, _IOWR(0x49, 0x00, struct ion_allocation_data), &alloc) == 0) { | ||
| struct ion_fd_data { | ||
| __u32 handle; | ||
| __u32 fd; | ||
| } fd_data = { .handle = alloc.handle }; | ||
| if (ioctl(ion_fd, _IOWR(0x49, 0x08, struct ion_fd_data), &fd_data) == 0) { | ||
| close(ion_fd); | ||
| return (int)fd_data.fd; | ||
| } | ||
| } | ||
| close(ion_fd); | ||
| } | ||
| } | ||
|
|
||
| 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. | ||
| // ashmem fds also succeed fstat but st_size is always 0 | ||
| // (ashmem doesn't maintain i_size, only accessible via ioctl). | ||
| struct stat st; | ||
| if (fstat(fd, &st) == 0 && st.st_size > 0) | ||
| return (int)st.st_size; | ||
|
|
||
| // Fall back to backend-specific size query | ||
| if (ASharedMemory_getSize) { | ||
| int ret = ASharedMemory_getSize(fd); | ||
| if (ret > 0) return ret; | ||
| } | ||
| return TEMP_FAILURE_RETRY(ioctl(fd, ASHMEM_GET_SIZE, NULL)); | ||
| } | ||
|
|
||
| static void ashv_check_pid() | ||
|
|
@@ -275,9 +339,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 +463,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); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
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.
__NR_memfd_create is not restricted by ANDROID_API guard. It is not needed here. Probably
should be enough.
Which means there should not be custom
__NR_memfd_createdefinition here and ndk builtin is fine.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.
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.
MFD_*defines are not needed as well.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.