Re: [PATCH] selftests/powerpc: Skip tm-unavailable if TM is not enabled

[Gustavo Romero <gromero@linux.vnet.ibm.com>]

Hi Cyril,

On 03/05/2018 08:49 PM, Cyril Bur wrote:
> On Mon, 2018-03-05 at 15:48 -0500, Gustavo Romero wrote:
>> Some processor revisions do not support transactional memory, and
>> additionally kernel support can be disabled. In either case the
>> tm-unavailable test should be skipped, otherwise it will fail with
>> a SIGILL.
>>
>> That commit also sets this selftest to be called through the test
>> harness as it's done for other TM selftests.
>>
>> Finally, it avoids using "ping" as a thread name since it's
>> ambiguous and can be confusing when shown, for instance,
>> in a kernel backtrace log.
>>
> 
> I spent more time than I care to admit looking at backtraces wondering
> how "ping" got in the mix ;).

heh sorry about that... :)


>> Fixes: 77fad8bfb1d2 ("selftests/powerpc: Check FP/VEC on exception in TM")
>> Signed-off-by: Gustavo Romero <gromero@linux.vnet.ibm.com>
> 
> Reviewed-by: Cyril Bur <cyrilbur@gmail.com>

Thanks for reviewing it.


Cheers,
Gustavo

>> ---
>>  .../testing/selftests/powerpc/tm/tm-unavailable.c  | 24 ++++++++++++++--------
>>  1 file changed, 16 insertions(+), 8 deletions(-)
>>
>> diff --git a/tools/testing/selftests/powerpc/tm/tm-unavailable.c b/tools/testing/selftests/powerpc/tm/tm-unavailable.c
>> index e6a0fad..156c8e7 100644
>> --- a/tools/testing/selftests/powerpc/tm/tm-unavailable.c
>> +++ b/tools/testing/selftests/powerpc/tm/tm-unavailable.c
>> @@ -80,7 +80,7 @@ bool is_failure(uint64_t condition_reg)
>>  	return ((condition_reg >> 28) & 0xa) == 0xa;
>>  }
>>  
>> -void *ping(void *input)
>> +void *tm_una_ping(void *input)
>>  {
>>  
>>  	/*
>> @@ -280,7 +280,7 @@ void *ping(void *input)
>>  }
>>  
>>  /* Thread to force context switch */
>> -void *pong(void *not_used)
>> +void *tm_una_pong(void *not_used)
>>  {
>>  	/* Wait thread get its name "pong". */
>>  	if (DEBUG)
>> @@ -311,11 +311,11 @@ void test_fp_vec(int fp, int vec, pthread_attr_t *attr)
>>  	do {
>>  		int rc;
>>  
>> -		/* Bind 'ping' to CPU 0, as specified in 'attr'. */
>> -		rc = pthread_create(&t0, attr, ping, (void *) &flags);
>> +		/* Bind to CPU 0, as specified in 'attr'. */
>> +		rc = pthread_create(&t0, attr, tm_una_ping, (void *) &flags);
>>  		if (rc)
>>  			pr_err(rc, "pthread_create()");
>> -		rc = pthread_setname_np(t0, "ping");
>> +		rc = pthread_setname_np(t0, "tm_una_ping");
>>  		if (rc)
>>  			pr_warn(rc, "pthread_setname_np");
>>  		rc = pthread_join(t0, &ret_value);
>> @@ -333,13 +333,15 @@ void test_fp_vec(int fp, int vec, pthread_attr_t *attr)
>>  	}
>>  }
>>  
>> -int main(int argc, char **argv)
>> +int tm_unavailable_test(void)
>>  {
>>  	int rc, exception; /* FP = 0, VEC = 1, VSX = 2 */
>>  	pthread_t t1;
>>  	pthread_attr_t attr;
>>  	cpu_set_t cpuset;
>>  
>> +	SKIP_IF(!have_htm());
>> +
>>  	/* Set only CPU 0 in the mask. Both threads will be bound to CPU 0. */
>>  	CPU_ZERO(&cpuset);
>>  	CPU_SET(0, &cpuset);
>> @@ -354,12 +356,12 @@ int main(int argc, char **argv)
>>  	if (rc)
>>  		pr_err(rc, "pthread_attr_setaffinity_np()");
>>  
>> -	rc = pthread_create(&t1, &attr /* Bind 'pong' to CPU 0 */, pong, NULL);
>> +	rc = pthread_create(&t1, &attr /* Bind to CPU 0 */, tm_una_pong, NULL);
>>  	if (rc)
>>  		pr_err(rc, "pthread_create()");
>>  
>>  	/* Name it for systemtap convenience */
>> -	rc = pthread_setname_np(t1, "pong");
>> +	rc = pthread_setname_np(t1, "tm_una_pong");
>>  	if (rc)
>>  		pr_warn(rc, "pthread_create()");
>>  
>> @@ -394,3 +396,9 @@ int main(int argc, char **argv)
>>  		exit(0);
>>  	}
>>  }
>> +
>> +int main(int argc, char **argv)
>> +{
>> +	test_harness_set_timeout(220);
>> +	return test_harness(tm_unavailable_test, "tm_unavailable_test");
>> +}
>