kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [kvm-unit-tests PATCH] s390x: selftest: Fix report output
@ 2021-05-31 10:50 Janosch Frank
  2021-05-31 10:56 ` Claudio Imbrenda
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Janosch Frank @ 2021-05-31 10:50 UTC (permalink / raw)
  To: kvm; +Cc: linux-s390, imbrenda, david, thuth, cohuck

To make our TAP parser (and me) happy we don't want to have to reports
with exactly the same wording.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 s390x/selftest.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/s390x/selftest.c b/s390x/selftest.c
index b2fe2e7b..c2ca9896 100644
--- a/s390x/selftest.c
+++ b/s390x/selftest.c
@@ -47,12 +47,19 @@ static void test_malloc(void)
 	*tmp2 = 123456789;
 	mb();
 
-	report((uintptr_t)tmp & 0xf000000000000000ul, "malloc: got vaddr");
-	report(*tmp == 123456789, "malloc: access works");
+	report_prefix_push("malloc");
+	report_prefix_push("ptr_0");
+	report((uintptr_t)tmp & 0xf000000000000000ul, "allocated memory");
+	report(*tmp == 123456789, "wrote allocated memory");
+	report_prefix_pop();
+
+	report_prefix_push("ptr_1");
 	report((uintptr_t)tmp2 & 0xf000000000000000ul,
-	       "malloc: got 2nd vaddr");
-	report((*tmp2 == 123456789), "malloc: access works");
-	report(tmp != tmp2, "malloc: addresses differ");
+	       "allocated memory");
+	report((*tmp2 == 123456789), "wrote allocated memory");
+	report_prefix_pop();
+
+	report(tmp != tmp2, "allocated memory addresses differ");
 
 	expect_pgm_int();
 	configure_dat(0);
@@ -62,6 +69,7 @@ static void test_malloc(void)
 
 	free(tmp);
 	free(tmp2);
+	report_prefix_pop();
 }
 
 int main(int argc, char**argv)
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [kvm-unit-tests PATCH] s390x: selftest: Fix report output
  2021-05-31 10:50 [kvm-unit-tests PATCH] s390x: selftest: Fix report output Janosch Frank
@ 2021-05-31 10:56 ` Claudio Imbrenda
  2021-05-31 11:11 ` Cornelia Huck
  2021-05-31 11:15 ` David Hildenbrand
  2 siblings, 0 replies; 6+ messages in thread
From: Claudio Imbrenda @ 2021-05-31 10:56 UTC (permalink / raw)
  To: Janosch Frank; +Cc: kvm, linux-s390, david, thuth, cohuck

On Mon, 31 May 2021 10:50:03 +0000
Janosch Frank <frankja@linux.ibm.com> wrote:

> To make our TAP parser (and me) happy we don't want to have to reports
> with exactly the same wording.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>

Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

> ---
>  s390x/selftest.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/s390x/selftest.c b/s390x/selftest.c
> index b2fe2e7b..c2ca9896 100644
> --- a/s390x/selftest.c
> +++ b/s390x/selftest.c
> @@ -47,12 +47,19 @@ static void test_malloc(void)
>  	*tmp2 = 123456789;
>  	mb();
>  
> -	report((uintptr_t)tmp & 0xf000000000000000ul, "malloc: got
> vaddr");
> -	report(*tmp == 123456789, "malloc: access works");
> +	report_prefix_push("malloc");
> +	report_prefix_push("ptr_0");
> +	report((uintptr_t)tmp & 0xf000000000000000ul, "allocated
> memory");
> +	report(*tmp == 123456789, "wrote allocated memory");
> +	report_prefix_pop();
> +
> +	report_prefix_push("ptr_1");
>  	report((uintptr_t)tmp2 & 0xf000000000000000ul,
> -	       "malloc: got 2nd vaddr");
> -	report((*tmp2 == 123456789), "malloc: access works");
> -	report(tmp != tmp2, "malloc: addresses differ");
> +	       "allocated memory");
> +	report((*tmp2 == 123456789), "wrote allocated memory");
> +	report_prefix_pop();
> +
> +	report(tmp != tmp2, "allocated memory addresses differ");
>  
>  	expect_pgm_int();
>  	configure_dat(0);
> @@ -62,6 +69,7 @@ static void test_malloc(void)
>  
>  	free(tmp);
>  	free(tmp2);
> +	report_prefix_pop();
>  }
>  
>  int main(int argc, char**argv)


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [kvm-unit-tests PATCH] s390x: selftest: Fix report output
  2021-05-31 10:50 [kvm-unit-tests PATCH] s390x: selftest: Fix report output Janosch Frank
  2021-05-31 10:56 ` Claudio Imbrenda
@ 2021-05-31 11:11 ` Cornelia Huck
  2021-05-31 12:09   ` Janosch Frank
  2021-05-31 11:15 ` David Hildenbrand
  2 siblings, 1 reply; 6+ messages in thread
