Skip to contentSkip to conversation

swapping: replace mem_free by mem_available

PATCH New Li Wang 2 participants 6 messages Jul 20, 2016 – Aug 1, 2016

lore
Share & tools
lore

Li Wang

author
#0 lore raw

PATCH 1/1 swapping: replace mem_free by mem_available

On some ppc64 systems, there free memory are larger than available memory, and swap size is very small. Then this swapping01 easily get failures like:

swapping01    0  TINFO  :  free physical memory: 14651 MB
swapping01    0  TINFO  :  try to allocate: 19046 MB
swapping01    1  TBROK  :  swapping01.c:134: malloc: errno=ENOMEM(12): Cannot allocate memory
swapping01    2  TBROK  :  swapping01.c:134: Remaining cases broken
swapping01    1  TBROK  :  swapping01.c:151: child was not stopped.
swapping01    2  TBROK  :  swapping01.c:151: Remaining cases broken
 # free -m
               total        used        free      shared  buff/cache   available
 Mem:          15316         238       14651           0         427       14478
 Swap:          4607         202        4405

That's because 14478(available_mem) + 4405(swap_free) < 19046(expected: 14651(free_mem) * 1.3), so we get malloc ENOMEM errors.

This patch replaces the free memory by available, and clean up some code.

---
 testcases/kernel/mem/swapping/swapping01.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)
testcases/kernel/mem/swapping/swapping01.c +14 -14
index b530ee2..caadb19 100644
@@ -68,7 +68,7 @@ static void init_meminfo(void);
 static void do_alloc(void);
 static void check_swapping(void);
 
-static long mem_free_init;
+static long mem_available_init;
 static long swap_free_init;
 static long mem_over;
 static long mem_over_max;
@@ -108,17 +108,17 @@ int main(int argc, char *argv[])
 static void init_meminfo(void)
 {
 	swap_free_init = read_meminfo("SwapFree:");
-	mem_free_init = read_meminfo("MemFree:");
-	mem_over = mem_free_init * COE_SLIGHT_OVER;
-	mem_over_max = mem_free_init * COE_DELTA;
-
-	/* at least 10MB free physical memory needed */
-	if (mem_free_init < 10240) {
-		sleep(5);
-		if (mem_free_init < 10240)
+	if (!FILE_LINES_SCANF(cleanup, "/proc/meminfo", "MemAvailable: %ld",
+				&mem_available_init))
+		mem_available_init = read_meminfo("MemFree:") + read_meminfo("Cached:");
+	mem_over = mem_available_init * COE_SLIGHT_OVER;
+	mem_over_max = mem_available_init * COE_DELTA;
+
+	/*@least 10MB available physical memory needed */
+	if (mem_available_init < 10240)
 			tst_brkm(TCONF, cleanup,
-				 "Not enough free memory to test.");
-	}
+				 "Not enough avalable memory to test.");
+
 	if (swap_free_init < mem_over)
 		tst_brkm(TCONF, cleanup, "Not enough swap space to test.");
 }
@@ -128,8 +128,8 @@ static void do_alloc(void)
 	long mem_count;
 	void *s;
 
-	tst_resm(TINFO, "free physical memory: %ld MB", mem_free_init / 1024);
-	mem_count = mem_free_init + mem_over;
+	tst_resm(TINFO, "available physical memory: %ld MB", mem_available_init / 1024);
+	mem_count = mem_available_init + mem_over;
 	tst_resm(TINFO, "try to allocate: %ld MB", mem_count / 1024);
 	s = malloc(mem_count * 1024);
 	if (s == NULL)
@@ -160,7 +160,7 @@ static void check_swapping(void)
 			tst_brkm(TFAIL, cleanup, "heavy swapping detected: "
 				 "%ld MB swapped.", swapped / 1024);
 		}
-		sleep(1);
+		usleep(100000);
 	}
 	tst_resm(TPASS, "no heavy swapping detected, %ld MB swapped.",
 		 swapped / 1024);
Signature · 1 line
1.8.3.1
author
#1 lore raw

