* [Qemu-devel] [PATCH v2 0/3] some fix in tests/migration
@ 2019-09-11 3:31 Mao Zhongyi
2019-09-11 3:31 ` [Qemu-devel] [PATCH v2 1/3] tests/migration: mem leak fix Mao Zhongyi
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Mao Zhongyi @ 2019-09-11 3:31 UTC (permalink / raw)
To: qemu-devel; +Cc: tony.nguyen, alex.bennee, armbru, Mao Zhongyi, laurent
This patchset mainly fixes memory leak, typo and return
value of stress function in stress test.
v2->v1:
- use g_autofree to release memory automatically instead
of free(). [Alex Bennée]
Cc: armbru@redhat.com
Cc: laurent@vivier.eu
Cc: tony.nguyen@bt.com
Cc: alex.bennee@linaro.org
Mao Zhongyi (3):
tests/migration: mem leak fix
tests/migration: fix a typo in comment
tests/migration:fix unreachable path in stress test
tests/migration/stress.c | 17 +++++------------
1 file changed, 5 insertions(+), 12 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH v2 1/3] tests/migration: mem leak fix
2019-09-11 3:31 [Qemu-devel] [PATCH v2 0/3] some fix in tests/migration Mao Zhongyi
@ 2019-09-11 3:31 ` Mao Zhongyi
2019-09-11 8:21 ` Alex Bennée
2019-10-01 15:31 ` Laurent Vivier
2019-09-11 3:31 ` [Qemu-devel] [PATCH v2 2/3] tests/migration: fix a typo in comment Mao Zhongyi
2019-09-11 3:31 ` [Qemu-devel] [PATCH v2 3/3] tests/migration:fix unreachable path in stress test Mao Zhongyi
2 siblings, 2 replies; 14+ messages in thread
From: Mao Zhongyi @ 2019-09-11 3:31 UTC (permalink / raw)
To: qemu-devel; +Cc: tony.nguyen, alex.bennee, armbru, Mao Zhongyi, laurent
‘data’ has the possibility of memory leaks, so use the
glic macros g_autofree recommended by CODING_STYLE.rst
to automatically release the memory that returned from
g_malloc().
Cc: armbru@redhat.com
Cc: laurent@vivier.eu
Cc: tony.nguyen@bt.com
Cc: alex.bennee@linaro.org
Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
---
tests/migration/stress.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/tests/migration/stress.c b/tests/migration/stress.c
index d9aa4afe92..6cbb2d49d3 100644
--- a/tests/migration/stress.c
+++ b/tests/migration/stress.c
@@ -170,10 +170,10 @@ static unsigned long long now(void)
static int stressone(unsigned long long ramsizeMB)
{
size_t pagesPerMB = 1024 * 1024 / PAGE_SIZE;
- char *ram = malloc(ramsizeMB * 1024 * 1024);
+ g_autofree char *ram = malloc(ramsizeMB * 1024 * 1024);
char *ramptr;
size_t i, j, k;
- char *data = malloc(PAGE_SIZE);
+ g_autofree char *data = malloc(PAGE_SIZE);
char *dataptr;
size_t nMB = 0;
unsigned long long before, after;
@@ -186,7 +186,6 @@ static int stressone(unsigned long long ramsizeMB)
if (!data) {
fprintf(stderr, "%s (%d): ERROR: cannot allocate %d bytes of RAM: %s\n",
argv0, gettid(), PAGE_SIZE, strerror(errno));
- free(ram);
return -1;
}
@@ -198,8 +197,6 @@ static int stressone(unsigned long long ramsizeMB)
memset(ram, 0xfe, ramsizeMB * 1024 * 1024);
if (random_bytes(data, PAGE_SIZE) < 0) {
- free(ram);
- free(data);
return -1;
}
@@ -227,9 +224,6 @@ static int stressone(unsigned long long ramsizeMB)
}
}
}
-
- free(data);
- free(ram);
}
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH v2 2/3] tests/migration: fix a typo in comment
2019-09-11 3:31 [Qemu-devel] [PATCH v2 0/3] some fix in tests/migration Mao Zhongyi
2019-09-11 3:31 ` [Qemu-devel] [PATCH v2 1/3] tests/migration: mem leak fix Mao Zhongyi
@ 2019-09-11 3:31 ` Mao Zhongyi
2019-10-01 15:31 ` Laurent Vivier
2019-10-04 14:23 ` [Qemu-devel] " Philippe Mathieu-Daudé
2019-09-11 3:31 ` [Qemu-devel] [PATCH v2 3/3] tests/migration:fix unreachable path in stress test Mao Zhongyi
2 siblings, 2 replies; 14+ messages in thread
From: Mao Zhongyi @ 2019-09-11 3:31 UTC (permalink / raw)
To: qemu-devel; +Cc: tony.nguyen, alex.bennee, armbru, Mao Zhongyi, laurent
Cc: armbru@redhat.com
Cc: laurent@vivier.eu
Cc: tony.nguyen@bt.com
Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---
tests/migration/stress.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/migration/stress.c b/tests/migration/stress.c
index 6cbb2d49d3..19a6eff5fd 100644
--- a/tests/migration/stress.c
+++ b/tests/migration/stress.c
@@ -191,7 +191,7 @@ static int stressone(unsigned long long ramsizeMB)
/* We don't care about initial state, but we do want
* to fault it all into RAM, otherwise the first iter
- * of the loop below will be quite slow. We cna't use
+ * of the loop below will be quite slow. We can't use
* 0x0 as the byte as gcc optimizes that away into a
* calloc instead :-) */
memset(ram, 0xfe, ramsizeMB * 1024 * 1024);
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH v2 3/3] tests/migration:fix unreachable path in stress test
2019-09-11 3:31 [Qemu-devel] [PATCH v2 0/3] some fix in tests/migration Mao Zhongyi
2019-09-11 3:31 ` [Qemu-devel] [PATCH v2 1/3] tests/migration: mem leak fix Mao Zhongyi
2019-09-11 3:31 ` [Qemu-devel] [PATCH v2 2/3] tests/migration: fix a typo in comment Mao Zhongyi
@ 2019-09-11 3:31 ` Mao Zhongyi
2019-10-01 15:22 ` maozy
2019-10-01 15:46 ` Laurent Vivier
2 siblings, 2 replies; 14+ messages in thread
From: Mao Zhongyi @ 2019-09-11 3:31 UTC (permalink / raw)
To: qemu-devel; +Cc: tony.nguyen, alex.bennee, armbru, Mao Zhongyi, laurent
if stress function always return 0, the path
'if (stress(ramsizeGB, ncpus) < 0)' is nerver unreachable,
so fix it to allow the test failed.
Cc: armbru@redhat.com
Cc: laurent@vivier.eu
Cc: tony.nguyen@bt.com
Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
---
tests/migration/stress.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/tests/migration/stress.c b/tests/migration/stress.c
index 19a6eff5fd..35903d90c4 100644
--- a/tests/migration/stress.c
+++ b/tests/migration/stress.c
@@ -224,6 +224,7 @@ static int stressone(unsigned long long ramsizeMB)
}
}
}
+ return 0;
}
@@ -248,9 +249,7 @@ static int stress(unsigned long long ramsizeGB, int ncpus)
stressthread, &ramsizeMB);
}
- stressone(ramsizeMB);
-
- return 0;
+ return stressone(ramsizeMB);
}
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/3] tests/migration: mem leak fix
2019-09-11 3:31 ` [Qemu-devel] [PATCH v2 1/3] tests/migration: mem leak fix Mao Zhongyi
@ 2019-09-11 8:21 ` Alex Bennée
2019-10-01 15:31 ` Laurent Vivier
1 sibling, 0 replies; 14+ messages in thread
From: Alex Bennée @ 2019-09-11 8:21 UTC (permalink / raw)
To: Mao Zhongyi; +Cc: laurent, tony.nguyen, qemu-devel, armbru
Mao Zhongyi <maozhongyi@cmss.chinamobile.com> writes:
> ‘data’ has the possibility of memory leaks, so use the
> glic macros g_autofree recommended by CODING_STYLE.rst
nit: glib
> to automatically release the memory that returned from
> g_malloc().
>
> Cc: armbru@redhat.com
> Cc: laurent@vivier.eu
> Cc: tony.nguyen@bt.com
> Cc: alex.bennee@linaro.org
>
> Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> tests/migration/stress.c | 10 ++--------
> 1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/tests/migration/stress.c b/tests/migration/stress.c
> index d9aa4afe92..6cbb2d49d3 100644
> --- a/tests/migration/stress.c
> +++ b/tests/migration/stress.c
> @@ -170,10 +170,10 @@ static unsigned long long now(void)
> static int stressone(unsigned long long ramsizeMB)
> {
> size_t pagesPerMB = 1024 * 1024 / PAGE_SIZE;
> - char *ram = malloc(ramsizeMB * 1024 * 1024);
> + g_autofree char *ram = malloc(ramsizeMB * 1024 * 1024);
> char *ramptr;
> size_t i, j, k;
> - char *data = malloc(PAGE_SIZE);
> + g_autofree char *data = malloc(PAGE_SIZE);
> char *dataptr;
> size_t nMB = 0;
> unsigned long long before, after;
> @@ -186,7 +186,6 @@ static int stressone(unsigned long long ramsizeMB)
> if (!data) {
> fprintf(stderr, "%s (%d): ERROR: cannot allocate %d bytes of RAM: %s\n",
> argv0, gettid(), PAGE_SIZE, strerror(errno));
> - free(ram);
> return -1;
> }
>
> @@ -198,8 +197,6 @@ static int stressone(unsigned long long ramsizeMB)
> memset(ram, 0xfe, ramsizeMB * 1024 * 1024);
>
> if (random_bytes(data, PAGE_SIZE) < 0) {
> - free(ram);
> - free(data);
> return -1;
> }
>
> @@ -227,9 +224,6 @@ static int stressone(unsigned long long ramsizeMB)
> }
> }
> }
> -
> - free(data);
> - free(ram);
> }
--
Alex Bennée
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 3/3] tests/migration:fix unreachable path in stress test
2019-09-11 3:31 ` [Qemu-devel] [PATCH v2 3/3] tests/migration:fix unreachable path in stress test Mao Zhongyi
@ 2019-10-01 15:22 ` maozy
2019-10-01 15:46 ` Laurent Vivier
1 sibling, 0 replies; 14+ messages in thread
From: maozy @ 2019-10-01 15:22 UTC (permalink / raw)
To: qemu-devel; +Cc: tony.nguyen, alex.bennee, armbru, laurent
ping...
On 9/11/19 11:31 AM, Mao Zhongyi wrote:
> if stress function always return 0, the path
> 'if (stress(ramsizeGB, ncpus) < 0)' is nerver unreachable,
> so fix it to allow the test failed.
>
> Cc: armbru@redhat.com
> Cc: laurent@vivier.eu
> Cc: tony.nguyen@bt.com
>
> Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
> ---
> tests/migration/stress.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/tests/migration/stress.c b/tests/migration/stress.c
> index 19a6eff5fd..35903d90c4 100644
> --- a/tests/migration/stress.c
> +++ b/tests/migration/stress.c
> @@ -224,6 +224,7 @@ static int stressone(unsigned long long ramsizeMB)
> }
> }
> }
> + return 0;
> }
>
>
> @@ -248,9 +249,7 @@ static int stress(unsigned long long ramsizeGB, int ncpus)
> stressthread, &ramsizeMB);
> }
>
> - stressone(ramsizeMB);
> -
> - return 0;
> + return stressone(ramsizeMB);
> }
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/3] tests/migration: mem leak fix
2019-09-11 3:31 ` [Qemu-devel] [PATCH v2 1/3] tests/migration: mem leak fix Mao Zhongyi
2019-09-11 8:21 ` Alex Bennée
@ 2019-10-01 15:31 ` Laurent Vivier
2019-10-03 5:34 ` maozy
1 sibling, 1 reply; 14+ messages in thread
From: Laurent Vivier @ 2019-10-01 15:31 UTC (permalink / raw)
To: Mao Zhongyi, qemu-devel; +Cc: tony.nguyen, alex.bennee, armbru
Le 11/09/2019 à 05:31, Mao Zhongyi a écrit :
> ‘data’ has the possibility of memory leaks, so use the
> glic macros g_autofree recommended by CODING_STYLE.rst
> to automatically release the memory that returned from
> g_malloc().
>
> Cc: armbru@redhat.com
> Cc: laurent@vivier.eu
> Cc: tony.nguyen@bt.com
> Cc: alex.bennee@linaro.org
>
> Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
> ---
> tests/migration/stress.c | 10 ++--------
> 1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/tests/migration/stress.c b/tests/migration/stress.c
> index d9aa4afe92..6cbb2d49d3 100644
> --- a/tests/migration/stress.c
> +++ b/tests/migration/stress.c
> @@ -170,10 +170,10 @@ static unsigned long long now(void)
> static int stressone(unsigned long long ramsizeMB)
> {
> size_t pagesPerMB = 1024 * 1024 / PAGE_SIZE;
> - char *ram = malloc(ramsizeMB * 1024 * 1024);
> + g_autofree char *ram = malloc(ramsizeMB * 1024 * 1024);
> char *ramptr;
> size_t i, j, k;
> - char *data = malloc(PAGE_SIZE);
> + g_autofree char *data = malloc(PAGE_SIZE);
So perhaps g_malloc() could be a better choice as it will exit on
allocation failure?
Thanks,
Laurent
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/3] tests/migration: fix a typo in comment
2019-09-11 3:31 ` [Qemu-devel] [PATCH v2 2/3] tests/migration: fix a typo in comment Mao Zhongyi
@ 2019-10-01 15:31 ` Laurent Vivier
2019-10-04 14:23 ` [Qemu-devel] " Philippe Mathieu-Daudé
1 sibling, 0 replies; 14+ messages in thread
From: Laurent Vivier @ 2019-10-01 15:31 UTC (permalink / raw)
To: Mao Zhongyi, qemu-devel; +Cc: tony.nguyen, alex.bennee, armbru
Le 11/09/2019 à 05:31, Mao Zhongyi a écrit :
> Cc: armbru@redhat.com
> Cc: laurent@vivier.eu
> Cc: tony.nguyen@bt.com
>
> Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> tests/migration/stress.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/migration/stress.c b/tests/migration/stress.c
> index 6cbb2d49d3..19a6eff5fd 100644
> --- a/tests/migration/stress.c
> +++ b/tests/migration/stress.c
> @@ -191,7 +191,7 @@ static int stressone(unsigned long long ramsizeMB)
>
> /* We don't care about initial state, but we do want
> * to fault it all into RAM, otherwise the first iter
> - * of the loop below will be quite slow. We cna't use
> + * of the loop below will be quite slow. We can't use
> * 0x0 as the byte as gcc optimizes that away into a
> * calloc instead :-) */
> memset(ram, 0xfe, ramsizeMB * 1024 * 1024);
>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 3/3] tests/migration:fix unreachable path in stress test
2019-09-11 3:31 ` [Qemu-devel] [PATCH v2 3/3] tests/migration:fix unreachable path in stress test Mao Zhongyi
2019-10-01 15:22 ` maozy
@ 2019-10-01 15:46 ` Laurent Vivier
2019-10-03 7:17 ` _[PATCH_v2_3/3]_tests/migration:fix_unreachable_path_in_stress_test maozy
1 sibling, 1 reply; 14+ messages in thread
From: Laurent Vivier @ 2019-10-01 15:46 UTC (permalink / raw)
To: Mao Zhongyi, qemu-devel; +Cc: tony.nguyen, alex.bennee, armbru
Le 11/09/2019 à 05:31, Mao Zhongyi a écrit :
> if stress function always return 0, the path
> 'if (stress(ramsizeGB, ncpus) < 0)' is nerver unreachable,
> so fix it to allow the test failed.
>
> Cc: armbru@redhat.com
> Cc: laurent@vivier.eu
> Cc: tony.nguyen@bt.com
>
> Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
> ---
> tests/migration/stress.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/tests/migration/stress.c b/tests/migration/stress.c
> index 19a6eff5fd..35903d90c4 100644
> --- a/tests/migration/stress.c
> +++ b/tests/migration/stress.c
> @@ -224,6 +224,7 @@ static int stressone(unsigned long long ramsizeMB)
> }
> }
> }
> + return 0;
> }
before the return, we have an infinite loop "while(1) { }".
So this part is dead code.
In fact, if the function exits, it's because it fails, otherwise it
loops infinitely, so I think we should change its type to void and
stress should always return -1.
Thanks,
Laurent
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/3] tests/migration: mem leak fix
2019-10-01 15:31 ` Laurent Vivier
@ 2019-10-03 5:34 ` maozy
0 siblings, 0 replies; 14+ messages in thread
From: maozy @ 2019-10-03 5:34 UTC (permalink / raw)
To: Laurent Vivier, qemu-devel; +Cc: tony.nguyen, alex.bennee, armbru
On 10/1/19 11:31 PM, Laurent Vivier wrote:
> Le 11/09/2019 à 05:31, Mao Zhongyi a écrit :
>> ‘data’ has the possibility of memory leaks, so use the
>> glic macros g_autofree recommended by CODING_STYLE.rst
>> to automatically release the memory that returned from
>> g_malloc().
>>
>> Cc: armbru@redhat.com
>> Cc: laurent@vivier.eu
>> Cc: tony.nguyen@bt.com
>> Cc: alex.bennee@linaro.org
>>
>> Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
>> ---
>> tests/migration/stress.c | 10 ++--------
>> 1 file changed, 2 insertions(+), 8 deletions(-)
>>
>> diff --git a/tests/migration/stress.c b/tests/migration/stress.c
>> index d9aa4afe92..6cbb2d49d3 100644
>> --- a/tests/migration/stress.c
>> +++ b/tests/migration/stress.c
>> @@ -170,10 +170,10 @@ static unsigned long long now(void)
>> static int stressone(unsigned long long ramsizeMB)
>> {
>> size_t pagesPerMB = 1024 * 1024 / PAGE_SIZE;
>> - char *ram = malloc(ramsizeMB * 1024 * 1024);
>> + g_autofree char *ram = malloc(ramsizeMB * 1024 * 1024);
>> char *ramptr;
>> size_t i, j, k;
>> - char *data = malloc(PAGE_SIZE);
>> + g_autofree char *data = malloc(PAGE_SIZE);
> So perhaps g_malloc() could be a better choice as it will exit on
> allocation failure?
right, will fix it in next.
Thanks,
Mao
>
> Thanks,
> Laurent
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: _[PATCH_v2_3/3]_tests/migration:fix_unreachable_path_in_stress_test
2019-10-01 15:46 ` Laurent Vivier
@ 2019-10-03 7:17 ` maozy
2019-10-03 9:23 ` _[PATCH_v2_3/3]_tests/migration:fix_unreachable_path_in_stress_test Laurent Vivier
0 siblings, 1 reply; 14+ messages in thread
From: maozy @ 2019-10-03 7:17 UTC (permalink / raw)
To: Laurent Vivier, qemu-devel; +Cc: tony.nguyen, alex.bennee, armbru
Hi, Laurent
On 10/1/19 11:46 PM, Laurent Vivier wrote:
> Le 11/09/2019 à 05:31, Mao Zhongyi a écrit :
>> if stress function always return 0, the path
>> 'if (stress(ramsizeGB, ncpus) < 0)' is nerver unreachable,
>> so fix it to allow the test failed.
>>
>> Cc: armbru@redhat.com
>> Cc: laurent@vivier.eu
>> Cc: tony.nguyen@bt.com
>>
>> Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
>> ---
>> tests/migration/stress.c | 5 ++---
>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/tests/migration/stress.c b/tests/migration/stress.c
>> index 19a6eff5fd..35903d90c4 100644
>> --- a/tests/migration/stress.c
>> +++ b/tests/migration/stress.c
>> @@ -224,6 +224,7 @@ static int stressone(unsigned long long ramsizeMB)
>> }
>> }
>> }
>> + return 0;
>> }
> before the return, we have an infinite loop "while(1) { }".
>
> So this part is dead code.
>
> In fact, if the function exits, it's because it fails, otherwise it
> loops infinitely, so I think we should change its type to void and
> stress should always return -1.
Yes, I think it's ok to change stressone typo to void because
no one cares about its return value, but if make stress always
return -1, main will always exited in exit_failure, like this:
...
if (stress(ramsizeGB, ncpus) < 0)
exit_failure();
exit_success();
}
so, perhaps also change stress typo to void may be good. then:
...
stress(ramsizeGB, ncpus);
exit_success();
}
Anther way , make stressone return 0 when infinite loop fails to
exit, then main can handle both success and failure case.
what do you think?
Thanks,
Mao
> Thanks,
> Laurent
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: _[PATCH_v2_3/3]_tests/migration:fix_unreachable_path_in_stress_test
2019-10-03 7:17 ` _[PATCH_v2_3/3]_tests/migration:fix_unreachable_path_in_stress_test maozy
@ 2019-10-03 9:23 ` Laurent Vivier
2019-10-04 1:59 ` _[PATCH_v2_3/3]_tests/migration:fix_unreachable_path_in_stress_test maozy
0 siblings, 1 reply; 14+ messages in thread
From: Laurent Vivier @ 2019-10-03 9:23 UTC (permalink / raw)
To: maozy, qemu-devel; +Cc: tony.nguyen, alex.bennee, armbru
Le 03/10/2019 à 09:17, maozy a écrit :
> Hi, Laurent
>
> On 10/1/19 11:46 PM, Laurent Vivier wrote:
>> Le 11/09/2019 à 05:31, Mao Zhongyi a écrit :
>>> if stress function always return 0, the path
>>> 'if (stress(ramsizeGB, ncpus) < 0)' is nerver unreachable,
>>> so fix it to allow the test failed.
>>>
>>> Cc: armbru@redhat.com
>>> Cc: laurent@vivier.eu
>>> Cc: tony.nguyen@bt.com
>>>
>>> Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
>>> ---
>>> tests/migration/stress.c | 5 ++---
>>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/tests/migration/stress.c b/tests/migration/stress.c
>>> index 19a6eff5fd..35903d90c4 100644
>>> --- a/tests/migration/stress.c
>>> +++ b/tests/migration/stress.c
>>> @@ -224,6 +224,7 @@ static int stressone(unsigned long long ramsizeMB)
>>> }
>>> }
>>> }
>>> + return 0;
>>> }
>> before the return, we have an infinite loop "while(1) { }".
>>
>> So this part is dead code.
>>
>> In fact, if the function exits, it's because it fails, otherwise it
>> loops infinitely, so I think we should change its type to void and
>> stress should always return -1.
> Yes, I think it's ok to change stressone typo to void because
> no one cares about its return value, but if make stress always
> return -1, main will always exited in exit_failure, like this:
>
> ...
> if (stress(ramsizeGB, ncpus) < 0)
> exit_failure();
>
> exit_success();
> }
>
> so, perhaps also change stress typo to void may be good. then:
>
> ...
> stress(ramsizeGB, ncpus);
>
> exit_success();
> }
>
> Anther way , make stressone return 0 when infinite loop fails to
> exit, then main can handle both success and failure case.
>
> what do you think?
>
If stressone() or stress() exits it's because of a failure because the
test runs forever otherwise.
So I think there is no problem to use exit_failure() as the exit
function of main(): it should never be reached if the test runs without
error.
Thanks,
Laurent
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: _[PATCH_v2_3/3]_tests/migration:fix_unreachable_path_in_stress_test
2019-10-03 9:23 ` _[PATCH_v2_3/3]_tests/migration:fix_unreachable_path_in_stress_test Laurent Vivier
@ 2019-10-04 1:59 ` maozy
0 siblings, 0 replies; 14+ messages in thread
From: maozy @ 2019-10-04 1:59 UTC (permalink / raw)
To: Laurent Vivier, qemu-devel; +Cc: tony.nguyen, alex.bennee, armbru
On 10/3/19 5:23 PM, Laurent Vivier wrote:
> Le 03/10/2019 à 09:17, maozy a écrit :
>> Hi, Laurent
>>
>> On 10/1/19 11:46 PM, Laurent Vivier wrote:
>>> Le 11/09/2019 à 05:31, Mao Zhongyi a écrit :
>>>> if stress function always return 0, the path
>>>> 'if (stress(ramsizeGB, ncpus) < 0)' is nerver unreachable,
>>>> so fix it to allow the test failed.
>>>>
>>>> Cc: armbru@redhat.com
>>>> Cc: laurent@vivier.eu
>>>> Cc: tony.nguyen@bt.com
>>>>
>>>> Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
>>>> ---
>>>> tests/migration/stress.c | 5 ++---
>>>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/tests/migration/stress.c b/tests/migration/stress.c
>>>> index 19a6eff5fd..35903d90c4 100644
>>>> --- a/tests/migration/stress.c
>>>> +++ b/tests/migration/stress.c
>>>> @@ -224,6 +224,7 @@ static int stressone(unsigned long long ramsizeMB)
>>>> }
>>>> }
>>>> }
>>>> + return 0;
>>>> }
>>> before the return, we have an infinite loop "while(1) { }".
>>>
>>> So this part is dead code.
>>>
>>> In fact, if the function exits, it's because it fails, otherwise it
>>> loops infinitely, so I think we should change its type to void and
>>> stress should always return -1.
>> Yes, I think it's ok to change stressone typo to void because
>> no one cares about its return value, but if make stress always
>> return -1, main will always exited in exit_failure, like this:
>>
>> ...
>> if (stress(ramsizeGB, ncpus) < 0)
>> exit_failure();
>>
>> exit_success();
>> }
>>
>> so, perhaps also change stress typo to void may be good. then:
>>
>> ...
>> stress(ramsizeGB, ncpus);
>>
>> exit_success();
>> }
>>
>> Anther way , make stressone return 0 when infinite loop fails to
>> exit, then main can handle both success and failure case.
>>
>> what do you think?
>>
>
> If stressone() or stress() exits it's because of a failure because the
> test runs forever otherwise.
>
> So I think there is no problem to use exit_failure() as the exit
> function of main(): it should never be reached if the test runs without
> error.
OK, I see, thanks a lot.
>
> Thanks,
> Laurent
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/3] tests/migration: fix a typo in comment
2019-09-11 3:31 ` [Qemu-devel] [PATCH v2 2/3] tests/migration: fix a typo in comment Mao Zhongyi
2019-10-01 15:31 ` Laurent Vivier
@ 2019-10-04 14:23 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-10-04 14:23 UTC (permalink / raw)
To: Mao Zhongyi, qemu-devel; +Cc: tony.nguyen, alex.bennee, armbru, laurent
On 9/11/19 5:31 AM, Mao Zhongyi wrote:
> Cc: armbru@redhat.com
> Cc: laurent@vivier.eu
> Cc: tony.nguyen@bt.com
>
> Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> tests/migration/stress.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/migration/stress.c b/tests/migration/stress.c
> index 6cbb2d49d3..19a6eff5fd 100644
> --- a/tests/migration/stress.c
> +++ b/tests/migration/stress.c
> @@ -191,7 +191,7 @@ static int stressone(unsigned long long ramsizeMB)
>
> /* We don't care about initial state, but we do want
> * to fault it all into RAM, otherwise the first iter
> - * of the loop below will be quite slow. We cna't use
> + * of the loop below will be quite slow. We can't use
> * 0x0 as the byte as gcc optimizes that away into a
> * calloc instead :-) */
> memset(ram, 0xfe, ramsizeMB * 1024 * 1024);
>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-10-04 14:24 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-11 3:31 [Qemu-devel] [PATCH v2 0/3] some fix in tests/migration Mao Zhongyi
2019-09-11 3:31 ` [Qemu-devel] [PATCH v2 1/3] tests/migration: mem leak fix Mao Zhongyi
2019-09-11 8:21 ` Alex Bennée
2019-10-01 15:31 ` Laurent Vivier
2019-10-03 5:34 ` maozy
2019-09-11 3:31 ` [Qemu-devel] [PATCH v2 2/3] tests/migration: fix a typo in comment Mao Zhongyi
2019-10-01 15:31 ` Laurent Vivier
2019-10-04 14:23 ` [Qemu-devel] " Philippe Mathieu-Daudé
2019-09-11 3:31 ` [Qemu-devel] [PATCH v2 3/3] tests/migration:fix unreachable path in stress test Mao Zhongyi
2019-10-01 15:22 ` maozy
2019-10-01 15:46 ` Laurent Vivier
2019-10-03 7:17 ` _[PATCH_v2_3/3]_tests/migration:fix_unreachable_path_in_stress_test maozy
2019-10-03 9:23 ` _[PATCH_v2_3/3]_tests/migration:fix_unreachable_path_in_stress_test Laurent Vivier
2019-10-04 1:59 ` _[PATCH_v2_3/3]_tests/migration:fix_unreachable_path_in_stress_test maozy
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.