Date:   Tue, 3 Oct 2017 17:12:41 -0700
From:   Laura Abbott <labbott@...hat.com>
To:     Pintu Agarwal <pintu.ping@...il.com>, linux-kernel@...r.kernel.org,
        gregkh@...uxfoundation.org
Cc:     pintu_agarwal@...oo.com
Subject: Re: [PATCHv2 1/1] [tools]: android/ion: userspace test utility for
 ion buffer sharing

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
> <ion_close_buffer_fd>: 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
> <ion_close_buffer_fd>: buffer release successfully....
> 
> Signed-off-by: Pintu Agarwal <pintu.ping@...il.com>
> ---
>  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?

> +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 <pintu.ping@...il.com>
> + *
> + * 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 <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <errno.h>
> +#include <sys/time.h>
> +#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 <help>] [-i <heap id>] [-s <size in bytes>]\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 <pintu.ping@...il.com>
> + *
> + * 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 <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <string.h>
> +#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 <stdio.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <fcntl.h>
> +#include <errno.h>
> +#include <sys/ioctl.h>
> +#include <sys/mman.h>
> +#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.

> +	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

> +		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

> +		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.


> +	map_len = ion_info->buflen;
> +	buffd = ion_info->buffd;
> +
> +	if (buffd <= 0 || map_len <= 0) {

0 is a valid file descriptor

> +		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

Powered by blists - more mailing lists