PATCH 1/1 swapping: replace mem_free by mem_available

testcases/kernel/mem/swapping/swapping01.c int main(int argc, char *argv[]) · line 112

Quoted 4 lines · Li Wang
testcases/kernel/mem/swapping/swapping01.c +2 -2
-		sleep(5);
-		if (mem_free_init < 10240)
+	if (!FILE_LINES_SCANF(cleanup, "/proc/meminfo", "MemAvailable: %ld",
+				&mem_available_init))

slight correction: the "!" is redundant. That should be like:

if (FILE_LINES_SCANF(cleanup, "/proc/meminfo", "MemAvailable: %ld",
&mem_available_init))

Regards,
Li Wang

#2 lore raw

PATCH 1/1 swapping: replace mem_free by mem_available

In reply to Li Wang

----- Original Message -----

Quoted 66 lines

From: "Li Wang" <[email protected]>
To: [email protected]
Sent: Wednesday, 20 July, 2016 9:48:16 AM
Subject: [LTP] [PATCH] swapping: replace mem_free by mem_available

On some ppc64 systems, there free memory are larger than available memory, and
swap size is very small. Then this swapping01 easily get failures like:

swapping01    0  TINFO  :  free physical memory: 14651 MB
swapping01    0  TINFO  :  try to allocate: 19046 MB
swapping01    1  TBROK  :  swapping01.c:134: malloc: errno=ENOMEM(12): Cannot
allocate memory
swapping01    2  TBROK  :  swapping01.c:134: Remaining cases broken
swapping01    1  TBROK  :  swapping01.c:151: child was not stopped.
swapping01    2  TBROK  :  swapping01.c:151: Remaining cases broken
 # free -m
               total        used        free      shared  buff/cache
               available
 Mem:          15316         238       14651           0         427
 14478
 Swap:          4607         202        4405

That's because 14478(available_mem) + 4405(swap_free) < 19046(expected:
14651(free_mem) * 1.3),
so we get malloc ENOMEM errors.

This patch replaces the free memory by available, and clean up some code.

---
 testcases/kernel/mem/swapping/swapping01.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)
testcases/kernel/mem/swapping/swapping01.c +6 -9
index b530ee2..caadb19 100644
@@ -68,7 +68,7 @@ static void init_meminfo(void);
 static void do_alloc(void);
 static void check_swapping(void);
 
-static long mem_free_init;
+static long mem_available_init;
 static long swap_free_init;
 static long mem_over;
 static long mem_over_max;
@@ -108,17 +108,17 @@ int main(int argc, char *argv[])
 static void init_meminfo(void)
 {
 	swap_free_init = read_meminfo("SwapFree:");
-	mem_free_init = read_meminfo("MemFree:");
-	mem_over = mem_free_init * COE_SLIGHT_OVER;
-	mem_over_max = mem_free_init * COE_DELTA;
-
-	/* at least 10MB free physical memory needed */
-	if (mem_free_init < 10240) {
-		sleep(5);
-		if (mem_free_init < 10240)
+	if (!FILE_LINES_SCANF(cleanup, "/proc/meminfo", "MemAvailable: %ld",
+				&mem_available_init))
+		mem_available_init = read_meminfo("MemFree:") + read_meminfo("Cached:");
+	mem_over = mem_available_init * COE_SLIGHT_OVER;
+	mem_over_max = mem_available_init * COE_DELTA;

testcases/kernel/mem/swapping/swapping01.c int main(int argc, char *argv[]) · line 123

Quoted 10 lines
testcases/kernel/mem/swapping/swapping01.c +5 -2
+
+	/* at least 10MB available physical memory needed */
+	if (mem_available_init < 10240)
 			tst_brkm(TCONF, cleanup,
-				 "Not enough free memory to test.");
-	}
+				 "Not enough avalable memory to test.");
+
 	if (swap_free_init < mem_over)
 		tst_brkm(TCONF, cleanup, "Not enough swap space to test.");

Hi,

