Message-ID: <9fb9b4ee-2834-511b-ff78-d14ea5609728@kernel.org>
Date:   Tue, 17 Oct 2017 14:58:32 -0600
From:   Shuah Khan <shuah@...nel.org>
To:     Laura Abbott <labbott@...hat.com>,
        Pintu Agarwal <pintu.ping@...il.com>,
        linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org,
        gregkh@...uxfoundation.org, dvhart@...radead.org,
        bamvor.zhangjian@...aro.org, Shuah Khan <shuah@...nel.org>,
        Shuah Khan <shuahkh@....samsung.com>
Subject: Re: [PATCHv3 1/1] [tools/selftests]: android/ion: userspace test
 utility for ion buffer sharing

On 10/17/2017 02:21 PM, Laura Abbott wrote:
> On 10/14/2017 04:36 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 (with SCM_RIGHTS) as IPC to transfer an FD to
>> another process to share the same buffer.
>> This utility demonstrates how ION buffer sharing can be implemented between
>> two user space processes, using various heap types.
>>
>> This utility is made to be run as part of kselftest framework in kernel.
>> The utility is verified on Ubuntu-32 bit system with Linux Kernel 4.14,
>> using ION system heap and CMA heap.
>>
>> For more information about the utility please check the README file.
>>
>> Signed-off-by: Pintu Agarwal <pintu.ping@...il.com>
>> ---
>>  tools/testing/selftests/Makefile                   |   3 +-
>>  tools/testing/selftests/android/Makefile           |  44 ++++
>>  tools/testing/selftests/android/ion/.gitignore     |   2 +
>>  tools/testing/selftests/android/ion/Makefile       |  16 ++
>>  tools/testing/selftests/android/ion/README         | 132 +++++++++++
>>  tools/testing/selftests/android/ion/config         |   3 +
>>  tools/testing/selftests/android/ion/ion_test.sh    |  61 +++++
>>  .../testing/selftests/android/ion/ionapp_export.c  | 151 ++++++++++++
>>  .../testing/selftests/android/ion/ionapp_import.c  |  88 +++++++
>>  tools/testing/selftests/android/ion/ionutils.c     | 259 +++++++++++++++++++++
>>  tools/testing/selftests/android/ion/ionutils.h     |  55 +++++
>>  tools/testing/selftests/android/ion/ipcsocket.c    | 227 ++++++++++++++++++
>>  tools/testing/selftests/android/ion/ipcsocket.h    |  35 +++
>>  tools/testing/selftests/android/run.sh             |   3 +
>>  14 files changed, 1078 insertions(+), 1 deletion(-)
>>  create mode 100644 tools/testing/selftests/android/Makefile
>>  create mode 100644 tools/testing/selftests/android/ion/.gitignore
>>  create mode 100644 tools/testing/selftests/android/ion/Makefile
>>  create mode 100644 tools/testing/selftests/android/ion/README
>>  create mode 100644 tools/testing/selftests/android/ion/config
>>  create mode 100755 tools/testing/selftests/android/ion/ion_test.sh
>>  create mode 100644 tools/testing/selftests/android/ion/ionapp_export.c
>>  create mode 100644 tools/testing/selftests/android/ion/ionapp_import.c
>>  create mode 100644 tools/testing/selftests/android/ion/ionutils.c
>>  create mode 100644 tools/testing/selftests/android/ion/ionutils.h
>>  create mode 100644 tools/testing/selftests/android/ion/ipcsocket.c
>>  create mode 100644 tools/testing/selftests/android/ion/ipcsocket.h
>>  create mode 100755 tools/testing/selftests/android/run.sh
>>
>> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
>> index ff80564..61bc77b 100644
>> --- a/tools/testing/selftests/Makefile
>> +++ b/tools/testing/selftests/Makefile
>> @@ -1,4 +1,5 @@
>> -TARGETS =  bpf
>> +TARGETS = android
>> +TARGETS += bpf
>>  TARGETS += breakpoints
>>  TARGETS += capabilities
>>  TARGETS += cpufreq
>> diff --git a/tools/testing/selftests/android/Makefile b/tools/testing/selftests/android/Makefile
>> new file mode 100644
>> index 0000000..ee76446
>> --- /dev/null
>> +++ b/tools/testing/selftests/android/Makefile
>> @@ -0,0 +1,44 @@
>> +SUBDIRS := ion
>> +
>> +TEST_PROGS := run.sh
>> +
>> +.PHONY: all clean
>> +
>> +include ../lib.mk
>> +
>> +all:
>> +	@for DIR in $(SUBDIRS); do		\
>> +		BUILD_TARGET=$(OUTPUT)/$$DIR;	\
>> +		mkdir $$BUILD_TARGET  -p;	\
>> +		make OUTPUT=$$BUILD_TARGET -C $$DIR $@;\
>> +		if [ -e $$DIR/$(TEST_PROGS) ]; then
>> +			rsync -a $$DIR/$(TEST_PROGS) $$BUILD_TARGET/;
>> +		fi
>> +	done
>> +
>> +override define RUN_TESTS
>> +	@cd $(OUTPUT); ./run.sh
>> +endef
>> +
>> +override define INSTALL_RULE
>> +	mkdir -p $(INSTALL_PATH)
>> +	install -t $(INSTALL_PATH) $(TEST_PROGS) $(TEST_PROGS_EXTENDED) $(TEST_FILES)
>> +
>> +	@for SUBDIR in $(SUBDIRS); do \
>> +		BUILD_TARGET=$(OUTPUT)/$$SUBDIR;	\
>> +		mkdir $$BUILD_TARGET  -p;	\
>> +		$(MAKE) OUTPUT=$$BUILD_TARGET -C $$SUBDIR INSTALL_PATH=$(INSTALL_PATH)/$$SUBDIR install; \
>> +	done;
>> +endef
>> +
>> +override define EMIT_TESTS
>> +	echo "./run.sh"
>> +endef
>> +
>> +override define CLEAN
>> +	@for DIR in $(SUBDIRS); do		\
>> +		BUILD_TARGET=$(OUTPUT)/$$DIR;	\
>> +		mkdir $$BUILD_TARGET  -p;	\
>> +		make OUTPUT=$$BUILD_TARGET -C $$DIR $@;\
>> +	done
>> +endef
>> diff --git a/tools/testing/selftests/android/ion/.gitignore b/tools/testing/selftests/android/ion/.gitignore
>> new file mode 100644
>> index 0000000..67e6f39
>> --- /dev/null
>> +++ b/tools/testing/selftests/android/ion/.gitignore
>> @@ -0,0 +1,2 @@
>> +ionapp_export
>> +ionapp_import
>> diff --git a/tools/testing/selftests/android/ion/Makefile b/tools/testing/selftests/android/ion/Makefile
>> new file mode 100644
>> index 0000000..b84e3b1
>> --- /dev/null
>> +++ b/tools/testing/selftests/android/ion/Makefile
>> @@ -0,0 +1,16 @@
>> +
>> +INCLUDEDIR := -I../../../../../drivers/staging/android/uapi/
>> +CFLAGS := $(INCLUDEDIR) -Wall -O2 -g
>> +
>> +TEST_GEN_FILES := ionapp_export ionapp_import
>> +
>> +all: $(TEST_GEN_FILES)
>> +
>> +$(TEST_GEN_FILES): ipcsocket.c ionutils.c
>> +
>> +TEST_PROGS := ion_test.sh
>> +
>> +include ../../lib.mk
>> +
>> +$(OUTPUT)/ionapp_export: ionapp_export.c ipcsocket.c ionutils.c
>> +$(OUTPUT)/ionapp_import: ionapp_import.c ipcsocket.c ionutils.c
>> diff --git a/tools/testing/selftests/android/ion/README b/tools/testing/selftests/android/ion/README
>> new file mode 100644
>> index 0000000..163e353
>> --- /dev/null
>> +++ b/tools/testing/selftests/android/ion/README
>> @@ -0,0 +1,132 @@
>> +ION BUFFER SHARING UTILITY
>> +==========================
>> +File: ion_test.sh : Utility to test ION driver buffer sharing mechanism.
>> +Author: Pintu Kumar <pintu.ping@...il.com>
>> +
>> +Introduction:
>> +-------------
>> +This is a test utility to verify ION buffer sharing in user space
>> +between 2 independent processes.
>> +It uses unix domain socket (with SCM_RIGHTS) as IPC to transfer an FD to
>> +another process to share the same buffer.
>> +This utility demonstrates how ION buffer sharing can be implemented between
>> +two user space processes, using various heap types.
>> +The following heap types are supported by ION driver.
>> +ION_HEAP_TYPE_SYSTEM (0)
>> +ION_HEAP_TYPE_SYSTEM_CONTIG (1)
>> +ION_HEAP_TYPE_CARVEOUT (2)
>> +ION_HEAP_TYPE_CHUNK (3)
>> +ION_HEAP_TYPE_DMA (4)
>> +
>> +By default only the SYSTEM and SYSTEM_CONTIG heaps are supported.
>> +Each heap is associated with the respective heap id.
>> +This utility is designed in the form of client/server program.
>> +The server part (ionapp_export) is the exporter of the buffer.
>> +It is responsible for creating an ION client, allocating the buffer based on
>> +the heap id, writing some data to this buffer and then exporting the FD
>> +(associated with this buffer) to another process using socket IPC.
>> +This FD is called as buffer FD (which is different than the ION client FD).
>> +
>> +The client part (ionapp_import) is the importer of the buffer.
>> +It retrives the FD from the socket data and installs into its address space.
>> +This new FD internally points to the same kernel buffer.
>> +So first it reads the data that is stored in this buffer and prints it.
>> +Then it writes the different size of data (it could be different data) to the
>> +same buffer.
>> +Finally the buffer FD must be closed by both the exporter and importer.
>> +Thus the same kernel buffer is shared among two user space processes using
>> +ION driver and only one time allocation.
>> +
>> +Prerequisite:
>> +-------------
>> +This utility works only if /dev/ion interface is present.
>> +The following configs needs to be enabled in kernel to include ion driver.
>> +CONFIG_ANDROID=y
>> +CONFIG_ION=y
>> +CONFIG_ION_SYSTEM_HEAP=y
> 
> You also need CONFIG_STAGING right now as well.

