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

[Pintu Kumar]

On Sat, Oct 21, 2017 at 12:51 AM, Shuah Khan <shuahkh@xxxxxxxxxxxxxxx> wrote:
> On 10/20/2017 09:18 AM, Pintu Kumar wrote:
>> 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.
>
> That should be fine. Let me play with it and let you know.
>
>> So, the sub-dir TEST_PROGS is not installed. Only the TEST_GEN_FILES
>> are installed.
>
> Right. This is not for install path. This is setting up the O=dir with
> the right files for run_tests target.
>
>> 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.
>
> I don't think that is the problem.'
>
>> If I could not figure out, I will also change TEST_PROGS to run.sh and push it.
>

Ok I found the problem and fixed it. Now it is working fine with
ion_test.sh script as well.
I verified that the same problem is there in futex as well, if I
change the name of script inside functional from run.sh to some other
name.
Now I verified all the various test using kselftest.
- make kselftest
- ./kselftest_install.sh
- make TARGETS=android kselftest
- make -C tools/testing/selftests TARGETS=android run_tests
- make O=/tmp/kselftest TARGETS=android kselftest

And I have verified using root and without root as well.
I will push the new patch set soon.

Thanks,
Pintu


> thanks,
> -- Shuah
>