check_swapping() compares swap increase to mem_over_max. Is bad kernel really using that much swap? If so, then condition above looks wrong, since it doesn't check that we can fit so much into swap.

Quoted 14 lines
testcases/kernel/mem/swapping/swapping01.c +2 -2
 }
@@ -128,8 +128,8 @@ static void do_alloc(void)
 	long mem_count;
 	void *s;
 
-	tst_resm(TINFO, "free physical memory: %ld MB", mem_free_init / 1024);
-	mem_count = mem_free_init + mem_over;
+	tst_resm(TINFO, "available physical memory: %ld MB", mem_available_init /
 1024);
+	mem_count = mem_available_init + mem_over;
 	tst_resm(TINFO, "try to allocate: %ld MB", mem_count / 1024);
 	s = malloc(mem_count * 1024);
testcases/kernel/mem/swapping/swapping01.c +0 -0
 	if (s == NULL)

testcases/kernel/mem/swapping/swapping01.c static void check_swapping(void) · line 163

Quoted 5 lines
testcases/kernel/mem/swapping/swapping01.c +1 -1
@@ -160,7 +160,7 @@ static void check_swapping(void)
 			tst_brkm(TFAIL, cleanup, "heavy swapping detected: "
 				 "%ld MB swapped.", swapped / 1024);
 		}
-		sleep(1);
+		usleep(100000);

The original code appears to check multiple times to be sure that writes to swap have settled. Did you test if shortened delay can still detect bug on bad kernel?

I was thinking we break the loop if we see no change in swap usage for certain period:

i = 0;
while (i < X) {
i++;
swapped = read_meminfo("SwapFree:"); sleep(1);
if (abs(swapped - read_meminfo("SwapFree:")) < 512)

    break;
}

if (swapped > mem_over_max) {
FAIL
}

Regards,
Jan

Quoted 10 lines
testcases/kernel/mem/swapping/swapping01.c +0 -1
 	}
 	tst_resm(TPASS, "no heavy swapping detected, %ld MB swapped.",
 		 swapped / 1024);
--

1.8.3.1

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

author
#3 lore raw

PATCH 1/1 swapping: replace mem_free by mem_available

Quoted 32 lines · Jan Stancek
Quoted 19 lines
testcases/kernel/mem/swapping/swapping01.c +5 -8
@@ -108,17 +108,17 @@ int main(int argc, char *argv[])
 static void init_meminfo(void)
 {
 	swap_free_init = read_meminfo("SwapFree:");
-	mem_free_init = read_meminfo("MemFree:");
-	mem_over = mem_free_init * COE_SLIGHT_OVER;
-	mem_over_max = mem_free_init * COE_DELTA;
-
-	/* at least 10MB free physical memory needed */
-	if (mem_free_init < 10240) {
-		sleep(5);
-		if (mem_free_init < 10240)
+	if (!FILE_LINES_SCANF(cleanup, "/proc/meminfo", "MemAvailable: %ld",
+				&mem_available_init))
+		mem_available_init = read_meminfo("MemFree:") + read_meminfo("Cached:");
+	mem_over = mem_available_init * COE_SLIGHT_OVER;
+	mem_over_max = mem_available_init * COE_DELTA;

testcases/kernel/mem/swapping/swapping01.c int main(int argc, char *argv[]) · line 123

Quoted 10 lines
testcases/kernel/mem/swapping/swapping01.c +5 -2
+
+	/* at least 10MB available physical memory needed */
+	if (mem_available_init < 10240)
 			tst_brkm(TCONF, cleanup,
-				 "Not enough free memory to test.");
-	}
+				 "Not enough avalable memory to test.");
+
 	if (swap_free_init < mem_over)
 		tst_brkm(TCONF, cleanup, "Not enough swap space to test.");

Hi,

Quoted 3 lines

check_swapping() compares swap increase to mem_over_max. Is bad kernel really using that much swap? If so, then condition above looks wrong, since it doesn't check that we can fit so much into swap.

