Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCHv5 1/1] [tools/selftests]: android/ion: userspace test
 utility for ion buffer sharing
To:     Shuah Khan <shuahkh@osg.samsung.com>,
        Pintu Kumar <pintu.ping@gmail.com>, shuah@kernel.org
Cc:     linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        dvhart@infradead.org,
        Bamvor Zhang Jian <bamvor.zhangjian@linaro.org>,
        Pintu Kumar <pintu_agarwal@yahoo.com>
References: <1508801195-5369-1-git-send-email-pintu.ping@gmail.com>
 <1509555639-26128-1-git-send-email-pintu.ping@gmail.com>
 <014ae05d-0404-10de-bcc8-7d18f441566a@kernel.org>
 <CAOuPNLjNYqA45g1RhzYAD0WaPoONjZ-zX6xwWpHML+MJMh+3_A@mail.gmail.com>
 <3304c07d-b578-2fd5-1937-22a7869021bf@osg.samsung.com>
 <ada2a345-e564-11fb-7984-d4ef1d8b6130@osg.samsung.com>
From:   Laura Abbott <labbott@redhat.com>
Message-ID: <6b0c39e6-5fb4-edea-6d8e-e68852dcd33b@redhat.com>
Date:   Thu, 2 Nov 2017 16:54:25 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.3.0
MIME-Version: 1.0
In-Reply-To: <ada2a345-e564-11fb-7984-d4ef1d8b6130@osg.samsung.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 11/02/2017 10:53 AM, Shuah Khan wrote:
> On 11/02/2017 10:19 AM, Shuah Khan wrote:
>> On 11/02/2017 10:15 AM, Pintu Kumar wrote:
>>> On Thu, Nov 2, 2017 at 8:37 PM, Shuah Khan <shuah@kernel.org> wrote:
>>>> Hi Pintu,
>>>>
>>>> On 11/01/2017 11:00 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.
>>>>>
>>>>> For more information about the utility please check the README file.
>>>>>
>>>>> Signed-off-by: Pintu Agarwal <pintu.ping@gmail.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.h          | 143 ++++++++++++
>>>>>  tools/testing/selftests/android/ion/ion_test.sh    |  55 +++++
>>>>>  .../testing/selftests/android/ion/ionapp_export.c  | 135 +++++++++++
>>>>>  .../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 +
>>>>>  15 files changed, 1171 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 100644 tools/testing/selftests/android/ion/ion.h
>>>>>  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
>>>>
>>>> Based on our discussion on dependency on staging headers, let's not add
>>>> this test to kselftest run. Remove it from here and users can run it
>>>> as needed.
>>>>
>>>>> +TARGETS += bpf
>>>>>  TARGETS += breakpoints
>>>>>  TARGETS += capabilities
>>>>>  TARGETS += cpufreq
>>>>> diff --git a/tools/testing/selftests/android/Makefile b/tools/testing/selftests/android/Makefil> 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
>>>>
>>>> This Makefile is fine, you still want to leverage common run_tests target
>>>> in lib.mk Install and emit tests should be fine, because these are built
>>>> in the source tree. I am looking to avoid attempts to build this test
>>>> outside kernel tree.
>>>
>>> Ok, I understood what you mean.
>>> You want to remove ion test from kselftest build, but still maintain
>>> the test code inside selftest.
>>> Yes, I agree with this, kselftest will succeed (outside of kernel
>>> source tree), and still somebody can build and run it locally.
>>
>> Right.
>>
>>>
>>> But, my concern is, whole this recent patches were made to include it
>>> into kselftest framework.
>>> Otherwise in my previous patch, I included it under:
>>> tools/testing/android (outside of selftest).
>>
>> I do want this test under kselftest framework. It doesn't make sense to
>> add this under tools/testing/adroid. At some point we could add it to
>> the selftest/Makefile TARGETS list for it to be run very easily if it
>> resides under selftests.
>>
>>>
>>> If, removing the android/ion from kselftest is fine (as of now), I can
>>> push the new changes by tomorrow.
>>> Just waiting for Greg and Laura to confirm...
>>>
> 
> I did some sanity tests on this patch on a system with staging and android
> enabled. Most users will see
> 
> ion_test.sh: No /dev/ion device found
> ion_test.sh: May be CONFIG_ION is not set
> 
> Leaving this in kselftest run might be useful on adroid veification
> tests.
> 
> With ion.h local file, outside builds work. We will just have to remember
> to clean this up if and when ion moves from staging.
> 
> I am planning to take this patch as is for 4.15-rc1.
> 
> Laura! Are you good with the heap size changes in this patch?
> 
> thanks,
> -- Shuah
> 

Yes, I think this is okay.

Thanks,
Laura

From 1582977806601132275@xxx Thu Nov 02 17:54:37 +0000 2017
X-GM-THRID: 1579656416160844925
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread