Re: [kvm-unit-tests 1/1] arm64: microbench: Move the read of the count register and the ISB operation out of the while loop

[Andrew Jones <andrew.jones@linux.dev>]

Thanks for submitting the patch more correctly, but there's still two
more problems with the patch submission. The patch summary (email subject)
is too long. It also simply describes the change of implementation, which
is easy to see when looking at the patch. It should instead describe the
purpose of the patch, e.g.

  arm64: microbench: Improve measurement accuracy of tests

The second problem is it's missing your signed-off-by (which I think I
pointed out last time too).

Please see [1] for more information about patch formatting. You can also
run the Linux kernel's scripts/checkpatch.pl on the patch to catch these
types of things as well as other code style issues.

[1] https://www.kernel.org/doc/html/latest/process/submitting-patches.html#the-canonical-patch-format

Thanks,
drew

On Tue, Nov 07, 2023 at 02:40:06PM +0800, heqiong wrote:
> Reducing the impact of the cntvct_el0 register and isb() operation
> on microbenchmark test results to improve testing accuracy and reduce
> latency in test results.
> ---
>  arm/micro-bench.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/arm/micro-bench.c b/arm/micro-bench.c
> index fbe59d03..6b940d56 100644
> --- a/arm/micro-bench.c
> +++ b/arm/micro-bench.c
> @@ -346,17 +346,21 @@ static void loop_test(struct exit_test *test)
>  		}
>  	}
>  
> +	dsb(ish);
> +	isb();
> +	start = read_sysreg(cntpct_el0);
> +	isb();
>  	while (ntimes < test->times && total_ns.ns < NS_5_SECONDS) {
> -		isb();
> -		start = read_sysreg(cntvct_el0);
>  		test->exec();
> -		isb();
> -		end = read_sysreg(cntvct_el0);
>  
>  		ntimes++;
> -		total_ticks += (end - start);
> -		ticks_to_ns_time(total_ticks, &total_ns);
>  	}
> +	dsb(ish);
> +	isb();
> +	end = read_sysreg(cntpct_el0);
> +
> +	total_ticks = end - start;
> +	ticks_to_ns_time(total_ticks, &total_ns);
>  
>  	if (test->post) {
>  		test->post(ntimes, &total_ticks);
> -- 
> 2.31.1
>