[RFC] lib: enhancement of tst_pollute_memory

This method base on set '2' to '/proc/sys/vm/overcommit_memory' which
helps to use the strict policy on memory overcommit. After comparing
the existing value of 'CommitLimit' with 'MemAvailable', to determine
how much overcommit_ratio should be used in those two situations:

1. 'CommitLimit' >= 'MemAvailable'

This means system already set a proper ratio (by default is 50), and it
quite safe for memory allocating with 'overcommit_memory=2'. Don't worry
about OOM-Killer because, MAP_FAILED will be returned instantly if the
whole system allocated-memory reaches the limitation.

2. 'CommitLimit' < 'MemAvailable'

This means system possibly has a little size of swap-space (or disabled),
so that wouldn't provide enough memory on expected allocating, more easily
exceed CommitLimit and trigger OOM. We recalculate the capability and let
system won't go over a commit-ratio larger than '(FreeMem - safety) / TotalMem',
that also make safety-margin take effect to it.

And, I propose to skip this function to be performed on larger RAM (128G)
system, because it takes too long to dirty memory.

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

Notes:
    Hi Xinpeng,
    
    Before Cyril coming up with a new idea for solving this, I'd suggest you
    can help to test this patch with cancel the TCONF on 128G temporarily, to
    verify if my thought works for you.

 lib/tst_memutils.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

