Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sun, 7 Aug 2022 13:25:46 +0800
From:   Leo Yan <leo.yan@linaro.org>
To:     carsten.haitzler@foss.arm.com
Cc:     linux-kernel@vger.kernel.org, coresight@lists.linaro.org,
        suzuki.poulose@arm.com, mathieu.poirier@linaro.org,
        mike.leach@linaro.org, linux-perf-users@vger.kernel.org,
        acme@kernel.org
Subject: Re: [PATCH v5 11/14] perf test: Add unroll thread test tool
Message-ID: <20220807052546.GI34254@leoy-ThinkPad-X240s>
References: <20220728145256.2985298-1-carsten.haitzler@foss.arm.com>
 <20220728145256.2985298-12-carsten.haitzler@foss.arm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220728145256.2985298-12-carsten.haitzler@foss.arm.com>
Precedence: bulk

On Thu, Jul 28, 2022 at 03:52:53PM +0100, carsten.haitzler@foss.arm.com wrote:
> From: "Carsten Haitzler (Rasterman)" <raster@rasterman.com>
> 
> Add test tool to be driven by further test scripts. This is a simple C
> based test that is for arm64 with some inline ASM to manually unroll a
> lot of code to have a very long sequence of commands.
> 
> Signed-off-by: Carsten Haitzler <carsten.haitzler@arm.com>
> ---
>  tools/perf/tests/shell/coresight/Makefile     |  3 +-
>  .../coresight/unroll_loop_thread/.gitignore   |  1 +
>  .../coresight/unroll_loop_thread/Makefile     | 33 +++++++++
>  .../unroll_loop_thread/unroll_loop_thread.c   | 74 +++++++++++++++++++
>  4 files changed, 110 insertions(+), 1 deletion(-)
>  create mode 100644 tools/perf/tests/shell/coresight/unroll_loop_thread/.gitignore
>  create mode 100644 tools/perf/tests/shell/coresight/unroll_loop_thread/Makefile
>  create mode 100644 tools/perf/tests/shell/coresight/unroll_loop_thread/unroll_loop_thread.c
> 
> diff --git a/tools/perf/tests/shell/coresight/Makefile b/tools/perf/tests/shell/coresight/Makefile
> index 004974a71fb8..3b2b876cd9e2 100644
> --- a/tools/perf/tests/shell/coresight/Makefile
> +++ b/tools/perf/tests/shell/coresight/Makefile
> @@ -7,7 +7,8 @@ include ../../../../../tools/scripts/utilities.mak
>  SUBDIRS = \
>  	asm_pure_loop \
>  	memcpy_thread \
> -	thread_loop
> +	thread_loop \
> +	unroll_loop_thread
>  
>  all: $(SUBDIRS)
>  $(SUBDIRS):
> diff --git a/tools/perf/tests/shell/coresight/unroll_loop_thread/.gitignore b/tools/perf/tests/shell/coresight/unroll_loop_thread/.gitignore
> new file mode 100644
> index 000000000000..2cb4e996dbf3
> --- /dev/null
> +++ b/tools/perf/tests/shell/coresight/unroll_loop_thread/.gitignore
> @@ -0,0 +1 @@
> +unroll_loop_thread
> diff --git a/tools/perf/tests/shell/coresight/unroll_loop_thread/Makefile b/tools/perf/tests/shell/coresight/unroll_loop_thread/Makefile
> new file mode 100644
> index 000000000000..6264c4e3abd1
> --- /dev/null
> +++ b/tools/perf/tests/shell/coresight/unroll_loop_thread/Makefile
> @@ -0,0 +1,33 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# Carsten Haitzler <carsten.haitzler@arm.com>, 2021
> +include ../Makefile.miniconfig
> +
> +# Binary to produce
> +BIN=unroll_loop_thread
> +# Any linking/libraries needed for the binary - empty if none needed
> +LIB=-pthread
> +
> +all: $(BIN)
> +
> +$(BIN): $(BIN).c
> +ifdef CORESIGHT
> +ifeq ($(ARCH),arm64)
> +# Build line
> +	$(Q)$(CC) $(BIN).c -o $(BIN) $(LIB)
> +endif
> +endif
> +
> +install-tests: all
> +ifdef CORESIGHT
> +ifeq ($(ARCH),arm64)
> +# Install the test tool in the right place
> +	$(call QUIET_INSTALL, tests) \
> +		$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(perfexec_instdir_SQ)/$(INSTDIR_SUB)/$(BIN)'; \
> +		$(INSTALL) $(BIN) '$(DESTDIR_SQ)$(perfexec_instdir_SQ)/$(INSTDIR_SUB)/$(BIN)/$(BIN)'
> +endif
> +endif
> +
> +clean:
> +	$(Q)$(RM) -f $(BIN)
> +
> +.PHONY: all clean install-tests
> diff --git a/tools/perf/tests/shell/coresight/unroll_loop_thread/unroll_loop_thread.c b/tools/perf/tests/shell/coresight/unroll_loop_thread/unroll_loop_thread.c
> new file mode 100644
> index 000000000000..cb9d22c7dfb9
> --- /dev/null
> +++ b/tools/perf/tests/shell/coresight/unroll_loop_thread/unroll_loop_thread.c
> @@ -0,0 +1,74 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Carsten Haitzler <carsten.haitzler@arm.com>, 2021
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <string.h>
> +#include <pthread.h>
> +
> +struct args {
> +	pthread_t th;
> +	unsigned int in, out;

nitpick: "out" field is not used, drop it?

With this minor fixing, the patch LGTM:

Reviewed-by: Leo Yan <leo.yan@linaro.org>

> +	void *ret;
> +};
> +
> +static void *thrfn(void *arg)
> +{
> +	struct args *a = arg;
> +	unsigned int i, in = a->in;
> +
> +	for (i = 0; i < 10000; i++) {
> +		asm volatile (
> +// force an unroll of thia add instruction so we can test long runs of code
> +#define SNIP1 "add %[in], %[in], #1\n"
> +// 10
> +#define SNIP2 SNIP1 SNIP1 SNIP1 SNIP1 SNIP1 SNIP1 SNIP1 SNIP1 SNIP1 SNIP1
> +// 100
> +#define SNIP3 SNIP2 SNIP2 SNIP2 SNIP2 SNIP2 SNIP2 SNIP2 SNIP2 SNIP2 SNIP2
> +// 1000
> +#define SNIP4 SNIP3 SNIP3 SNIP3 SNIP3 SNIP3 SNIP3 SNIP3 SNIP3 SNIP3 SNIP3
> +// 10000
> +#define SNIP5 SNIP4 SNIP4 SNIP4 SNIP4 SNIP4 SNIP4 SNIP4 SNIP4 SNIP4 SNIP4
> +// 100000
> +			SNIP5 SNIP5 SNIP5 SNIP5 SNIP5 SNIP5 SNIP5 SNIP5 SNIP5 SNIP5
> +			: /* out */
> +			: /* in */ [in] "r" (in)
> +			: /* clobber */
> +		);
> +	}
> +}
> +
> +static pthread_t new_thr(void *(*fn) (void *arg), void *arg)
> +{
> +	pthread_t t;
> +	pthread_attr_t attr;
> +
> +	pthread_attr_init(&attr);
> +	pthread_create(&t, &attr, fn, arg);
> +	return t;
> +}
> +
> +int main(int argc, char **argv)
> +{
> +	unsigned int i, thr;
> +	pthread_t threads[256];
> +	struct args args[256];
> +
> +	if (argc < 2) {
> +		printf("ERR: %s [numthreads]\n", argv[0]);
> +		exit(1);
> +	}
> +
> +	thr = atoi(argv[1]);
> +	if ((thr > 256) || (thr < 1)) {
> +		printf("ERR: threads 1-256\n");
> +		exit(1);
> +	}
> +	for (i = 0; i < thr; i++) {
> +		args[i].in = rand();
> +		args[i].th = new_thr(thrfn, &(args[i]));
> +	}
> +	for (i = 0; i < thr; i++)
> +		pthread_join(args[i].th, &(args[i].ret));
> +	return 0;
> +}
> -- 
> 2.32.0
>