Good catch! If the total swap size is smaller than mem_over_max, there seems no need to run this case, because theoretically it'll always get PASS on bad kernel.

I'd like to add additional check:

	if (swap_total < mem_over_max)
		tst_brkm(TCONF, cleanup, "swap size is not fit to test");
Quoted 16 lines
Quoted 16 lines
testcases/kernel/mem/swapping/swapping01.c +2 -2
 }
@@ -128,8 +128,8 @@ static void do_alloc(void)
 	long mem_count;
 	void *s;
 
-	tst_resm(TINFO, "free physical memory: %ld MB", mem_free_init / 1024);
-	mem_count = mem_free_init + mem_over;
+	tst_resm(TINFO, "available physical memory: %ld MB", mem_available_init /
 1024);
+	mem_count = mem_available_init + mem_over;
 	tst_resm(TINFO, "try to allocate: %ld MB", mem_count / 1024);
 	s = malloc(mem_count * 1024);
testcases/kernel/mem/swapping/swapping01.c +0 -0
 	if (s == NULL)
@@ -160,7 +160,7 @@ static void check_swapping(void)
 			tst_brkm(TFAIL, cleanup, "heavy swapping detected: "
Quoted 8 lines

testcases/kernel/mem/swapping/swapping01.c static void check_swapping(void) · line 163

Quoted 4 lines
testcases/kernel/mem/swapping/swapping01.c +1 -1
 				 "%ld MB swapped.", swapped / 1024);
 		}
-		sleep(1);
+		usleep(100000);

The original code appears to check multiple times to be sure that writes to swap have settled. Did you test if shortened delay can still detect bug on bad kernel?

I didn't test it with bad kernel. But your worries are necessary, the swap checker probably spend a long time on big RAM box.

Quoted 3 lines

I was thinking we break the loop if we see no change in swap usage for certain period:

Agreed, this method looks more stable. Just have a little query of that.

Quoted 3 lines

i = 0;
while (i < X) {

How much should the X defined?

Quoted 4 lines
  i++;
  swapped = read_meminfo("SwapFree:");
  sleep(1);
  if (abs(swapped - read_meminfo("SwapFree:")) < 512)

Why set the limitation as 512? why not 1024 or others?

Quoted 6 lines
    break;
}

if (swapped > mem_over_max) {
FAIL
}

Regards,
Li Wang

#4 lore raw

PATCH 1/1 swapping: replace mem_free by mem_available

----- Original Message -----

Quoted 44 lines

From: "Li Wang" <[email protected]>
To: "Jan Stancek" <[email protected]>
Cc: [email protected]
Sent: Monday, 1 August, 2016 8:43:19 AM
Subject: Re: [LTP] [PATCH] swapping: replace mem_free by mem_available

Quoted 33 lines · Jan Stancek
Quoted 20 lines
testcases/kernel/mem/swapping/swapping01.c +5 -8
@@ -108,17 +108,17 @@ int main(int argc, char *argv[])
 static void init_meminfo(void)
 {
 	swap_free_init = read_meminfo("SwapFree:");
-	mem_free_init = read_meminfo("MemFree:");
-	mem_over = mem_free_init * COE_SLIGHT_OVER;
-	mem_over_max = mem_free_init * COE_DELTA;
-
-	/* at least 10MB free physical memory needed */
-	if (mem_free_init < 10240) {
-		sleep(5);
-		if (mem_free_init < 10240)
+	if (!FILE_LINES_SCANF(cleanup, "/proc/meminfo", "MemAvailable: %ld",
+				&mem_available_init))
+		mem_available_init = read_meminfo("MemFree:") +
 read_meminfo("Cached:");
+	mem_over = mem_available_init * COE_SLIGHT_OVER;
+	mem_over_max = mem_available_init * COE_DELTA;

testcases/kernel/mem/swapping/swapping01.c int main(int argc, char *argv[]) · line 124

Quoted 10 lines
testcases/kernel/mem/swapping/swapping01.c +5 -2
+
+	/* at least 10MB available physical memory needed */
+	if (mem_available_init < 10240)
 			tst_brkm(TCONF, cleanup,
-				 "Not enough free memory to test.");
-	}
+				 "Not enough avalable memory to test.");
+
 	if (swap_free_init < mem_over)
 		tst_brkm(TCONF, cleanup, "Not enough swap space to test.");

