[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a9009a07-b4df-8fd7-96bc-4702f290e8fb@osg.samsung.com>
Date: Thu, 2 Nov 2017 18:32:13 -0600
From: Shuah Khan <shuahkh@....samsung.com>
To: Pintu Kumar <pintu.ping@...il.com>, shuah@...nel.org,
Pintu Kumar <pintu_agarwal@...oo.com>
Cc: 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: [PATCHv5 1/1] [tools/selftests]: android/ion: userspace test
utility for ion buffer sharing
On 11/02/2017 05:54 PM, Laura Abbott wrote:
> 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@...nel.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@...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.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
>
Applied to linux-kselftest next for 4.15-rc1.
thanks,
-- Shuah
Powered by blists - more mailing lists