From: Cornelia Huck @ 2021-05-31 11:11 UTC (permalink / raw)
  To: Janosch Frank; +Cc: kvm, linux-s390, imbrenda, david, thuth

On Mon, 31 May 2021 10:50:03 +0000
Janosch Frank <frankja@linux.ibm.com> wrote:

> To make our TAP parser (and me) happy we don't want to have to reports

"we want to have two reports" ?

If that's not what has been intended, I'm confused :)

> with exactly the same wording.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  s390x/selftest.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/s390x/selftest.c b/s390x/selftest.c
> index b2fe2e7b..c2ca9896 100644
> --- a/s390x/selftest.c
> +++ b/s390x/selftest.c
> @@ -47,12 +47,19 @@ static void test_malloc(void)
>  	*tmp2 = 123456789;
>  	mb();
>  
> -	report((uintptr_t)tmp & 0xf000000000000000ul, "malloc: got vaddr");
> -	report(*tmp == 123456789, "malloc: access works");
> +	report_prefix_push("malloc");
> +	report_prefix_push("ptr_0");
> +	report((uintptr_t)tmp & 0xf000000000000000ul, "allocated memory");
> +	report(*tmp == 123456789, "wrote allocated memory");
> +	report_prefix_pop();
> +
> +	report_prefix_push("ptr_1");
>  	report((uintptr_t)tmp2 & 0xf000000000000000ul,
> -	       "malloc: got 2nd vaddr");
> -	report((*tmp2 == 123456789), "malloc: access works");
> -	report(tmp != tmp2, "malloc: addresses differ");
> +	       "allocated memory");
> +	report((*tmp2 == 123456789), "wrote allocated memory");
> +	report_prefix_pop();
> +
> +	report(tmp != tmp2, "allocated memory addresses differ");
>  
>  	expect_pgm_int();
>  	configure_dat(0);
> @@ -62,6 +69,7 @@ static void test_malloc(void)
>  
>  	free(tmp);
>  	free(tmp2);
> +	report_prefix_pop();
>  }
>  
>  int main(int argc, char**argv)


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [kvm-unit-tests PATCH] s390x: selftest: Fix report output
  2021-05-31 10:50 [kvm-unit-tests PATCH] s390x: selftest: Fix report output Janosch Frank
  2021-05-31 10:56 ` Claudio Imbrenda
  2021-05-31 11:11 ` Cornelia Huck
@ 2021-05-31 11:15 ` David Hildenbrand
  2021-05-31 12:07   ` Janosch Frank
  2 siblings, 1 reply; 6+ messages in thread
From: David Hildenbrand @ 2021-05-31 11:15 UTC (permalink / raw)
  To: Janosch Frank, kvm; +Cc: linux-s390, imbrenda, thuth, cohuck

