Re: [PATCH] memcg_stress: survive OOM by targeting the stressors

Hi Andrea,

Andrea Cervesato wrote:

> From: Andrea Cervesato <andrea.cervesato-IBi9RG/[email protected]>
> 
> The single-cgroup subtest intentionally lets one process fault in the
> whole memory budget to push the system to its memory ceiling, so hitting
> the OOM killer is an expected part of the stress. The problem is that the
> OOM killer could reap the driver script, turning that expected pressure
> into a spurious TBROK.
> 
> Mark the stress processes as the preferred OOM victims. A killed stressor
> is already reaped and cleaned up by the test, while the driver survives
> and reports the result.
> 
> Fixes: 02961a7b2bb4 ("memcg_stress_test.sh: Fix reserved mem calculate")
> Signed-off-by: Andrea Cervesato <andrea.cervesato-IBi9RG/[email protected]>
> ---
> On some systems this might happens:
> 
> [  244.207623][T117130] kirk[3061]: memcg_stress: start (command: memcg_stress_test.sh)
> [  244.391438][T117164] memcg_stress_te (117164): drop_caches: 3
> [ 1444.509722][T119900] kirk[3061]: memcg_stress: end (returncode: -1)
> [ 1444.629434][T119934] kirk[3061]: memcg_control: start (command: memcg_control_test.sh)
> [ 1445.856012][T120020] mem_process invoked oom-killer: gfp_mask=0xcc0(GFP_KERNEL), order=0, oom_score_adj=0

I agree with the underlying goal, keep the driver alive and let the
stressors absorb the OOM pressure, but I'd like to suggest a different
way that mirrors what we already do in the C harness, and that fixes
this at the lib level rather than in the individual test.

In lib/tst_test.c we don't raise the score of the stress/test processes.
Instead we protect the harness and then explicitly drop that protection
in the child before it runs the actual test:

$ grep -A 8 -B 9 tst_enable_oom_protection lib/tst_test.c

void tst_run_tcases(int argc, char *argv[], struct tst_test *self)
{
	unsigned int test_variants = 1;
	struct utsname uval;

	tst_test = self;

	do_setup(argc, argv);
	tst_enable_oom_protection(context->lib_pid);

	SAFE_SIGNAL(SIGALRM, alarm_handler);
	SAFE_SIGNAL(SIGUSR1, heartbeat_handler);

	tst_res(TINFO, "LTP version: "LTP_VERSION);

	uname(&uval);
	tst_res(TINFO, "Tested kernel: %s %s %s", uval.release, uval.version, uval.machine);

$ grep -A 8 -B 9 tst_disable_oom_protection lib/tst_test.c
	alarm(context->overall_time);

	show_failure_hints = 1;

	test_pid = fork();
	if (test_pid < 0)
		tst_brk(TBROK | TERRNO, "fork()");

	if (!test_pid) {
		tst_disable_oom_protection(0);
		SAFE_SIGNAL(SIGALRM, SIG_DFL);
		SAFE_SIGNAL(SIGUSR1, SIG_DFL);
		SAFE_SIGNAL(SIGTERM, SIG_DFL);
		SAFE_SIGNAL(SIGINT, SIG_DFL);
		SAFE_SETPGID(0, 0);
		testrun();
	}

The important detail is that oom_score_adj is inherited across fork.
So the model is: the library process sets itself to -1000, and child
resets itself back to 0 before doing any real work. The harness survives,
the workload stays a normal OOM candidate.

What do you think about doing this in the shell lib so all shell
tests benefit, rather than only memcg_stress?

-- 
Regards,
Li Wang

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