[LTP] [PATCH v1] swapping01.c: Add sleeps in the loop that dirties the memory

[LTP] [PATCH v1] swapping01.c: Add sleeps in the loop that dirties the memory

[Wei Gao via ltp]

In our test env this case failed sporadically, we suspect swapout speed not fast enough
in some situation.

Detail info please check following comments;
https://bugzilla.suse.com/show_bug.cgi?id=1217850#c34

Signed-off-by: Wei Gao <wegao@suse.com>
---
 testcases/kernel/mem/swapping/swapping01.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/testcases/kernel/mem/swapping/swapping01.c b/testcases/kernel/mem/swapping/swapping01.c
index a0030a75f..338881c8e 100644
--- a/testcases/kernel/mem/swapping/swapping01.c
+++ b/testcases/kernel/mem/swapping/swapping01.c
@@ -47,6 +47,7 @@
 #define COE_DELTA       1
 /* will try to alloc 1.3 * phy_mem */
 #define COE_SLIGHT_OVER 0.3
+#define MEM_SIZE 1024 * 1024
 
 static void init_meminfo(void);
 static void do_alloc(int allow_raise);
@@ -101,6 +102,13 @@ static void init_meminfo(void)
 				swap_free_init, mem_over_max);
 }
 
+static void memset_blocks(char *ptr, int mem_count, int sleep_time_ms) {
+	for (int i = 0; i < mem_count / 1024; i++) {
+		memset(ptr + (i * MEM_SIZE), 1, MEM_SIZE);
+		usleep(sleep_time_ms * 1000);
+	}
+}
+
 static void do_alloc(int allow_raise)
 {
 	long mem_count;
@@ -115,7 +123,7 @@ static void do_alloc(int allow_raise)
 	if (allow_raise == 1)
 		tst_res(TINFO, "try to allocate: %ld MB", mem_count / 1024);
 	s = SAFE_MALLOC(mem_count * 1024);
-	memset(s, 1, mem_count * 1024);
+	memset_blocks(s, mem_count, 10);
 
 	if ((allow_raise == 1) && (raise(SIGSTOP) == -1)) {
 		tst_res(TINFO, "memory allocated: %ld MB", mem_count / 1024);
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v1] swapping01.c: Add sleeps in the loop that dirties the memory

[Cyril Hrubis]

Hi!
> https://bugzilla.suse.com/show_bug.cgi?id=1217850#c34

If I'm counting right this is 10ms * 1024 which is 10s, I would be
happier with 1ms sleep instead. Or if we did 10ms sleep once every 10
memsets() instead.

Also since this was proposed by Vlastimil in the bugzilla he should
ideally give his ack here as well (now in CC).

> Signed-off-by: Wei Gao <wegao@suse.com>
> ---
>  testcases/kernel/mem/swapping/swapping01.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/testcases/kernel/mem/swapping/swapping01.c b/testcases/kernel/mem/swapping/swapping01.c
> index a0030a75f..338881c8e 100644
> --- a/testcases/kernel/mem/swapping/swapping01.c
> +++ b/testcases/kernel/mem/swapping/swapping01.c
> @@ -47,6 +47,7 @@
>  #define COE_DELTA       1
>  /* will try to alloc 1.3 * phy_mem */
>  #define COE_SLIGHT_OVER 0.3
> +#define MEM_SIZE 1024 * 1024
>  
>  static void init_meminfo(void);
>  static void do_alloc(int allow_raise);
> @@ -101,6 +102,13 @@ static void init_meminfo(void)
>  				swap_free_init, mem_over_max);
>  }
>  
> +static void memset_blocks(char *ptr, int mem_count, int sleep_time_ms) {
> +	for (int i = 0; i < mem_count / 1024; i++) {
> +		memset(ptr + (i * MEM_SIZE), 1, MEM_SIZE);
> +		usleep(sleep_time_ms * 1000);
> +	}
> +}
> +
>  static void do_alloc(int allow_raise)
>  {
>  	long mem_count;
> @@ -115,7 +123,7 @@ static void do_alloc(int allow_raise)
>  	if (allow_raise == 1)
>  		tst_res(TINFO, "try to allocate: %ld MB", mem_count / 1024);
>  	s = SAFE_MALLOC(mem_count * 1024);
> -	memset(s, 1, mem_count * 1024);
> +	memset_blocks(s, mem_count, 10);
>  
>  	if ((allow_raise == 1) && (raise(SIGSTOP) == -1)) {
>  		tst_res(TINFO, "memory allocated: %ld MB", mem_count / 1024);
> -- 
> 2.35.3
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v1] swapping01.c: Add sleeps in the loop that dirties the memory

[Vlastimil Babka via ltp]

On 4/18/24 6:34 PM, Cyril Hrubis wrote:
> Hi!
>> https://bugzilla.suse.com/show_bug.cgi?id=1217850#c34
> 
> If I'm counting right this is 10ms * 1024 which is 10s, I would be
> happier with 1ms sleep instead. Or if we did 10ms sleep once every 10
> memsets() instead.

Yeah I proposed you can try to reduce the sleep time/frequency to basically
to lowest value (with some margin) that resolves the issue reliably in your
testing. That 10ms per MB was just a possible starting point.

> Also since this was proposed by Vlastimil in the bugzilla he should
> ideally give his ack here as well (now in CC).
> 
>> Signed-off-by: Wei Gao <wegao@suse.com>

Acked-by: Vlastimil Babka <vbabka@suse.cz>

>> ---
>>  testcases/kernel/mem/swapping/swapping01.c | 10 +++++++++-
>>  1 file changed, 9 insertions(+), 1 deletion(-)
>> 
>> diff --git a/testcases/kernel/mem/swapping/swapping01.c b/testcases/kernel/mem/swapping/swapping01.c
>> index a0030a75f..338881c8e 100644
>> --- a/testcases/kernel/mem/swapping/swapping01.c
>> +++ b/testcases/kernel/mem/swapping/swapping01.c
>> @@ -47,6 +47,7 @@
>>  #define COE_DELTA       1
>>  /* will try to alloc 1.3 * phy_mem */
>>  #define COE_SLIGHT_OVER 0.3
>> +#define MEM_SIZE 1024 * 1024
>>  
>>  static void init_meminfo(void);
>>  static void do_alloc(int allow_raise);
>> @@ -101,6 +102,13 @@ static void init_meminfo(void)
>>  				swap_free_init, mem_over_max);
>>  }
>>  
>> +static void memset_blocks(char *ptr, int mem_count, int sleep_time_ms) {
>> +	for (int i = 0; i < mem_count / 1024; i++) {
>> +		memset(ptr + (i * MEM_SIZE), 1, MEM_SIZE);
>> +		usleep(sleep_time_ms * 1000);
>> +	}
>> +}
>> +
>>  static void do_alloc(int allow_raise)
>>  {
>>  	long mem_count;
>> @@ -115,7 +123,7 @@ static void do_alloc(int allow_raise)
>>  	if (allow_raise == 1)
>>  		tst_res(TINFO, "try to allocate: %ld MB", mem_count / 1024);
>>  	s = SAFE_MALLOC(mem_count * 1024);
>> -	memset(s, 1, mem_count * 1024);
>> +	memset_blocks(s, mem_count, 10);
>>  
>>  	if ((allow_raise == 1) && (raise(SIGSTOP) == -1)) {
>>  		tst_res(TINFO, "memory allocated: %ld MB", mem_count / 1024);
>> -- 
>> 2.35.3
>> 
>> 
>> -- 
>> Mailing list info: https://lists.linux.it/listinfo/ltp
> 


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v1] swapping01.c: Add sleeps in the loop that dirties the memory

[Cyril Hrubis]

Hi!
> Yeah I proposed you can try to reduce the sleep time/frequency to basically
> to lowest value (with some margin) that resolves the issue reliably in your
> testing. That 10ms per MB was just a possible starting point.

The usuall practice we do in LTP is to measure the minimal value
required for the test to pass and then double that for the final value.
But even with that I suppose that the value should be less than 10s.

Wei can you please try to measure how long are the shortest sleeps we
need in order to make the test pass?

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v1] swapping01.c: Add sleeps in the loop that dirties the memory

