[LTP] [PATCH] tst_pollute_memory(): Set minimal safety margin to 64MB

[LTP] [PATCH] tst_pollute_memory(): Set minimal safety margin to 64MB

[Martin Doucha]

4096 pages amounts to 16MB on x86_64 or 256MB on PPC64. 16MB is not enough
to avoid OOM killer on some systems so set the safety margin to either 64MB
or 4096 pages, whichever is higher.

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---

Here's my proposed fix for the OOM issues with ioctl_sg01. I can't reproduce
the issue on my x86_64 VMs/machines so please confirm that it works.

Don't forget to run `make clean` after applying this patch because ioctl_sg01
will not be rebuilt automatically due to weak dependency on libltp.a.

 lib/tst_memutils.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/tst_memutils.c b/lib/tst_memutils.c
index f134d90c9..c20c94a00 100644
--- a/lib/tst_memutils.c
+++ b/lib/tst_memutils.c
@@ -20,7 +20,8 @@ void tst_pollute_memory(size_t maxsize, int fillchar)
 	struct sysinfo info;
 
 	SAFE_SYSINFO(&info);
-	safety = 4096 * SAFE_SYSCONF(_SC_PAGESIZE) / info.mem_unit;
+	safety = MAX(4096 * SAFE_SYSCONF(_SC_PAGESIZE), 64 * 1024 * 1024);
+	safety /= info.mem_unit;
 
 	if (info.freeswap > safety)
 		safety = 0;
-- 
2.29.2

[LTP] [PATCH] tst_pollute_memory(): Set minimal safety margin to 64MB

[Cyril Hrubis]

Hi!
>  	SAFE_SYSINFO(&info);
> -	safety = 4096 * SAFE_SYSCONF(_SC_PAGESIZE) / info.mem_unit;
> +	safety = MAX(4096 * SAFE_SYSCONF(_SC_PAGESIZE), 64 * 1024 * 1024);
> +	safety /= info.mem_unit;

I guess that this is safe enough for the release, since it will only
increase the safety margin.

Naresh can you please test this patch ASAP?

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH] tst_pollute_memory(): Set minimal safety margin to 64MB

[Naresh Kamboju]

On Tue, 19 Jan 2021 at 18:12, Cyril Hrubis <chrubis@suse.cz> wrote:
>
> Hi!
> >       SAFE_SYSINFO(&info);
> > -     safety = 4096 * SAFE_SYSCONF(_SC_PAGESIZE) / info.mem_unit;
> > +     safety = MAX(4096 * SAFE_SYSCONF(_SC_PAGESIZE), 64 * 1024 * 1024);
> > +     safety /= info.mem_unit;
>
> I guess that this is safe enough for the release, since it will only
> increase the safety margin.
>
> Naresh can you please test this patch ASAP?

I have applied your patch and rebuilt completely and retested
ioctl_sg01 test case in a loop on three different devices.

1 PASS out of 20 runs with overcommit_memory 0 on x86_64.
1 PASS out of 20 runs with overcommit_memory 1 on x86_64.

Which means 19 times the test case triggered oom-killer and the test was broken.

- Naresh

[LTP] [PATCH] tst_pollute_memory(): Set minimal safety margin to 64MB

[Cyril Hrubis]

Hi!
> > > -     safety = 4096 * SAFE_SYSCONF(_SC_PAGESIZE) / info.mem_unit;
> > > +     safety = MAX(4096 * SAFE_SYSCONF(_SC_PAGESIZE), 64 * 1024 * 1024);
> > > +     safety /= info.mem_unit;
> >
> > I guess that this is safe enough for the release, since it will only
> > increase the safety margin.
> >
> > Naresh can you please test this patch ASAP?
> 
> I have applied your patch and rebuilt completely and retested
> ioctl_sg01 test case in a loop on three different devices.
> 
> 1 PASS out of 20 runs with overcommit_memory 0 on x86_64.
> 1 PASS out of 20 runs with overcommit_memory 1 on x86_64.
> 
> Which means 19 times the test case triggered oom-killer and the test was broken.

So it looks like 64MB is not enough in your case.

Does it work with 128MB or 256MB?

i.e.:

safety = MAX(4096 * SAFE_SYSCONF(_SC_PAGESIZE), 128 * 1024 * 1024);
safety = MAX(4096 * SAFE_SYSCONF(_SC_PAGESIZE), 256 * 1024 * 1024);

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH] tst_pollute_memory(): Set minimal safety margin to 64MB

[Naresh Kamboju]

