Re: [PATCH 3/3] selftests/powerpc: Add transactional syscall test

[Anshuman Khandual <khandual@linux.vnet.ibm.com>]

On 03/19/2015 10:13 AM, Sam Bobroff wrote:
> Check that a syscall made during an active transaction will fail with
> the correct failure code and that one made during a suspended
> transaction will succeed.
> 
> Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>

The test works.

> +
> +int tm_syscall(void)
> +{
> +	SKIP_IF(!((long)get_auxv_entry(AT_HWCAP2) & PPC_FEATURE2_HTM));
> +	setbuf(stdout, 0);
> +	FAIL_IF(!t_active_getppid_test());
> +	printf("%d active transactions correctly aborted.\n", TM_TEST_RUNS);
> +	FAIL_IF(!t_suspended_getppid_test());
> +	printf("%d suspended transactions succeeded.\n", TM_TEST_RUNS);
> +	return 0;
> +}
> +
> +int main(void)
> +{
> +	return test_harness(tm_syscall, "tm_syscall");
> +}
> +

There is an extra blank line at the end of this file. Interchanging return
codes of 0 and 1 for various functions make it very confusing along with
negative FAIL_IF checks in the primary test function. Control flow structures
like these can use some in-code documentation for readability.

+	for (i = 0; i < TM_RETRIES; i++) {
+		if (__builtin_tbegin(0)) {
+			getppid();
+			__builtin_tend(0);
+			return 1;
+		}
+		if (t_failure_persistent())
+			return 0;

or

+		if (__builtin_tbegin(0)) {
+			__builtin_tsuspend();
+			getppid();
+			__builtin_tresume();
+			__builtin_tend(0);
+			return 1;
+		}
+		if (t_failure_persistent())
+			return 0;