[Wei Gao via ltp]

On Fri, Apr 19, 2024 at 10:36:19AM +0200, Cyril Hrubis wrote:
> Hi!
> > Yeah I proposed you can try to reduce the sleep time/frequency to basically
> > to lowest value (with some margin) that resolves the issue reliably in your
> > testing. That 10ms per MB was just a possible starting point.
> 
> The usuall practice we do in LTP is to measure the minimal value
> required for the test to pass and then double that for the final value.
> But even with that I suppose that the value should be less than 10s.
> 
> Wei can you please try to measure how long are the shortest sleeps we
> need in order to make the test pass?
>

Run 500 times on same build for each measurement.

1) No any sleep, 17 case will failed
https://openqa.suse.de/tests/14111469#next_previous

2) sleep 1ms, no failed case happen
https://openqa.suse.de/tests/14110476#next_previous

3) sleep 0.5ms, 1 failed case happen 
https://openqa.suse.de/tests/14110476#next_previous

So currently 1ms is good candidate. I will sent second patch.


> -- 
> Cyril Hrubis
> chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2] swapping01.c: Add sleeps in the loop that dirties the memory

[Wei Gao via ltp]

In our test env this case failed sporadically, we suspect swapout speed not fast enough
in some situation.

Detail info please check following comments;
https://bugzilla.suse.com/show_bug.cgi?id=1217850#c34

Signed-off-by: Wei Gao <wegao@suse.com>
---
 testcases/kernel/mem/swapping/swapping01.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/testcases/kernel/mem/swapping/swapping01.c b/testcases/kernel/mem/swapping/swapping01.c