In which case, please make sure the test fails gracefully when the
these config options are disabled.

What does the test do when all of these options are disabled?

> 
>> +
>> +This utility requires to be run as root user.
>> +
>> +
>> +Compile and test:
>> +-----------------
>> +This utility is made to be run as part of kselftest framework in kernel.
>> +To compile and run using kselftest you can simply do the following from the
>> +kernel top directory.
>> +linux$ make TARGETS=android kselftest

Please make sure

make O=/tmp/kselftest TARGETS=android kselftest

works.

>> +Or you can also use:
>> +linux$ make -C tools/testing/selftests TARGETS=android run_tests
>> +Using the selftest it can directly execute the ion_test.sh script to test the
>> +buffer sharing using ion system heap.
>> +Currently the heap size is hard coded as just 10 bytes inside this script.
>> +You need to be a root user to run under selftest.
>> +
>> +You can also compile and test manually using the following steps:
>> +ion$ make
>> +These will generate 2 executable: ionapp_export, ionapp_import
>> +Now you can run the export and import manually by specifying the heap type
>> +and the heap size.
>> +You can also directly execute the shell script to run the test automatically.
>> +Simply use the following command to run the test.
>> +ion$ sudo ./ion_test.sh
>> +
>> +Test Results:
>> +-------------
>> +The utility is verified on Ubuntu-32 bit system with Linux Kernel 4.14.
>> +Here is the snapshot of the test result using kselftest.
>> +
>> +linux# make TARGETS=android kselftest
>> +make[2]: Entering directory '/home/pintu/PINTU/OPEN_SOURCE/KERNEL/

