[RFC,11/11] selftests/powerpc: Adapt the test

Message ID 1536781219-13938-12-git-send-email-leitao@debian.org
State New
Headers show
Series
  • New TM Model
Related show

Checks

Context Check Description
snowpatch_ozlabs/build-ppc32 success Test build-ppc32 on branch next
snowpatch_ozlabs/build-ppc64e success Test build-ppc64e on branch next
snowpatch_ozlabs/build-ppc64be success Test build-ppc64be on branch next
snowpatch_ozlabs/build-ppc64le success Test build-ppc64le on branch next
snowpatch_ozlabs/checkpatch success Test checkpatch on branch next
snowpatch_ozlabs/apply_patch success next/apply_patch Successfully applied

Commit Message

Breno Leitao Sept. 12, 2018, 7:40 p.m.
The Documentation/powerpc/transactional_memory.txt says:

 "Syscalls made from within a suspended transaction are performed as normal
  and the transaction is not explicitly doomed by the kernel.  However,
  what the kernel does to perform the syscall may result in the transaction
  being doomed by the hardware."

With this new TM mechanism, the syscall will continue to be executed if the
syscall happens on a suspended syscall, but, the syscall will *fail* if the
transaction is still active during the syscall invocation.

On the syscall path, if the transaction is active and not suspended, it
will call TM_KERNEL_ENTRY which will reclaim and recheckpoint the
transaction, thus, dooming the transaction on userspace return, with
failure code TM_CAUSE_SYSCALL.

This new model will break part of this test, but I understand that that the
documentation above didn't guarantee that the syscall would succeed, and it
will never succeed anymore now on.

In fact, glibc is calling 'tabort' before every syscalls, thus, any syscall
called through glibc from inside a transaction will be doomed anyhow.

This patch updates the test case to not assume that a syscall inside a
active transaction will succeed, because it will not anymore.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 tools/testing/selftests/powerpc/tm/tm-syscall.c | 6 ------
 1 file changed, 6 deletions(-)

Comments

Michael Neuling Sept. 18, 2018, 6:36 a.m. | #1
On Wed, 2018-09-12 at 16:40 -0300, Breno Leitao wrote:
> The Documentation/powerpc/transactional_memory.txt says:
> 
>  "Syscalls made from within a suspended transaction are performed as normal
>   and the transaction is not explicitly doomed by the kernel.  However,
>   what the kernel does to perform the syscall may result in the transaction
>   being doomed by the hardware."
> 
> With this new TM mechanism, the syscall will continue to be executed if the
> syscall happens on a suspended syscall, but, the syscall will *fail* if the
> transaction is still active during the syscall invocation.

Not sure I get this. This doesn't seem any different to before.

An active (not suspended) transaction *will* result in the syscall failing and
the transaction being doomed.  

A syscall in a suspended transaction should succeed and the transaction.

You might need to clean up the language. I try to use:

   Active == transactional but not suspended (ie MSR[TS] = T)
   Suspended == suspended (ie MSR [TS] = S)
   Doomed == transaction to be rolled back at next opportinity (ie tcheck returns doomed)

(note: the kernel MSR_TM_ACTIVE() macro is not consistent with this since it's
MSR[TS] == S or T).

> On the syscall path, if the transaction is active and not suspended, it
> will call TM_KERNEL_ENTRY which will reclaim and recheckpoint the
> transaction, thus, dooming the transaction on userspace return, with
> failure code TM_CAUSE_SYSCALL.

But the test below is on a suspend transaction?

> This new model will break part of this test, but I understand that that the
> documentation above didn't guarantee that the syscall would succeed, and it
> will never succeed anymore now on.

The syscall should pass in suspend (modulo the normal syscall checks). The
transaction may fail as a result.

> In fact, glibc is calling 'tabort' before every syscalls, thus, any syscall
> called through glibc from inside a transaction will be doomed anyhow.
> 
> This patch updates the test case to not assume that a syscall inside a
> active transaction will succeed, because it will not anymore.
> 
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
>  tools/testing/selftests/powerpc/tm/tm-syscall.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/tools/testing/selftests/powerpc/tm/tm-syscall.c
> b/tools/testing/selftests/powerpc/tm/tm-syscall.c
> index 454b965a2db3..1439a87eba3a 100644
> --- a/tools/testing/selftests/powerpc/tm/tm-syscall.c
> +++ b/tools/testing/selftests/powerpc/tm/tm-syscall.c
> @@ -78,12 +78,6 @@ int tm_syscall(void)
>  	timeradd(&end, &now, &end);
>  
>  	for (count = 0; timercmp(&now, &end, <); count++) {
> -		/*
> -		 * Test a syscall within a suspended transaction and verify
> -		 * that it succeeds.
> -		 */
> -		FAIL_IF(getppid_tm(true) == -1); /* Should succeed. */
> -
>  		/*
>  		 * Test a syscall within an active transaction and verify that
>  		 * it fails with the correct failure code.

Patch

diff --git a/tools/testing/selftests/powerpc/tm/tm-syscall.c b/tools/testing/selftests/powerpc/tm/tm-syscall.c
index 454b965a2db3..1439a87eba3a 100644
--- a/tools/testing/selftests/powerpc/tm/tm-syscall.c
+++ b/tools/testing/selftests/powerpc/tm/tm-syscall.c
@@ -78,12 +78,6 @@  int tm_syscall(void)
 	timeradd(&end, &now, &end);
 
 	for (count = 0; timercmp(&now, &end, <); count++) {
-		/*
-		 * Test a syscall within a suspended transaction and verify
-		 * that it succeeds.
-		 */
-		FAIL_IF(getppid_tm(true) == -1); /* Should succeed. */
-
 		/*
 		 * Test a syscall within an active transaction and verify that
 		 * it fails with the correct failure code.