index a0030a75f..79dd2b4d5 100644
--- a/testcases/kernel/mem/swapping/swapping01.c
+++ b/testcases/kernel/mem/swapping/swapping01.c
@@ -47,6 +47,7 @@
 #define COE_DELTA       1
 /* will try to alloc 1.3 * phy_mem */
 #define COE_SLIGHT_OVER 0.3
+#define MEM_SIZE 1024 * 1024
 
 static void init_meminfo(void);
 static void do_alloc(int allow_raise);
@@ -101,6 +102,13 @@ static void init_meminfo(void)
 				swap_free_init, mem_over_max);
 }
 
+static void memset_blocks(char *ptr, int mem_count, int sleep_time_ms) {
+	for (int i = 0; i < mem_count / 1024; i++) {
+		memset(ptr + (i * MEM_SIZE), 1, MEM_SIZE);
+		usleep(sleep_time_ms * 1000);
+	}
+}
+
 static void do_alloc(int allow_raise)
 {
 	long mem_count;
@@ -115,7 +123,7 @@ static void do_alloc(int allow_raise)
 	if (allow_raise == 1)
 		tst_res(TINFO, "try to allocate: %ld MB", mem_count / 1024);
 	s = SAFE_MALLOC(mem_count * 1024);
-	memset(s, 1, mem_count * 1024);
+	memset_blocks(s, mem_count, 1);
 
 	if ((allow_raise == 1) && (raise(SIGSTOP) == -1)) {
 		tst_res(TINFO, "memory allocated: %ld MB", mem_count / 1024);
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2] swapping01.c: Add sleeps in the loop that dirties the memory

[Petr Vorel]

Hi Wei,

> In our test env this case failed sporadically, we suspect swapout speed not fast enough
> in some situation.

> Detail info please check following comments;
> https://bugzilla.suse.com/show_bug.cgi?id=1217850#c34

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2] swapping01.c: Add sleeps in the loop that dirties the memory

[Petr Vorel]

Hi Wei,

> +++ b/testcases/kernel/mem/swapping/swapping01.c
> @@ -47,6 +47,7 @@
>  #define COE_DELTA       1
>  /* will try to alloc 1.3 * phy_mem */
>  #define COE_SLIGHT_OVER 0.3
> +#define MEM_SIZE 1024 * 1024

>  static void init_meminfo(void);
>  static void do_alloc(int allow_raise);
> @@ -101,6 +102,13 @@ static void init_meminfo(void)
>  				swap_free_init, mem_over_max);
>  }

> +static void memset_blocks(char *ptr, int mem_count, int sleep_time_ms) {
> +	for (int i = 0; i < mem_count / 1024; i++) {
> +		memset(ptr + (i * MEM_SIZE), 1, MEM_SIZE);
> +		usleep(sleep_time_ms * 1000);
> +	}
> +}
> +
>  static void do_alloc(int allow_raise)
>  {
>  	long mem_count;
> @@ -115,7 +123,7 @@ static void do_alloc(int allow_raise)
>  	if (allow_raise == 1)
>  		tst_res(TINFO, "try to allocate: %ld MB", mem_count / 1024);
>  	s = SAFE_MALLOC(mem_count * 1024);
> -	memset(s, 1, mem_count * 1024);
> +	memset_blocks(s, mem_count, 1);
nit: Actually when we have custom function used on single place, we might want
to avoid the last parameter 1, right? We could usleep(1000) directly.

We use MEM_SIZE definition outside of the function anyway.

I can change it before merge:

diff --git testcases/kernel/mem/swapping/swapping01.c testcases/kernel/mem/swapping/swapping01.c
index 79dd2b4d5..09820e3d9 100644
--- testcases/kernel/mem/swapping/swapping01.c
+++ testcases/kernel/mem/swapping/swapping01.c
@@ -102,10 +102,10 @@ static void init_meminfo(void)
 				swap_free_init, mem_over_max);
 }
 
-static void memset_blocks(char *ptr, int mem_count, int sleep_time_ms) {
+static void memset_blocks(char *ptr, int mem_count) {
 	for (int i = 0; i < mem_count / 1024; i++) {
 		memset(ptr + (i * MEM_SIZE), 1, MEM_SIZE);
-		usleep(sleep_time_ms * 1000);
+		usleep(1000);
 	}
 }
 
@@ -123,7 +123,7 @@ static void do_alloc(int allow_raise)
 	if (allow_raise == 1)
 		tst_res(TINFO, "try to allocate: %ld MB", mem_count / 1024);
 	s = SAFE_MALLOC(mem_count * 1024);
-	memset_blocks(s, mem_count, 1);
+	memset_blocks(s, mem_count);
 
 	if ((allow_raise == 1) && (raise(SIGSTOP) == -1)) {
 		tst_res(TINFO, "memory allocated: %ld MB", mem_count / 1024);

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2] swapping01.c: Add sleeps in the loop that dirties the memory

[Cyril Hrubis]

Hi!
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2] swapping01.c: Add sleeps in the loop that dirties the memory

[Petr Vorel]

Hi Wei, Cyril,

> Hi!
> Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

Thanks, merged the original version.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp