diff mbox series

lib: memutils: respect minimum memory watermark when polluting memory

Message ID 20211019150206.90335-1-krzysztof.kozlowski@canonical.com
State Superseded
Headers show
Series lib: memutils: respect minimum memory watermark when polluting memory | expand

Commit Message

Krzysztof Kozlowski Oct. 19, 2021, 3:02 p.m. UTC 
Previous fix for an out-of-memory killer killing ioctl_sg01 process
in commit 4d2e3d44fad5 ("lib: memutils: don't pollute
entire system memory to avoid OoM") was not fully effective.  While it
covers most of the cases, an ARM64 machine with 128 GB of memory, 64 kB
page size and v5.11 kernel hit it again.  Polluting the memory fails
with OoM:

  LTP: starting ioctl_sg01
  ioctl_sg01 invoked oom-killer: gfp_mask=0x100dca(GFP_HIGHUSER_MOVABLE|__GFP_ZERO), order=0, oom_score_adj=0
  ...
  Mem-Info:
  active_anon:309 inactive_anon:1964781 isolated_anon:0
                  active_file:94 inactive_file:0 isolated_file:0
                  unevictable:305 dirty:0 writeback:0
                  slab_reclaimable:1510 slab_unreclaimable:5012
                  mapped:115 shmem:339 pagetables:463 bounce:0
                  free:112043 free_pcp:1 free_cma:3159
  Node 0 active_anon:19776kB inactive_anon:125745984kB active_file:6016kB inactive_file:0kB unevictable:19520kB ...
  Node 0 DMA free:710656kB min:205120kB low:256384kB high:307648kB reserved_highatomic:0KB active_anon:0kB inactive_anon:3332032kB ...
  lowmem_reserve[]: 0 0 7908 7908 7908
  Node 0 Normal free:6460096kB min:6463168kB low:8078912kB high:9694656kB reserved_highatomic:0KB active_anon:19776kB inactive_anon:122413952kB ...
  lowmem_reserve[]: 0 0 0 0 0

The important part are details of memory on Node 0 in Normal zone:
1. free memory: 6460096 kB
2. min (minimum watermark): 6463168 kB

Parse the /proc/zoneinfo and include the "min" data when counting safety
(free memory which should not be polluted).  This way we also include
minimum memory for DMA zones and all nodes.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
---
 lib/tst_memutils.c | 31 +++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

Comments

Li Wang Oct. 20, 2021, 4:07 a.m. UTC | #1 
On Tue, Oct 19, 2021 at 11:02 PM Krzysztof Kozlowski <
krzysztof.kozlowski@canonical.com> wrote:

> Previous fix for an out-of-memory killer killing ioctl_sg01 process
> in commit 4d2e3d44fad5 ("lib: memutils: don't pollute
> entire system memory to avoid OoM") was not fully effective.  While it
> covers most of the cases, an ARM64 machine with 128 GB of memory, 64 kB
> page size and v5.11 kernel hit it again.  Polluting the memory fails
> with OoM:
>
>   LTP: starting ioctl_sg01
>   ioctl_sg01 invoked oom-killer:
> gfp_mask=0x100dca(GFP_HIGHUSER_MOVABLE|__GFP_ZERO), order=0, oom_score_adj=0
>   ...
>   Mem-Info:
>   active_anon:309 inactive_anon:1964781 isolated_anon:0
>                   active_file:94 inactive_file:0 isolated_file:0
>                   unevictable:305 dirty:0 writeback:0
>                   slab_reclaimable:1510 slab_unreclaimable:5012
>                   mapped:115 shmem:339 pagetables:463 bounce:0
>                   free:112043 free_pcp:1 free_cma:3159
>   Node 0 active_anon:19776kB inactive_anon:125745984kB active_file:6016kB
> inactive_file:0kB unevictable:19520kB ...
>   Node 0 DMA free:710656kB min:205120kB low:256384kB high:307648kB
> reserved_highatomic:0KB active_anon:0kB inactive_anon:3332032kB ...
>   lowmem_reserve[]: 0 0 7908 7908 7908
>   Node 0 Normal free:6460096kB min:6463168kB low:8078912kB high:9694656kB
> reserved_highatomic:0KB active_anon:19776kB inactive_anon:122413952kB ...
>   lowmem_reserve[]: 0 0 0 0 0
>
> The important part are details of memory on Node 0 in Normal zone:
> 1. free memory: 6460096 kB
> 2. min (minimum watermark): 6463168 kB
>
> Parse the /proc/zoneinfo and include the "min" data when counting safety
> (free memory which should not be polluted).  This way we also include
> minimum memory for DMA zones and all nodes.
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> ---
>  lib/tst_memutils.c | 31 +++++++++++++++++++++++++++++--
>  1 file changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/lib/tst_memutils.c b/lib/tst_memutils.c
> index af132bcc6c24..f34ba582ec93 100644
> --- a/lib/tst_memutils.c
> +++ b/lib/tst_memutils.c
> @@ -6,22 +6,49 @@
>  #include <unistd.h>
>  #include <limits.h>
>  #include <sys/sysinfo.h>
> +#include <stdio.h>
>  #include <stdlib.h>
>
>  #define TST_NO_DEFAULT_MAIN
>  #include "tst_test.h"
> +#include "tst_safe_stdio.h"
>
>  #define BLOCKSIZE (16 * 1024 * 1024)
>
> +unsigned long tst_min_memory(void)
>

The prefix "tst_" is used for a function which extern to the whole LTP.
In this case the function is only used in the library, so we'd better
avoid using tst_.

As it parses minimal pages from /proc/zoneinfo, so I'd suggest rename
it to count_min_pages.

The remaining part looks good to me.

Reviewed-by: Li Wang <liwang@redhat.com>

+{
> +       FILE *fp;
> +       int ret;
> +       unsigned long total_pages = 0;
> +       unsigned long pages;
> +       char line[BUFSIZ];
> +
> +       fp = SAFE_FOPEN("/proc/zoneinfo", "r");
> +
> +       while (fgets(line, BUFSIZ, fp) != NULL) {
> +               ret = sscanf(line, " min %lu", &pages);
> +               if (ret == 1)
> +                       total_pages += pages;
> +       }
> +
> +       SAFE_FCLOSE(fp);
> +
> +       /* Apply a margin because we cannot get below "min" watermark */
> +       total_pages += (total_pages / 10);
> +
> +       return total_pages;
> +}
> +
>  void tst_pollute_memory(size_t maxsize, int fillchar)
>  {
>         size_t i, map_count = 0, safety = 0, blocksize = BLOCKSIZE;
> +       long pagesize = SAFE_SYSCONF(_SC_PAGESIZE);
>         void **map_blocks;
>         struct sysinfo info;
>
>         SAFE_SYSINFO(&info);
> -       safety = MAX(4096 * SAFE_SYSCONF(_SC_PAGESIZE), 128 * 1024 * 1024);
> -       safety = MAX(safety, (info.freeram / 64));
> +       safety = MAX(4096 * pagesize, 128 * 1024 * 1024);
> +       safety = MAX(safety, tst_min_memory() * pagesize);
>         safety /= info.mem_unit;
>
>         if (info.freeswap > safety)
> --
> 2.30.2
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
>
>
liuxp11@chinatelecom.cn Oct. 20, 2021, 5:56 a.m. UTC | #2 
Serveral months ago,i report this issue.I suggest to use available memory.
You can test with available memory instead of freeram,It will be OK.

Report-by: Liu Xinpeng <liuxp11@chinatelecom.cn>
Reviewed-by: Liu Xinpeng <liuxp11@chinatelecom.cn>



Thanks！

 
From: Krzysztof Kozlowski
Date: 2021-10-19 23:02
To: ltp
Subject: [LTP] [PATCH] lib: memutils: respect minimum memory watermark when polluting memory
Previous fix for an out-of-memory killer killing ioctl_sg01 process
in commit 4d2e3d44fad5 ("lib: memutils: don't pollute
entire system memory to avoid OoM") was not fully effective.  While it
covers most of the cases, an ARM64 machine with 128 GB of memory, 64 kB
page size and v5.11 kernel hit it again.  Polluting the memory fails
with OoM:
 
  LTP: starting ioctl_sg01
  ioctl_sg01 invoked oom-killer: gfp_mask=0x100dca(GFP_HIGHUSER_MOVABLE|__GFP_ZERO), order=0, oom_score_adj=0
  ...
  Mem-Info:
  active_anon:309 inactive_anon:1964781 isolated_anon:0
                  active_file:94 inactive_file:0 isolated_file:0
                  unevictable:305 dirty:0 writeback:0
                  slab_reclaimable:1510 slab_unreclaimable:5012
                  mapped:115 shmem:339 pagetables:463 bounce:0
                  free:112043 free_pcp:1 free_cma:3159
  Node 0 active_anon:19776kB inactive_anon:125745984kB active_file:6016kB inactive_file:0kB unevictable:19520kB ...
  Node 0 DMA free:710656kB min:205120kB low:256384kB high:307648kB reserved_highatomic:0KB active_anon:0kB inactive_anon:3332032kB ...
  lowmem_reserve[]: 0 0 7908 7908 7908
  Node 0 Normal free:6460096kB min:6463168kB low:8078912kB high:9694656kB reserved_highatomic:0KB active_anon:19776kB inactive_anon:122413952kB ...
  lowmem_reserve[]: 0 0 0 0 0
 
The important part are details of memory on Node 0 in Normal zone:
1. free memory: 6460096 kB
2. min (minimum watermark): 6463168 kB
 
Parse the /proc/zoneinfo and include the "min" data when counting safety
(free memory which should not be polluted).  This way we also include
minimum memory for DMA zones and all nodes.
 
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
---
lib/tst_memutils.c | 31 +++++++++++++++++++++++++++++--
1 file changed, 29 insertions(+), 2 deletions(-)
 
diff --git a/lib/tst_memutils.c b/lib/tst_memutils.c
index af132bcc6c24..f34ba582ec93 100644
--- a/lib/tst_memutils.c
+++ b/lib/tst_memutils.c
@@ -6,22 +6,49 @@
#include <unistd.h>
#include <limits.h>
#include <sys/sysinfo.h>
+#include <stdio.h>
#include <stdlib.h>
#define TST_NO_DEFAULT_MAIN
#include "tst_test.h"
+#include "tst_safe_stdio.h"
#define BLOCKSIZE (16 * 1024 * 1024)
+unsigned long tst_min_memory(void)
+{
+ FILE *fp;
+ int ret;
+ unsigned long total_pages = 0;
+ unsigned long pages;
+ char line[BUFSIZ];
+
+ fp = SAFE_FOPEN("/proc/zoneinfo", "r");
+
+ while (fgets(line, BUFSIZ, fp) != NULL) {
+ ret = sscanf(line, " min %lu", &pages);
+ if (ret == 1)
+ total_pages += pages;
+ }
+
+ SAFE_FCLOSE(fp);
+
+ /* Apply a margin because we cannot get below "min" watermark */
+ total_pages += (total_pages / 10);
+
+ return total_pages;
+}
+
void tst_pollute_memory(size_t maxsize, int fillchar)
{
size_t i, map_count = 0, safety = 0, blocksize = BLOCKSIZE;
+ long pagesize = SAFE_SYSCONF(_SC_PAGESIZE);
void **map_blocks;
struct sysinfo info;
SAFE_SYSINFO(&info);
- safety = MAX(4096 * SAFE_SYSCONF(_SC_PAGESIZE), 128 * 1024 * 1024);
- safety = MAX(safety, (info.freeram / 64));
+ safety = MAX(4096 * pagesize, 128 * 1024 * 1024);
+ safety = MAX(safety, tst_min_memory() * pagesize);
safety /= info.mem_unit;
if (info.freeswap > safety)
Krzysztof Kozlowski Oct. 20, 2021, 6:12 a.m. UTC | #3 
On 20/10/2021 07:56, liuxp11@chinatelecom.cn wrote:
> Serveral months ago,i report this issue.I suggest to use available memory.
> You can test with available memory instead of freeram,It will be OK.
> 
> Report-by: Liu Xinpeng <liuxp11@chinatelecom.cn <mailto:liwang@redhat.com>>
> Reviewed-by: Liu Xinpeng <liuxp11@chinatelecom.cn
> <mailto:liwang@redhat.com>>
> 

Don't respond with HTML, please.

It was reported a year ago also here:
https://bugs.launchpad.net/ubuntu-kernel-tests/+bug/1899413

Using MemAvailable seems to be incorrect as it returns too much memory
available. It does not respect min watermark.


Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/lib/tst_memutils.c b/lib/tst_memutils.c
index af132bcc6c24..f34ba582ec93 100644
--- a/lib/tst_memutils.c
+++ b/lib/tst_memutils.c
@@ -6,22 +6,49 @@ 
 #include <unistd.h>
 #include <limits.h>
 #include <sys/sysinfo.h>
+#include <stdio.h>
 #include <stdlib.h>
 
 #define TST_NO_DEFAULT_MAIN
 #include "tst_test.h"
+#include "tst_safe_stdio.h"
 
 #define BLOCKSIZE (16 * 1024 * 1024)
 
+unsigned long tst_min_memory(void)
+{
+	FILE *fp;
+	int ret;
+	unsigned long total_pages = 0;
+	unsigned long pages;
+	char line[BUFSIZ];
+
+	fp = SAFE_FOPEN("/proc/zoneinfo", "r");
+
+	while (fgets(line, BUFSIZ, fp) != NULL) {
+		ret = sscanf(line, " min %lu", &pages);
+		if (ret == 1)
+			total_pages += pages;
+	}
+
+	SAFE_FCLOSE(fp);
+
+	/* Apply a margin because we cannot get below "min" watermark */
+	total_pages += (total_pages / 10);
+
+	return total_pages;
+}
+
 void tst_pollute_memory(size_t maxsize, int fillchar)
 {
 	size_t i, map_count = 0, safety = 0, blocksize = BLOCKSIZE;
+	long pagesize = SAFE_SYSCONF(_SC_PAGESIZE);
 	void **map_blocks;
 	struct sysinfo info;
 
 	SAFE_SYSINFO(&info);
-	safety = MAX(4096 * SAFE_SYSCONF(_SC_PAGESIZE), 128 * 1024 * 1024);
-	safety = MAX(safety, (info.freeram / 64));
+	safety = MAX(4096 * pagesize, 128 * 1024 * 1024);
+	safety = MAX(safety, tst_min_memory() * pagesize);
 	safety /= info.mem_unit;
 
 	if (info.freeswap > safety)