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

From: Shuah Khan <shuah@kernel.org>
To: Pintu Agarwal <pintu.ping@gmail.com>,
	linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
	labbott@redhat.com, gregkh@linuxfoundation.org,
	dvhart@infradead.org, bamvor.zhangjian@linaro.org,
	pintu_agarwal@yahoo.com, Shuah Khan <shuahkh@osg.samsung.com>
Cc: Shuah Khan <shuah@kernel.org>
Subject: Re: [PATCHv5 1/1] [tools/selftests]: android/ion: userspace test utility for ion buffer sharing
Date: Thu, 2 Nov 2017 09:07:47 -0600	[thread overview]
Message-ID: <014ae05d-0404-10de-bcc8-7d18f441566a@kernel.org> (raw)
In-Reply-To: <1509555639-26128-1-git-send-email-pintu.ping@gmail.com>

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.

thanks,
-- Shuah