Hi!
>  lib/tst_memutils.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/lib/tst_memutils.c b/lib/tst_memutils.c
> index dd09db490..af2178a9c 100644
> --- a/lib/tst_memutils.c
> +++ b/lib/tst_memutils.c
> @@ -15,6 +15,7 @@
>  
>  void tst_pollute_memory(size_t maxsize, int fillchar)
>  {
> +	unsigned int ori_overcommit_mem, ori_overcommit_rat, new_overcommit_rat;
>  	size_t i, map_count = 0, safety = 0, blocksize = BLOCKSIZE;
>  	void **map_blocks;
>  	struct sysinfo info;
> @@ -40,6 +41,25 @@ void tst_pollute_memory(size_t maxsize, int fillchar)
>  	map_count = maxsize / blocksize;
>  	map_blocks = SAFE_MALLOC(map_count * sizeof(void *));
>  
> +	/* Only allow to run on systems with less than or equal to 128GB of memory */
> +	if (info.totalram  >= (137438953472 / info.mem_unit)) {
> +		tst_brk(TCONF, "Skip test - totalram (%lukB) is larger than 128GB",
> +				info.totalram * info.mem_unit / 1024);
> +	}

This is completely wrong, attempt to dirty memory only increases chances
of reproducing the kernel bug but it's not strictly required.

It's okay to limit the memory or completely skip the rest of this
function but we must not exit the test. The test is valid even if we do
not dirty any memory, it's just less likely to fail on buggy kernel.

> +	SAFE_FILE_SCANF("/proc/sys/vm/overcommit_memory", "%u", &ori_overcommit_mem);
> +	SAFE_FILE_SCANF("/proc/sys/vm/overcommit_ratio",  "%u", &ori_overcommit_rat);
> +
> +	/* Disable the memory overcommit to prevent test invoking OOM killer */
> +	if (SAFE_READ_MEMINFO("CommitLimit:") >= SAFE_READ_MEMINFO("MemAvailable:")) {
> +		SAFE_FILE_PRINTF("/proc/sys/vm/overcommit_memory", "2");
> +	} else {
> +		new_overcommit_rat = (maxsize / info.mem_unit * 100) / info.totalram;
> +		SAFE_FILE_PRINTF("/proc/sys/vm/overcommit_ratio", "%u", new_overcommit_rat);
> +
> +		SAFE_FILE_PRINTF("/proc/sys/vm/overcommit_memory", "2");
> +	}

And I do not like this that much either, I would be happier with
attempt to dirty only half of the available memory instead of depending
on specific kernel behavior like this.

>  	/*
>  	 * Keep allocating until the first failure. The address space may be
>  	 * too fragmented or just smaller than maxsize.
> @@ -60,4 +80,7 @@ void tst_pollute_memory(size_t maxsize, int fillchar)
>  		SAFE_MUNMAP(map_blocks[i], blocksize);
>  
>  	free(map_blocks);
> +
> +	SAFE_FILE_PRINTF("/proc/sys/vm/overcommit_memory", "%u", ori_overcommit_mem);
> +	SAFE_FILE_PRINTF("/proc/sys/vm/overcommit_ratio", "%u",  ori_overcommit_rat);
>  }
> -- 
> 2.21.3
>

On Mon, Feb 1, 2021 at 9:45 PM Cyril Hrubis <chrubis@suse.cz> wrote:

> Hi!
> >  lib/tst_memutils.c | 23 +++++++++++++++++++++++
> >  1 file changed, 23 insertions(+)
> >
> > diff --git a/lib/tst_memutils.c b/lib/tst_memutils.c
> > index dd09db490..af2178a9c 100644
> > --- a/lib/tst_memutils.c
> > +++ b/lib/tst_memutils.c
> > @@ -15,6 +15,7 @@
> >
> >  void tst_pollute_memory(size_t maxsize, int fillchar)
> >  {
> > +     unsigned int ori_overcommit_mem, ori_overcommit_rat,
> new_overcommit_rat;
> >       size_t i, map_count = 0, safety = 0, blocksize = BLOCKSIZE;
> >       void **map_blocks;
> >       struct sysinfo info;
> > @@ -40,6 +41,25 @@ void tst_pollute_memory(size_t maxsize, int fillchar)
> >       map_count = maxsize / blocksize;
> >       map_blocks = SAFE_MALLOC(map_count * sizeof(void *));
> >
> > +     /* Only allow to run on systems with less than or equal to 128GB
> of memory */
> > +     if (info.totalram  >= (137438953472 / info.mem_unit)) {
> > +             tst_brk(TCONF, "Skip test - totalram (%lukB) is larger
> than 128GB",
> > +                             info.totalram * info.mem_unit / 1024);
> > +     }
>
> This is completely wrong, attempt to dirty memory only increases chances
> of reproducing the kernel bug but it's not strictly required.
>
> It's okay to limit the memory or completely skip the rest of this
> function but we must not exit the test. The test is valid even if we do
> not dirty any memory, it's just less likely to fail on buggy kernel.
>

I thought dirty memory is a strict requirement before, so avoid test
on a large system.

Ok, I'm fine with removing these lines.



>
> > +     SAFE_FILE_SCANF("/proc/sys/vm/overcommit_memory", "%u",
> &ori_overcommit_mem);
> > +     SAFE_FILE_SCANF("/proc/sys/vm/overcommit_ratio",  "%u",
> &ori_overcommit_rat);
> > +
> > +     /* Disable the memory overcommit to prevent test invoking OOM
> killer */
> > +     if (SAFE_READ_MEMINFO("CommitLimit:") >=
> SAFE_READ_MEMINFO("MemAvailable:")) {
> > +             SAFE_FILE_PRINTF("/proc/sys/vm/overcommit_memory", "2");
> > +     } else {
> > +             new_overcommit_rat = (maxsize / info.mem_unit * 100) /
> info.totalram;
> > +             SAFE_FILE_PRINTF("/proc/sys/vm/overcommit_ratio", "%u",
> new_overcommit_rat);
> > +
> > +             SAFE_FILE_PRINTF("/proc/sys/vm/overcommit_memory", "2");
> > +     }
>
> And I do not like this that much either, I would be happier with
> attempt to dirty only half of the available memory instead of depending
> on specific kernel behavior like this.
>

The reason I come this way because I noticed that Xinpeng's system
has NO swap-space, that's probably the root cause to make 'CommitLimit'
less than 'MemAvailable'.

Then the system will prefer to use OOM (with overcommit_memory = 0 by
default) when allocated-memory reach to the CommitLimit.

[root@test-env-nm05-compute-14e5e72e38 ~]# cat /proc/meminfo
> MemTotal:       526997420 kB
> MemFree:        520224908 kB
> MemAvailable:   519936744 kB
> Buffers:               0 kB
> Cached:          2509036 kB
> SwapCached:            0 kB
> ...
> SwapTotal:             0 kB
> SwapFree:              0 kB
> ...
> CommitLimit:    263498708 kB
> Committed_AS:   10263760 kB
>