From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752205AbdJDL3X (ORCPT ); Wed, 4 Oct 2017 07:29:23 -0400 Received: from mail-lf0-f66.google.com ([209.85.215.66]:33337 "EHLO mail-lf0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751943AbdJDL3T (ORCPT ); Wed, 4 Oct 2017 07:29:19 -0400 X-Google-Smtp-Source: AOwi7QCxaoYnOUPLQ0cpTvsFl9rAdF9A9LZ+nHkQ3GxoovqE6ueppEr00mk/nq/Mstx3HwL7Feb5Bxyi8Lg4mX2XWOU= MIME-Version: 1.0 In-Reply-To: <414b44ec-6500-c3e5-f8ca-ce1d21a3eb58@redhat.com> References: <1506449281-8790-1-git-send-email-pintu.ping@gmail.com> <1507049339-16963-1-git-send-email-pintu.ping@gmail.com> <414b44ec-6500-c3e5-f8ca-ce1d21a3eb58@redhat.com> From: Pintu Kumar Date: Wed, 4 Oct 2017 16:59:17 +0530 Message-ID: Subject: Re: [PATCHv2 1/1] [tools]: android/ion: userspace test utility for ion buffer sharing To: Laura Abbott Cc: linux-kernel@vger.kernel.org, Greg Kroah-Hartman , Pintu Kumar Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Oct 4, 2017 at 5:42 AM, Laura Abbott wrote: > On 10/03/2017 09:48 AM, Pintu Agarwal wrote: >> This is a test utility to verify ION buffer sharing in user space >> between 2 independent processes. >> It uses unix domain socket as IPC to transfer an FD to another process >> and install it. >> >> This utility demonstrates how ION buffer sharing can be implemented between >> two user space processes, using various heap ids. >> >> This utility is verified on Ubuntu 32-bit machine using 2 independent >> process such as: ionapp_export (server) and ionapp_import (client). >> First the server needs to be run to export FD to the client. >> This utility works only if /dev/ion interface is present. >> >> Here is a sample demo example: >> >> linux/tools/android/ion$ sudo ./ionapp_export.out -i 1 -s 10 >> heap_type: 2, heap_size: 10 >> Fill buffer content: >> 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd >> Sharing fd: 6, Client fd: 5 >> : buffer release successfully.... >> >> linux/tools/android/ion$ sudo ./ionapp_import.out >> Received buffer fd: 4 >> Read buffer content: >> 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd >> 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 >> 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 >> Fill buffer content: >> 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd >> 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd >> 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd 0xfd >> : buffer release successfully.... >> >> Signed-off-by: Pintu Agarwal >> --- >> tools/android/ion/Makefile | 17 +++ >> tools/android/ion/ionapp_export.c | 151 ++++++++++++++++++++++++ >> tools/android/ion/ionapp_import.c | 86 ++++++++++++++ >> tools/android/ion/ionutils.c | 239 ++++++++++++++++++++++++++++++++++++++ >> tools/android/ion/ionutils.h | 54 +++++++++ >> tools/android/ion/ipcsocket.c | 227 ++++++++++++++++++++++++++++++++++++ >> tools/android/ion/ipcsocket.h | 35 ++++++ >> 7 files changed, 809 insertions(+) >> create mode 100644 tools/android/ion/Makefile >> create mode 100644 tools/android/ion/ionapp_export.c >> create mode 100644 tools/android/ion/ionapp_import.c >> create mode 100644 tools/android/ion/ionutils.c >> create mode 100644 tools/android/ion/ionutils.h >> create mode 100644 tools/android/ion/ipcsocket.c >> create mode 100644 tools/android/ion/ipcsocket.h >> >> diff --git a/tools/android/ion/Makefile b/tools/android/ion/Makefile >> new file mode 100644 >> index 0000000..57d2e98 >> --- /dev/null >> +++ b/tools/android/ion/Makefile >> @@ -0,0 +1,17 @@ >> + >> +CC := $(CROSS_COMPILE)gcc >> + >> +INCLUDEDIR := -I../../../drivers/staging/android/uapi/ >> + >> +CCFLAGS := $(INCLUDEDIR) -Wall -O2 -g >> + >> +all: ionapp_export ionapp_import >> + >> +ionapp_import : ionapp_import.c >> + $(CC) -o ionapp_import.out ionapp_import.c ipcsocket.c ionutils.c $(CCFLAGS) >> + >> +ionapp_export : ionapp_export.c >> + $(CC) -o ionapp_export.out ionapp_export.c ipcsocket.c ionutils.c $(CCFLAGS) >> + > > Having to run the two apps separately is a pain, > can you combine the two into one application and use fork? > The whole thing about this test is to share an FD over 2 independent processes. I think sharing an FD using fork() [parent/child] is not a real use case scenarios. Some people may not like the fork example. Initially when I started with ION, I also needed an FD sharing mechanism between 2 different process. Thus I came up with this framework using ipcsocket. Later, if required, we can even replace this with binder_ipc for android use cases. Anyways, binder_ipc also internally uses the same concept as this ipcsocket. To reduce the pain, we can invoke both the tests from a single shell scripts. I will try to include the same in kselftests, if possible. If fork example is really required, we can add another test for it. This is my opinion. >> +clean: >> + rm -rf *.o *~ *.out >> diff --git a/tools/android/ion/ionapp_export.c b/tools/android/ion/ionapp_export.c >> new file mode 100644 >> index 0000000..74b369b >> --- /dev/null >> +++ b/tools/android/ion/ionapp_export.c >> @@ -0,0 +1,151 @@ >> +/* >> + * ionapp_export.c >> + * >> + * It is a user space utility to create and export android >> + * ion memory buffer fd to another process using unix domain socket as IPC. >> + * This acts like a server for ionapp_import(client). >> + * So, this server has to be started first before the client. >> + * >> + * Copyright (C) 2017 Pintu Kumar >> + * >> + * This software is licensed under the terms of the GNU General Public >> + * License version 2, as published by the Free Software Foundation, and >> + * may be copied, distributed, and modified under those terms. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + */ >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include "ionutils.h" >> +#include "ipcsocket.h" >> + >> + >> +void print_usage(int argc, char *argv[]) >> +{ >> + printf("********** HEAP ID ***********\n"); >> + printf("0: ION_HEAP_TYPE_SYSTEM\n"); >> + printf("1: ION_HEAP_TYPE_SYSTEM_CONTIG\n"); >> + printf("2: ION_HEAP_TYPE_CARVEOUT\n"); >> + printf("3: ION_HEAP_TYPE_CHUNK\n"); >> + printf("4: ION_HEAP_TYPE_DMA\n"); >> + >> + printf("Usage: %s [-h ] [-i ] [-s ]\n", >> + argv[0]); >> +} >> + >> +int main(int argc, char *argv[]) >> +{ >> + int opt, ret, status, heapid; >> + int sockfd, client_fd, shared_fd; >> + unsigned char *map_buf; >> + unsigned long map_len, heap_type, heap_size, flags; >> + struct ion_buffer_info info; >> + struct socket_info skinfo; >> + >> + if (argc < 2) { >> + print_usage(argc, argv); >> + return -1; >> + } >> + >> + heap_size = 0; >> + flags = 0; >> + >> + while ((opt = getopt(argc, argv, "hi:s:")) != -1) { >> + switch (opt) { >> + case 'h': >> + print_usage(argc, argv); >> + exit(0); >> + break; >> + case 'i': >> + heapid = atoi(optarg); >> + switch (heapid) { >> + case 0: >> + heap_type = 1 << ION_HEAP_TYPE_SYSTEM; >> + break; >> + case 1: >> + heap_type = 1 << ION_HEAP_TYPE_SYSTEM_CONTIG; >> + break; >> + case 2: >> + heap_type = 1 << ION_HEAP_TYPE_CARVEOUT; >> + break; >> + case 3: >> + heap_type = 1 << ION_HEAP_TYPE_CHUNK; >> + break; >> + case 4: >> + heap_type = 1 << ION_HEAP_TYPE_DMA; >> + break; >> + default: >> + printf("ERROR: Wrong - heap type\n"); >> + exit(1); >> + } >> + break; >> + case 's': >> + heap_size = atoi(optarg); >> + break; >> + default: >> + print_usage(argc, argv); >> + exit(1); >> + break; >> + } >> + } >> + >> + if (heap_size <= 0) { >> + printf("heap_size cannot be 0\n"); >> + print_usage(argc, argv); >> + exit(1); >> + } >> + >> + printf("heap_type: %ld, heap_size: %ld\n", heap_type, heap_size); >> + info.heap_type = heap_type; >> + info.heap_size = heap_size; >> + info.flag_type = flags; >> + >> + /* This is server: open the socket connection first */ >> + /* Here; 1 indicates server or exporter */ >> + status = opensocket(&sockfd, SOCKET_NAME, 1); >> + if (status < 0) { >> + fprintf(stderr, "<%s>: Failed opensocket.\n", __func__); >> + goto err_socket; >> + } >> + skinfo.sockfd = sockfd; >> + >> + ret = ion_export_buffer_fd(&info); >> + if (ret < 0) { >> + fprintf(stderr, "FAILED: ion_get_buffer_fd\n"); >> + goto err_export; >> + } >> + client_fd = info.ionfd; >> + shared_fd = info.buffd; >> + map_buf = info.buffer; >> + map_len = info.buflen; >> + write_buffer(map_buf, map_len); >> + >> + /* share ion buf fd with other user process */ >> + printf("Sharing fd: %d, Client fd: %d\n", shared_fd, client_fd); >> + skinfo.datafd = shared_fd; >> + skinfo.buflen = map_len; >> + >> + ret = socket_send_fd(&skinfo); >> + if (ret < 0) { >> + fprintf(stderr, "FAILED: socket_send_fd\n"); >> + goto err_send; >> + } >> + >> +err_send: >> +err_export: >> + ion_close_buffer_fd(&info); >> + >> +err_socket: >> + closesocket(sockfd, SOCKET_NAME); >> + >> + return 0; >> +} >> diff --git a/tools/android/ion/ionapp_import.c b/tools/android/ion/ionapp_import.c >> new file mode 100644 >> index 0000000..4d8646d >> --- /dev/null >> +++ b/tools/android/ion/ionapp_import.c >> @@ -0,0 +1,86 @@ >> +/* >> + * ionapp_import.c >> + * >> + * It is a user space utility to receive android ion memory buffer fd >> + * over unix domain socket IPC that can be exported by ionapp_export. >> + * This acts like a client for ionapp_export. >> + * >> + * Copyright (C) 2017 Pintu Kumar >> + * >> + * This software is licensed under the terms of the GNU General Public >> + * License version 2, as published by the Free Software Foundation, and >> + * may be copied, distributed, and modified under those terms. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + */ >> + >> +#include >> +#include >> +#include >> +#include >> +#include "ionutils.h" >> +#include "ipcsocket.h" >> + >> + >> +int main(void) >> +{ >> + int ret, status; >> + int sockfd, shared_fd; >> + unsigned char *map_buf; >> + unsigned long map_len; >> + struct ion_buffer_info info; >> + struct socket_info skinfo; >> + >> + /* This is the client part. Here 0 means client or importer */ >> + status = opensocket(&sockfd, SOCKET_NAME, 0); >> + if (status < 0) { >> + fprintf(stderr, "No exporter exists...\n"); >> + goto err_socket; >> + } >> + >> + skinfo.sockfd = sockfd; >> + >> + ret = socket_receive_fd(&skinfo); >> + if (ret < 0) { >> + fprintf(stderr, "Failed: socket_receive_fd\n"); >> + goto err_recv; >> + } >> + >> + shared_fd = skinfo.datafd; >> + printf("Received buffer fd: %d\n", shared_fd); >> + if (shared_fd <= 0) { >> + fprintf(stderr, "ERROR: improper buf fd\n"); >> + goto err_fd; >> + } >> + >> + memset(&info, 0, sizeof(info)); >> + info.buffd = shared_fd; >> + info.buflen = ION_BUFFER_LEN; >> + >> + ret = ion_import_buffer_fd(&info); >> + if (ret < 0) { >> + fprintf(stderr, "Failed: ion_use_buffer_fd\n"); >> + goto err_import; >> + } >> + >> + map_buf = info.buffer; >> + map_len = info.buflen; >> + read_buffer(map_buf, map_len); >> + >> + /* Write probably new data to the same buffer again */ >> + map_len = ION_BUFFER_LEN; >> + write_buffer(map_buf, map_len); >> + >> +err_import: >> + ion_close_buffer_fd(&info); >> +err_fd: >> +err_recv: >> +err_socket: >> + closesocket(sockfd, SOCKET_NAME); >> + >> + return 0; >> +} >> diff --git a/tools/android/ion/ionutils.c b/tools/android/ion/ionutils.c >> new file mode 100644 >> index 0000000..2d9012b >> --- /dev/null >> +++ b/tools/android/ion/ionutils.c >> @@ -0,0 +1,239 @@ >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include "ionutils.h" >> +#include "ipcsocket.h" >> + >> + >> +void write_buffer(void *buffer, unsigned long len) >> +{ >> + int i; >> + unsigned char *ptr = (unsigned char *)buffer; >> + >> + if (!ptr) { >> + fprintf(stderr, "<%s>: Invalid buffer...\n", __func__); >> + return; >> + } >> + >> + printf("Fill buffer content:\n"); >> + memset(ptr, 0xfd, len); >> + for (i = 0; i < len; i++) >> + printf("0x%x ", ptr[i]); >> + printf("\n"); >> +} >> + >> +void read_buffer(void *buffer, unsigned long len) >> +{ >> + int i; >> + unsigned char *ptr = (unsigned char *)buffer; >> + >> + if (!ptr) { >> + fprintf(stderr, "<%s>: Invalid buffer...\n", __func__); >> + return; >> + } >> + >> + printf("Read buffer content:\n"); >> + for (i = 0; i < len; i++) >> + printf("0x%x ", ptr[i]); >> + printf("\n"); >> +} >> + >> +int ion_export_buffer_fd(struct ion_buffer_info *ion_info) >> +{ >> + int ret, ionfd, buffer_fd; >> + unsigned int heap_type, flag_type; >> + unsigned long heap_size, maplen; >> + unsigned char *map_buffer; >> + struct ion_allocation_data alloc_data; >> + >> + if (!ion_info) { >> + fprintf(stderr, "<%s>: Invalid ion info\n", __func__); >> + return -1; >> + } >> + >> + /* Create an ION client */ >> + ionfd = open(ION_DEVICE, O_RDWR); >> + if (ionfd < 0) { >> + fprintf(stderr, "<%s>: Failed to open ion client: %s\n", >> + __func__, strerror(errno)); >> + return -1; >> + } >> + >> + heap_type = ion_info->heap_type; >> + heap_size = ion_info->heap_size; >> + flag_type = ion_info->flag_type; >> + alloc_data.len = heap_size; >> + alloc_data.heap_id_mask = heap_type; > > The heap type and heap ID are not the same thing. You need > to determine the heap id from using the new query ioctl. > Ok I will check how query ioctl can be used here. Thanks >> + alloc_data.flags = flag_type; >> + >> + /* Allocate memory for this ION client as per heap_type */ >> + ret = ioctl(ionfd, ION_IOC_ALLOC, &alloc_data); >> + if (ret < 0) { >> + fprintf(stderr, "<%s>: Failed: ION_IOC_ALLOC: %s\n", >> + __func__, strerror(errno)); >> + goto err_alloc; >> + } >> + >> + /* This will return a valid buffer fd */ >> + buffer_fd = alloc_data.fd; >> + maplen = alloc_data.len; >> + >> + if (buffer_fd <= 0 || maplen <= 0) { > > 0 is a valid file descriptor > Ok I will fix it. Thanks >> + fprintf(stderr, "<%s>: Invalid map data, fd: %d, len: %ld\n", >> + __func__, buffer_fd, maplen); >> + goto err_fd_data; >> + } >> + >> + /* Create memory mapped buffer for the buffer fd */ >> + map_buffer = (unsigned char *)mmap(NULL, maplen, PROT_READ|PROT_WRITE, >> + MAP_SHARED, buffer_fd, 0); >> + if (ion_info->buffer == MAP_FAILED) { > > This should be checking map_buffer > Ok I will fix it. Thanks >> + fprintf(stderr, "<%s>: Failed: mmap: %s\n", >> + __func__, strerror(errno)); >> + goto err_mmap; >> + } >> + >> + ion_info->ionfd = ionfd; >> + ion_info->buffd = buffer_fd; >> + ion_info->buffer = map_buffer; >> + ion_info->buflen = maplen; >> + >> + return 0; >> + >> + munmap(map_buffer, maplen); >> + >> +err_fd_data: >> +err_mmap: >> + /* in case of error: close the buffer fd */ >> + if (buffer_fd > 0) >> + close(buffer_fd); >> + >> +err_alloc: >> + /* In case of error: close the ion client fd */ >> + if (ionfd > 0) >> + close(ionfd); >> + >> + return -1; >> +} >> + >> +int ion_import_buffer_fd(struct ion_buffer_info *ion_info) >> +{ >> + int ionfd, buffd; >> + unsigned char *map_buf; >> + unsigned long map_len; >> + >> + if (!ion_info) { >> + fprintf(stderr, "<%s>: Invalid ion info\n", __func__); >> + return -1; >> + } >> + >> + ionfd = open(ION_DEVICE, O_RDWR); >> + if (ionfd < 0) { >> + fprintf(stderr, "<%s>: Failed to open ion client: %s\n", >> + __func__, strerror(errno)); >> + return -1; >> + } >> + > > There's no need to open the ion device here since you can just use > the dma_buf directly. > Ok got it. Thanks > >> + map_len = ion_info->buflen; >> + buffd = ion_info->buffd; >> + >> + if (buffd <= 0 || map_len <= 0) { > > 0 is a valid file descriptor > Ok I will fix it. Thanks >> + fprintf(stderr, "<%s>: Invalid map data, fd: %d, len: %ld\n", >> + __func__, buffd, map_len); >> + goto err_fd_data; >> + } >> + >> + map_buf = (unsigned char *)mmap(NULL, map_len, PROT_READ|PROT_WRITE, >> + MAP_SHARED, buffd, 0); >> + if (map_buf == MAP_FAILED) { >> + printf("<%s>: Failed - mmap: %s\n", >> + __func__, strerror(errno)); >> + goto err_mmap; >> + } >> + >> + ion_info->ionfd = ionfd; >> + ion_info->buffd = buffd; >> + ion_info->buffer = map_buf; >> + ion_info->buflen = map_len; >> + >> + return 0; >> + >> +err_mmap: >> + if (buffd) >> + close(buffd); >> + >> +err_fd_data: >> + if (ionfd) >> + close(ionfd); >> + >> + return -1; >> +} >> + >> +void ion_close_buffer_fd(struct ion_buffer_info *ion_info) >> +{ >> + if (ion_info) { >> + /* unmap the buffer properly in the end */ >> + munmap(ion_info->buffer, ion_info->buflen); >> + /* close the buffer fd */ >> + if (ion_info->buffd > 0) >> + close(ion_info->buffd); >> + /* Finally, close the client fd */ >> + if (ion_info->ionfd > 0) >> + close(ion_info->ionfd); >> + printf("<%s>: buffer release successfully....\n", __func__); >> + } >> +} >> + >> +int socket_send_fd(struct socket_info *info) >> +{ >> + int status; >> + int fd, sockfd; >> + struct socketdata skdata; >> + >> + if (!info) { >> + fprintf(stderr, "<%s>: Invalid socket info\n", __func__); >> + return -1; >> + } >> + >> + sockfd = info->sockfd; >> + fd = info->datafd; >> + memset(&skdata, 0, sizeof(skdata)); >> + skdata.data = fd; >> + skdata.len = sizeof(skdata.data); >> + status = sendtosocket(sockfd, &skdata); >> + if (status < 0) { >> + fprintf(stderr, "<%s>: Failed: sendtosocket\n", __func__); >> + return -1; >> + } >> + >> + return 0; >> +} >> + >> +int socket_receive_fd(struct socket_info *info) >> +{ >> + int status; >> + int fd, sockfd; >> + struct socketdata skdata; >> + >> + if (!info) { >> + fprintf(stderr, "<%s>: Invalid socket info\n", __func__); >> + return -1; >> + } >> + >> + sockfd = info->sockfd; >> + memset(&skdata, 0, sizeof(skdata)); >> + status = receivefromsocket(sockfd, &skdata); >> + if (status < 0) { >> + fprintf(stderr, "<%s>: Failed: receivefromsocket\n", __func__); >> + return -1; >> + } >> + >> + fd = (int)skdata.data; >> + info->datafd = fd; >> + >> + return status; >> +} > > Thanks, > Laura