Message-ID: <CAOuPNLii9A78c6WJ97aNP94V=8ecQ6ss2nRmdZ0TkpAsjBG7Yg@mail.gmail.com>
Date:   Tue, 31 Oct 2017 15:19:13 +0530
From:   Pintu Kumar <pintu.ping@...il.com>
To:     Laura Abbott <labbott@...hat.com>
Cc:     linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        shuah@...nel.org, dvhart@...radead.org,
        Bamvor Zhang Jian <bamvor.zhangjian@...aro.org>,
        Pintu Kumar <pintu_agarwal@...oo.com>
Subject: Re: [PATCHv4 1/1] [tools/selftests]: android/ion: userspace test
 utility for ion buffer sharing

On Tue, Oct 31, 2017 at 4:26 AM, Laura Abbott <labbott@...hat.com> wrote:
> On 10/23/2017 04:26 PM, 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           |  46 ++++
>>   tools/testing/selftests/android/ion/.gitignore     |   2 +
>>   tools/testing/selftests/android/ion/Makefile       |  16 ++
>>   tools/testing/selftests/android/ion/README         | 101 ++++++++
>>   tools/testing/selftests/android/ion/config         |   4 +
>>   tools/testing/selftests/android/ion/ion_test.sh    |  61 +++++
>>   .../testing/selftests/android/ion/ionapp_export.c  | 144 ++++++++++++
>>   .../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, 1043 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..1a74922
>> --- /dev/null
>> +++ b/tools/testing/selftests/android/Makefile
>> @@ -0,0 +1,46 @@
>> +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 $@;\
>> +               #SUBDIR test prog name should be in the form:
>> SUBDIR_test.sh
>> +               TEST=$$DIR"_test.sh"; \
>> +               if [ -e $$DIR/$$TEST ]; then
>> +                       rsync -a $$DIR/$$TEST $$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..1399a5e
>> --- /dev/null
>> +++ b/tools/testing/selftests/android/ion/Makefile
>> @@ -0,0 +1,16 @@
>> +
>> +INCLUDEDIR := -I../../../../../drivers/staging/android/uapi/
>> +CFLAGS := $(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..21783e9
>> --- /dev/null
>> +++ b/tools/testing/selftests/android/ion/README
>> @@ -0,0 +1,101 @@
>> +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_STAGING=y
>> +CONFIG_ION=y
>> +CONFIG_ION_SYSTEM_HEAP=y
>> +
>> +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
>> +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
>> +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]
>> +
>> +ion_test.sh: done
>> diff --git a/tools/testing/selftests/android/ion/config
>> b/tools/testing/selftests/android/ion/config
>> new file mode 100644
>> index 0000000..19db6ca
>> --- /dev/null
>> +++ b/tools/testing/selftests/android/ion/config
>> @@ -0,0 +1,4 @@
>> +CONFIG_ANDROID=y
>> +CONFIG_STAGING=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..65bb28f
>> --- /dev/null
>> +++ b/tools/testing/selftests/android/ion/ion_test.sh
>> @@ -0,0 +1,61 @@
>> +#!/bin/bash
>> +
>> +heapsize=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..6eaea10
>> --- /dev/null
>> +++ b/tools/testing/selftests/android/ion/ionapp_export.c
>> @@ -0,0 +1,144 @@
>> +/*
>> + * 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("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 = ION_HEAP_TYPE_SYSTEM;
>> +                               break;
>> +                       case 1:
>> +                               heap_type = ION_HEAP_TYPE_SYSTEM_CONTIG;
>> +                               break;
>> +                       case 2:
>> +                               heap_type = ION_HEAP_TYPE_CARVEOUT;
>> +                               break;
>> +                       case 3:
>> +                               heap_type = ION_HEAP_TYPE_CHUNK;
>> +                               break;
>> +                       case 4:
>> +                               heap_type = ION_HEAP_TYPE_DMA;
>> +                               break;
>> +                       default:
>> +                               printf("ERROR: Wrong - heap type\n");
>> +                               exit(1);
>
>
> Again, why are we still supporting multiple heaps. You need to
> remove all references to heaps other than the system heap
> everywhere in the test.
>

Ok, I just kept this for reference, in case somebody wants to use it.
Ok, I will clean up everything related to heap type and re-submit
again by tomorrow.

Thanks,
Pintu

> Thanks,
> Laura
>
>
>> +                       }
>> +                       break;
>> +               case 's':
>> +                       heap_size = atoi(optarg);
>> +                       break;
>> +               default:
>> +                       print_usage(argc, argv);
>> +                       exit(1);
>> +                       break;
>> +               }
>> +       }
>
>
>

Powered by blists - more mailing lists