On 20/03/15 20:25, Anshuman Khandual wrote:
> 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.bobr...@au1.ibm.com>
>
> The test works.
Great :-)
>> +
>> +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;
>
Good points. I'll remove the blank line and comment the code.
I'm not sure I can do any better with the FAIL_IF() macro: I wanted it
to read "fail if the test failed", but I can see what you mean about a
double negative. Maybe it would be better to introduce a different
macro, more like a standard assert: TEST(XXX) which fails if XXX is
false. However, I think "TEST" would be too generic a name and I'm not
should what would be better. Any comments/suggestions?
Thanks for the review!
Cheers,
Sam.
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