Please don't include directory information in the README.

>> +linux/tools/testing/selftests/android'
>> +make[3]: Entering directory '/home/pintu/PINTU/OPEN_SOURCE/KERNEL/
>> +linux/tools/testing/selftests/android/ion'
>> +gcc -I../../../../../drivers/staging/android/uapi/ -Wall -O2 -g

Please don't include gcc output information in the README.

>> +ionapp_export.c ipcsocket.c ionutils.c   -o ionapp_export
>> +gcc -I../../../../../drivers/staging/android/uapi/ -Wall -O2 -g
>> +ionapp_import.c ipcsocket.c ionutils.c   -o ionapp_import
>> +

Include just the test results. Sorry I didn't catch this before.

>> +heap_type: 0, heap_size: 10
>> +--------------------------------------
>> +heap type: 0
>> +  heap id: 1
>> +heap name: ion_system_heap
>> +--------------------------------------
>> +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....
>> +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....
>> +ion_test.sh: heap_type: 0 - [PASS]
>> +
>> +heap_type: 1, heap_size: 10
>> +--------------------------------------
>> +heap type: 1
>> +  heap id: 0
>> +heap name: ion_system_contig_heap
>> +--------------------------------------
>> +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....
>> +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....
>> +ion_test.sh: heap_type: 1 - [PASS]
>> +
>> +ion_test.sh: done
>> +make[2]: Leaving directory '/home/pintu/PINTU/OPEN_SOURCE/KERNEL/
>> +linux/tools/testing/selftests/android'
>> diff --git a/tools/testing/selftests/android/ion/config b/tools/testing/selftests/android/ion/config
>> new file mode 100644
>> index 0000000..614b615
>> --- /dev/null
>> +++ b/tools/testing/selftests/android/ion/config
>> @@ -0,0 +1,3 @@
>> +CONFIG_ANDROID=y
>> +CONFIG_ION=y
>> +CONFIG_ION_SYSTEM_HEAP=y
>> diff --git a/tools/testing/selftests/android/ion/ion_test.sh b/tools/testing/selftests/android/ion/ion_test.sh
>> new file mode 100755
>> index 0000000..dc77a39
>> --- /dev/null
>> +++ b/tools/testing/selftests/android/ion/ion_test.sh
>> @@ -0,0 +1,61 @@
>> +#!/bin/bash
>> +
>> +heapsize=10
> 
> Nearly all the heaps are going to operate on page size granularity,
> just set this to 4096.
> 
>> +TCID="ion_test.sh"
>> +errcode=0
>> +
>> +run_test()
>> +{
>> +	heaptype=$1
>> +	./ionapp_export -i $heaptype -s $heapsize &
>> +	sleep 1
>> +	./ionapp_import
>> +	if [ $? -ne 0 ]; then
>> +		echo "$TCID: heap_type: $heaptype - [FAIL]"
>> +		errcode=1
>> +	else
>> +		echo "$TCID: heap_type: $heaptype - [PASS]"
>> +	fi
>> +	sleep 1
>> +	echo ""
>> +}
>> +
>> +check_root()
>> +{
>> +	uid=$(id -u)
>> +	if [ $uid -ne 0 ]; then
>> +		echo $TCID: must be run as root >&2
>> +		exit 0
>> +	fi
>> +}
>> +
>> +check_device()
>> +{
>> +	DEVICE=/dev/ion
>> +	if [ ! -e $DEVICE ]; then
>> +		echo $TCID: No $DEVICE device found >&2
>> +		echo $TCID: May be CONFIG_ION is not set >&2
>> +		exit 0
>> +	fi
>> +}
>> +
>> +main_function()
>> +{
>> +	check_device
>> +	check_root
>> +
>> +	# ION_SYSTEM_HEAP TEST
>> +	run_test 0
>> +	# ION_SYSTEM_CONTIG_HEAP TEST
>> +	run_test 1
>> +	# ION_CARVEOUT HEAP TEST
>> +	#run_test 2
>> +	# ION_CHUNK_HEAP TEST
>> +	#run_test 3
>> +	# ION_CMA_HEAP TEST
>> +	#run_test 4
>> +}
>> +
>> +main_function
>> +echo "$TCID: done"
>> +exit $errcode
>> diff --git a/tools/testing/selftests/android/ion/ionapp_export.c b/tools/testing/selftests/android/ion/ionapp_export.c
>> new file mode 100644
>> index 0000000..59b434f
>> --- /dev/null
>> +++ b/tools/testing/selftests/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 TYPE ***********\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]);
>> +}> +
> 
> Again, there is no need to test multiple heaps. Just
> do the test with system heap and remove all the
> other ones.
> 
> Thanks,
> Laura
> 
> 

thanks,
-- Shuah

Powered by blists - more mailing lists