On 31.05.21 12:50, Janosch Frank wrote:
> To make our TAP parser (and me) happy we don't want to have to reports
> with exactly the same wording.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>   s390x/selftest.c | 18 +++++++++++++-----
>   1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/s390x/selftest.c b/s390x/selftest.c
> index b2fe2e7b..c2ca9896 100644
> --- a/s390x/selftest.c
> +++ b/s390x/selftest.c
> @@ -47,12 +47,19 @@ static void test_malloc(void)
>   	*tmp2 = 123456789;
>   	mb();
>   
> -	report((uintptr_t)tmp & 0xf000000000000000ul, "malloc: got vaddr");
> -	report(*tmp == 123456789, "malloc: access works");
> +	report_prefix_push("malloc");
> +	report_prefix_push("ptr_0");

instead of this "ptr_0" vs. "ptr_1" I'd just use

"allocated 1st page"
"wrote to 1st page"
"allocated 2nd page"
"wrote to 2nd page"
"1st and 2nd page differ"

Avoids one hierarchy of prefix_push ...

> +	report((uintptr_t)tmp & 0xf000000000000000ul, "allocated memory");
> +	report(*tmp == 123456789, "wrote allocated memory");
> +	report_prefix_pop();
> +
> +	report_prefix_push("ptr_1");
>   	report((uintptr_t)tmp2 & 0xf000000000000000ul,
> -	       "malloc: got 2nd vaddr");
> -	report((*tmp2 == 123456789), "malloc: access works");
> -	report(tmp != tmp2, "malloc: addresses differ");
> +	       "allocated memory");
> +	report((*tmp2 == 123456789), "wrote allocated memory");
> +	report_prefix_pop();
> +
> +	report(tmp != tmp2, "allocated memory addresses differ");
>   
>   	expect_pgm_int();
>   	configure_dat(0);
> @@ -62,6 +69,7 @@ static void test_malloc(void)
>   
>   	free(tmp);
>   	free(tmp2);
> +	report_prefix_pop();
>   }
>   
>   int main(int argc, char**argv)
> 


-- 
Thanks,

David / dhildenb


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [kvm-unit-tests PATCH] s390x: selftest: Fix report output
  2021-05-31 11:15 ` David Hildenbrand
@ 2021-05-31 12:07   ` Janosch Frank
  0 siblings, 0 replies; 6+ messages in thread
From: Janosch Frank @ 2021-05-31 12:07 UTC (permalink / raw)
  To: David Hildenbrand, kvm; +Cc: linux-s390, imbrenda, thuth, cohuck

On 5/31/21 1:15 PM, David Hildenbrand wrote:
> On 31.05.21 12:50, Janosch Frank wrote:
>> To make our TAP parser (and me) happy we don't want to have to reports
>> with exactly the same wording.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>   s390x/selftest.c | 18 +++++++++++++-----
>>   1 file changed, 13 insertions(+), 5 deletions(-)
>>
>> diff --git a/s390x/selftest.c b/s390x/selftest.c
>> index b2fe2e7b..c2ca9896 100644
>> --- a/s390x/selftest.c
>> +++ b/s390x/selftest.c
>> @@ -47,12 +47,19 @@ static void test_malloc(void)
>>   	*tmp2 = 123456789;
>>   	mb();
>>   
>> -	report((uintptr_t)tmp & 0xf000000000000000ul, "malloc: got vaddr");
>> -	report(*tmp == 123456789, "malloc: access works");
>> +	report_prefix_push("malloc");
>> +	report_prefix_push("ptr_0");
> 
> instead of this "ptr_0" vs. "ptr_1" I'd just use
> 
> "allocated 1st page"
> "wrote to 1st page"
> "allocated 2nd page"
> "wrote to 2nd page"
> "1st and 2nd page differ"
> 
> Avoids one hierarchy of prefix_push ...
I'd like to keep them since I'll also move the allocation and writes
into a prefix section for the v2 which will provide me with better error
reports if we trigger asserts.

Also from the allocation alone these could be on the same page since we
allocate ints.

