Message-ID: <d1e69936-9e5e-fa60-542d-a0496cffe7f3@osg.samsung.com>
Date:   Fri, 20 Oct 2017 13:21:57 -0600
From:   Shuah Khan <shuahkh@....samsung.com>
To:     Pintu Kumar <pintu.ping@...il.com>
Cc:     shuah@...nel.org, Laura Abbott <labbott@...hat.com>,
        linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        dvhart@...radead.org,
        Bamvor Zhang Jian <bamvor.zhangjian@...aro.org>,
        Shuah Khan <shuahkh@....samsung.com>
Subject: Re: [PATCHv3 1/1] [tools/selftests]: android/ion: userspace test
 utility for ion buffer sharing

On 10/20/2017 09:18 AM, Pintu Kumar wrote:
> On Fri, Oct 20, 2017 at 8:10 PM, Shuah Khan <shuahkh@....samsung.com> wrote:
>> On 10/18/2017 04:38 AM, Pintu Kumar wrote:
>>> On Wed, Oct 18, 2017 at 2:28 AM, Shuah Khan <shuah@...nel.org> 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@...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.
>>>>
>>>
>>> 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.

thanks,
-- Shuah

Powered by blists - more mailing lists