Hi,

Quoted 3 lines

check_swapping() compares swap increase to mem_over_max. Is bad kernel really using that much swap? If so, then condition above looks wrong, since it doesn't check that we can fit so much into swap.

Quoted 8 lines

Good catch! If the total swap size is smaller than mem_over_max, there seems no need to run this case, because theoretically it'll always get PASS on bad kernel.

I'd like to add additional check:

	if (swap_total < mem_over_max)
		tst_brkm(TCONF, cleanup, "swap size is not fit to test");

I'd modify the existing one.

Quoted 34 lines
Quoted 18 lines
Quoted 18 lines

}

testcases/kernel/mem/swapping/swapping01.c +1 -2
@@ -128,8 +128,8 @@ static void do_alloc(void)
 	long mem_count;
 	void *s;
 
-	tst_resm(TINFO, "free physical memory: %ld MB", mem_free_init / 1024);
-	mem_count = mem_free_init + mem_over;
+	tst_resm(TINFO, "available physical memory: %ld MB", mem_available_init

/
1024);

testcases/kernel/mem/swapping/swapping01.c +1 -0
+	mem_count = mem_available_init + mem_over;
 	tst_resm(TINFO, "try to allocate: %ld MB", mem_count / 1024);
 	s = malloc(mem_count * 1024);
 	if (s == NULL)
@@ -160,7 +160,7 @@ static void check_swapping(void)
 			tst_brkm(TFAIL, cleanup, "heavy swapping detected: "
Quoted 8 lines

testcases/kernel/mem/swapping/swapping01.c static void check_swapping(void) · line 163

Quoted 4 lines
testcases/kernel/mem/swapping/swapping01.c +1 -1
 				 "%ld MB swapped.", swapped / 1024);
 		}
-		sleep(1);
+		usleep(100000);

The original code appears to check multiple times to be sure that writes to swap have settled. Did you test if shortened delay can still detect bug on bad kernel?

I didn't test it with bad kernel. But your worries are necessary, the swap checker probably spend a long time on big RAM box.

Quoted 3 lines

I was thinking we break the loop if we see no change in swap usage for certain period:

Quoted 7 lines

Agreed, this method looks more stable. Just have a little query of that.

Quoted 3 lines

i = 0;
while (i < X) {

How much should the X defined?

I'd say at least 10 as with original testcase.

Quoted 7 lines
Quoted 5 lines
  i++;
  swapped = read_meminfo("SwapFree:");
  sleep(1);
  if (abs(swapped - read_meminfo("SwapFree:")) < 512)

Why set the limitation as 512? why not 1024 or others?

I picked that by rule of thumb, the value should be smaller than what any system can write to swap within 1s. But if we go too low then some noise from background processes could be falsely recognized as swap usage.

Regards,
Jan

Quoted 11 lines
Quoted 7 lines
    break;
}

if (swapped > mem_over_max) {
FAIL
}

Regards,
Li Wang

author
#5 lore raw

PATCH 1/1 swapping: replace mem_free by mem_available

Quoted 13 lines · Jan Stancek

----- Original Message -----

Quoted 2 lines

How much should the X defined?

I'd say at least 10 as with original testcase.

Quoted 6 lines
Quoted 5 lines
  i++;
  swapped = read_meminfo("SwapFree:");
  sleep(1);
  if (abs(swapped - read_meminfo("SwapFree:")) < 512)
Quoted 6 lines
Quoted 1 line

Why set the limitation as 512? why not 1024 or others?

I picked that by rule of thumb, the value should be smaller than what any system can write to swap within 1s. But if we go too low then some noise from background processes could be falsely recognized as swap usage.

Ok, thanks for the explanaiton. Patch V2 is coming.

Regards,
Li Wang