> 
>> +	report((uintptr_t)tmp & 0xf000000000000000ul, "allocated memory");
>> +	report(*tmp == 123456789, "wrote allocated memory");
>> +	report_prefix_pop();
>> +
>> +	report_prefix_push("ptr_1");
>>   	report((uintptr_t)tmp2 & 0xf000000000000000ul,
>> -	       "malloc: got 2nd vaddr");
>> -	report((*tmp2 == 123456789), "malloc: access works");
>> -	report(tmp != tmp2, "malloc: addresses differ");
>> +	       "allocated memory");
>> +	report((*tmp2 == 123456789), "wrote allocated memory");
>> +	report_prefix_pop();
>> +
>> +	report(tmp != tmp2, "allocated memory addresses differ");
>>   
>>   	expect_pgm_int();
>>   	configure_dat(0);
>> @@ -62,6 +69,7 @@ static void test_malloc(void)
>>   
>>   	free(tmp);
>>   	free(tmp2);
>> +	report_prefix_pop();
>>   }
>>   
>>   int main(int argc, char**argv)
>>
> 
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [kvm-unit-tests PATCH] s390x: selftest: Fix report output
  2021-05-31 11:11 ` Cornelia Huck
@ 2021-05-31 12:09   ` Janosch Frank
  0 siblings, 0 replies; 6+ messages in thread
From: Janosch Frank @ 2021-05-31 12:09 UTC (permalink / raw)
  To: Cornelia Huck; +Cc: kvm, linux-s390, imbrenda, david, thuth

On 5/31/21 1:11 PM, Cornelia Huck wrote:
> On Mon, 31 May 2021 10:50:03 +0000
> Janosch Frank <frankja@linux.ibm.com> wrote:
> 
>> To make our TAP parser (and me) happy we don't want to have to reports
> 
> "we want to have two reports" ?
> 
> If that's not what has been intended, I'm confused :)

Things that happen if the following sentence is heard:
"Can you fix this quickly, please?"

Will fix, thanks for the review!

> 
>> with exactly the same wording.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>  s390x/selftest.c | 18 +++++++++++++-----
>>  1 file changed, 13 insertions(+), 5 deletions(-)
>>
>> diff --git a/s390x/selftest.c b/s390x/selftest.c
>> index b2fe2e7b..c2ca9896 100644
>> --- a/s390x/selftest.c
>> +++ b/s390x/selftest.c
>> @@ -47,12 +47,19 @@ static void test_malloc(void)
>>  	*tmp2 = 123456789;
>>  	mb();
>>  
>> -	report((uintptr_t)tmp & 0xf000000000000000ul, "malloc: got vaddr");
>> -	report(*tmp == 123456789, "malloc: access works");
>> +	report_prefix_push("malloc");
>> +	report_prefix_push("ptr_0");
>> +	report((uintptr_t)tmp & 0xf000000000000000ul, "allocated memory");
>> +	report(*tmp == 123456789, "wrote allocated memory");
>> +	report_prefix_pop();
>> +
>> +	report_prefix_push("ptr_1");
>>  	report((uintptr_t)tmp2 & 0xf000000000000000ul,
>> -	       "malloc: got 2nd vaddr");
>> -	report((*tmp2 == 123456789), "malloc: access works");
>> -	report(tmp != tmp2, "malloc: addresses differ");
>> +	       "allocated memory");
>> +	report((*tmp2 == 123456789), "wrote allocated memory");
>> +	report_prefix_pop();
>> +
>> +	report(tmp != tmp2, "allocated memory addresses differ");
>>  
>>  	expect_pgm_int();
>>  	configure_dat(0);
>> @@ -62,6 +69,7 @@ static void test_malloc(void)
>>  
>>  	free(tmp);
>>  	free(tmp2);
>> +	report_prefix_pop();
>>  }
>>  
>>  int main(int argc, char**argv)
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-05-31 12:09 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-31 10:50 [kvm-unit-tests PATCH] s390x: selftest: Fix report output Janosch Frank
2021-05-31 10:56 ` Claudio Imbrenda
2021-05-31 11:11 ` Cornelia Huck
2021-05-31 12:09   ` Janosch Frank
2021-05-31 11:15 ` David Hildenbrand
2021-05-31 12:07   ` Janosch Frank

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).