On Wed, 20 Jan 2021 at 20:23, Cyril Hrubis <chrubis@suse.cz> wrote:
>
> Hi!
> > > > -     safety = 4096 * SAFE_SYSCONF(_SC_PAGESIZE) / info.mem_unit;
> > > > +     safety = MAX(4096 * SAFE_SYSCONF(_SC_PAGESIZE), 64 * 1024 * 1024);
> > > > +     safety /= info.mem_unit;
> > >
> > > I guess that this is safe enough for the release, since it will only
> > > increase the safety margin.
> > >
> > > Naresh can you please test this patch ASAP?
> >
> > I have applied your patch and rebuilt completely and retested
> > ioctl_sg01 test case in a loop on three different devices.
> >
> > 1 PASS out of 20 runs with overcommit_memory 0 on x86_64.
> > 1 PASS out of 20 runs with overcommit_memory 1 on x86_64.
> >
> > Which means 19 times the test case triggered oom-killer and the test was broken.
>
> So it looks like 64MB is not enough in your case.
>
> Does it work with 128MB or 256MB?
>
> i.e.:
>
> safety = MAX(4096 * SAFE_SYSCONF(_SC_PAGESIZE), 128 * 1024 * 1024);
> safety = MAX(4096 * SAFE_SYSCONF(_SC_PAGESIZE), 256 * 1024 * 1024);


I will test with these changes and get back to you.

Meanwhile,

20 PASS out of 20 runs with overcommit_memory 2 on x86_64.

- Naresh

[LTP] [PATCH] tst_pollute_memory(): Set minimal safety margin to 64MB

[Cyril Hrubis]

Hi!
> > > I have applied your patch and rebuilt completely and retested
> > > ioctl_sg01 test case in a loop on three different devices.
> > >
> > > 1 PASS out of 20 runs with overcommit_memory 0 on x86_64.
> > > 1 PASS out of 20 runs with overcommit_memory 1 on x86_64.
> > >
> > > Which means 19 times the test case triggered oom-killer and the test was broken.
> >
> > So it looks like 64MB is not enough in your case.
> >
> > Does it work with 128MB or 256MB?
> >
> > i.e.:
> >
> > safety = MAX(4096 * SAFE_SYSCONF(_SC_PAGESIZE), 128 * 1024 * 1024);
> > safety = MAX(4096 * SAFE_SYSCONF(_SC_PAGESIZE), 256 * 1024 * 1024);
> 
> 
> I will test with these changes and get back to you.
> 
> Meanwhile,
> 
> 20 PASS out of 20 runs with overcommit_memory 2 on x86_64.

That is to be expected, since with overcommit_memory=2 we fail the
mmap() when the kernel does  not think there is enough free memory. See
the part where we do:

	if (map_blocks[i] == MAP_FAILED) {
		map_count = i;
		break;
	}

It would be interesting to print the map_count and the value of i in
this fucntion and see where it breaks with overcommit_memory=2.

i.e.

	if (map_blocks[i] == MAP_FAILED) {
		tst_res(TINFO, "mmap() failed at %zu expected %zu",
		        i, map_count);
		map_count = i;
		break;
	}


-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH] tst_pollute_memory(): Set minimal safety margin to 64MB

[Naresh Kamboju]

On Wed, 20 Jan 2021 at 20:35, Cyril Hrubis <chrubis@suse.cz> wrote:
>
> Hi!
> > > > I have applied your patch and rebuilt completely and retested
> > > > ioctl_sg01 test case in a loop on three different devices.
> > > >
> > > > 1 PASS out of 20 runs with overcommit_memory 0 on x86_64.
> > > > 1 PASS out of 20 runs with overcommit_memory 1 on x86_64.
> > > >
> > > > Which means 19 times the test case triggered oom-killer and the test was broken.
> > >
> > > So it looks like 64MB is not enough in your case.
> > >
> > > Does it work with 128MB or 256MB?
> > >
> > > i.e.:
> > >
> > > safety = MAX(4096 * SAFE_SYSCONF(_SC_PAGESIZE), 128 * 1024 * 1024);

After changing to 128MB the test got PASS with overcommit_memory 0 on x86_64.

- Naresh

[LTP] [PATCH] tst_pollute_memory(): Set minimal safety margin to 64MB

[Martin Doucha]

On 20. 01. 21 18:04, Naresh Kamboju wrote:
> On Wed, 20 Jan 2021 at 20:35, Cyril Hrubis <chrubis@suse.cz> wrote:
>>>>
>>>> So it looks like 64MB is not enough in your case.
>>>>
>>>> Does it work with 128MB or 256MB?
>>>>
>>>> i.e.:
>>>>
>>>> safety = MAX(4096 * SAFE_SYSCONF(_SC_PAGESIZE), 128 * 1024 * 1024);
> 
> After changing to 128MB the test got PASS with overcommit_memory 0 on x86_64.

OK, let's go with 128MB then.

-- 
Martin Doucha   mdoucha@suse.cz
QA Engineer for Software Maintenance
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic

[LTP] [PATCH] tst_pollute_memory(): Set minimal safety margin to 64MB

[Cyril Hrubis]

Hi!
> After changing to 128MB the test got PASS with overcommit_memory 0 on x86_64.

Thanks a lot, fix pushed.

-- 
Cyril Hrubis
chrubis@suse.cz