Re: [PATCHv3 1/1] [tools/selftests]: android/ion: userspace test utility for ion buffer sharing

[Pintu Kumar]

On Fri, Oct 20, 2017 at 8:10 PM, Shuah Khan <shuahkh@xxxxxxxxxxxxxxx> wrote:
> On 10/18/2017 04:38 AM, Pintu Kumar wrote:
>> On Wed, Oct 18, 2017 at 2:28 AM, Shuah Khan <shuah@xxxxxxxxxx> wrote:
>>> 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@xxxxxxxxx>
>>>>> ---
>>>>>  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@xxxxxxxxx>
>>>>> +
>>>>> +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.
>>>
>>
>> Ok, added CONFIG_STAGING under config and README
>>
>>> 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?
>>>
>> I assume that if these configs are not present the /dev/ion will also
>> not exists.
>> If that is the case then, I don't proceed with the test.
>> This is checked under ion_test script using: check_device
>>
>>>>
>>>>> +
>>>>> +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.
>>>
>>
>> Ok when I specify O as output directory. It did not work.
>> ./run.sh: 3: ./run.sh: ./ion_test.sh: not found
>> ../lib.mk:41: recipe for target 'run_tests' failed
>>
>> When I checked /tmp/kselftest/ion/, I see that ion_test.sh is not installed.
>> Only the executable are installed.
>> I followed the same Makefile as futex.
>> Currently I am trying to figure out what could be the problem.
>> Any hint will be really appreciated.
>
> Please see the latest futex Makefile from 4.14. You will see that the
> TEST_PROGS get copied from its all target for sub-dirs. You just have to
> add a copy in that path
>

Yes, I am already using the latest version of futex/Makefile.
Actually I figured out the problem, but currently I don't know how to fix it.
In futex case, both in the main dir as well as sub-dir they are using
the same TEST_PROGS name as <run.sh>.
So both the scripts are getting installed in /tmp/.
However in my case, in main dir I am using <run.sh> but in sub-dir I
am using <ion_test.sh> as TEST_PROGS.
So, the sub-dir TEST_PROGS is not installed. Only the TEST_GEN_FILES
are installed.
If I change the <ion_test.sh> name to <run.sh>, then it is working fine.

Currently I am checking more how to fix this. This could be problem
with futex as well.
If I could not figure out, I will also change TEST_PROGS to run.sh and push it.


> if [ -e $$DIR/$(TEST_PROGS) ]; then
>      rsync -a $$DIR/$(TEST_PROGS) $$BUILD_TARGET/;
> fi
>
>
> All target from futex/Makefile
> 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
>
>
> thanks,
> -- Shuah
>