On 06/14/2017 04:06 AM, Shuah Khan wrote:
> Hi Greg/Paul/Alice,
>
> On 06/12/2017 12:56 AM, Greg Kroah-Hartman wrote:
>> From: Paul Elder <paul.elder@xxxxxxxx>
>>
>> Make the membarrier test output in the TAP13 format by using the
>> TAP13 output functions defined in kselftest.h
>>
>> Signed-off-by: Paul Elder <paul.elder@xxxxxxxx>
>> Signed-off-by: Alice Ferrazzi <alice.ferrazzi@xxxxxxxxx>
>> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
>> ---
>> v2: Just use the standard function names, no _tap suffix - Alice
>
> I am pulling all of these into linux-kselftest next very soon.
>
> Please send a separate patch to fix the messages for this test.
> Details below:
>
> The output looks like this:
>
> TAP version 13
> ok 1 sys_membarrier available
> ok 2 membarrier command fail
> ok 3 Wrong flags should fail
> ok 4 execute MEMBARRIER_CMD_SHARED
> 1..4
> selftests: membarrier_test [PASS]
>
> Looks good overall - couple of comments:
> Please use these as reference for output.
>
> thanks again for doing this.
> -- Shuah
>
>>
>>
>> .../testing/selftests/membarrier/membarrier_test.c | 35 ++++++++++++----------
>> 1 file changed, 19 insertions(+), 16 deletions(-)
>>
>> diff --git a/tools/testing/selftests/membarrier/membarrier_test.c b/tools/testing/selftests/membarrier/membarrier_test.c
>> index 535f0fef4d0b..cae8c984dfb0 100644
>> --- a/tools/testing/selftests/membarrier/membarrier_test.c
>> +++ b/tools/testing/selftests/membarrier/membarrier_test.c
>> @@ -21,36 +21,42 @@ static int sys_membarrier(int cmd, int flags)
>> static enum test_membarrier_status test_membarrier_cmd_fail(void)
>> {
>> int cmd = -1, flags = 0;
>> + const char *test_name = "membarrier command fail";
>>
>> if (sys_membarrier(cmd, flags) != -1) {
>> - printf("membarrier: Wrong command should fail but passed.\n");
>> + ksft_test_result_fail(test_name);
>
> This changes the text - I think this should read:
> ok 2 membarrier command cmd=-1. Wrong command should fail but passed.
>
> instead of "ok 2 membarrier command fail"
>
>> return TEST_MEMBARRIER_FAIL;
>> }
>> +
>> + ksft_test_result_pass(test_name);
>> return TEST_MEMBARRIER_PASS;
>> }
>>
>> static enum test_membarrier_status test_membarrier_flags_fail(void)
>> {
>> int cmd = MEMBARRIER_CMD_QUERY, flags = 1;
>> + const char *test_name = "Wrong flags should fail";
>>
>> if (sys_membarrier(cmd, flags) != -1) {
>> - printf("membarrier: Wrong flags should fail but passed.\n");
>> + ksft_test_result_fail(test_name);
>
> This can be improved to read:
>
> "ok 3 MEMBARRIER_CMD_QUERY - Wrong flags should fail"
I see. Thank you. I wasn't sure what you meant the first time by adding more detail
to the messages.
Thanks,
Paul
>
>> return TEST_MEMBARRIER_FAIL;
>> }
>> +
>> + ksft_test_result_pass(test_name);
>> return TEST_MEMBARRIER_PASS;
>> }
>>
>> static enum test_membarrier_status test_membarrier_success(void)
>> {
>> int cmd = MEMBARRIER_CMD_SHARED, flags = 0;
>> + const char *test_name = "execute MEMBARRIER_CMD_SHARED";
>>
>> if (sys_membarrier(cmd, flags) != 0) {
>> - printf("membarrier: Executing MEMBARRIER_CMD_SHARED failed. %s.\n",
>> - strerror(errno));
>> + ksft_test_result_fail(test_name);
>> return TEST_MEMBARRIER_FAIL;
>> }
>>
>> - printf("membarrier: MEMBARRIER_CMD_SHARED success.\n");
>> + ksft_test_result_pass(test_name);
>> return TEST_MEMBARRIER_PASS;
>> }
>>
>> @@ -74,32 +80,30 @@ static enum test_membarrier_status test_membarrier_query(void)
>> {
>> int flags = 0, ret;
>>
>> - printf("membarrier MEMBARRIER_CMD_QUERY ");
>> ret = sys_membarrier(MEMBARRIER_CMD_QUERY, flags);
>> if (ret < 0) {
>> - printf("failed. %s.\n", strerror(errno));
>> - switch (errno) {
>> - case ENOSYS:
>> + if (errno == ENOSYS) {
>> /*
>> * It is valid to build a kernel with
>> * CONFIG_MEMBARRIER=n. However, this skips the tests.
>> */
>> - return TEST_MEMBARRIER_SKIP;
>> - case EINVAL:
>> - default:
>> - return TEST_MEMBARRIER_FAIL;
>> + ksft_test_result_skip("CONFIG_MEMBARRIER is not enabled\n");
>> + return ksft_exit_skip();
>> }
>> + ksft_test_result_fail("sys_membarrier() failed\n");
>> + return TEST_MEMBARRIER_FAIL;
>> }
>> if (!(ret & MEMBARRIER_CMD_SHARED)) {
>> - printf("command MEMBARRIER_CMD_SHARED is not supported.\n");
>> + ksft_test_result_fail("command MEMBARRIER_CMD_SHARED is not supported.\n");
>> return TEST_MEMBARRIER_FAIL;
>> }
>> - printf("syscall available.\n");
>> + ksft_test_result_pass("sys_membarrier available");
>> return TEST_MEMBARRIER_PASS;
>> }
>>
>> int main(int argc, char **argv)
>> {
>> + ksft_print_header();
>> switch (test_membarrier_query()) {
>> case TEST_MEMBARRIER_FAIL:
>> return ksft_exit_fail();
>> @@ -113,6 +117,5 @@ int main(int argc, char **argv)
>> return ksft_exit_skip();
>> }
>>
>> - printf("membarrier: tests done!\n");
>> return ksft_exit_pass();
>> }
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
