Re: [LTP] [PATCH] mem/oom: check for multiple outcomes if overcommit_memory is 0 or 1

[<ch...@su...>]

Hi!
> +/* oom/testoom outcomes */
> +#define EXIT_FLAG 0x100
> +#define SIGNAL_FLAG 0x200
> +#define OUTCOME_MASK 0xff
> +#define EXITED(n) (EXIT_FLAG + (n & OUTCOME_MASK))
> +#define SIGNALLED(n) (SIGNAL_FLAG + (n & OUTCOME_MASK))
> +
> +extern int EXITED_ENOMEM[];
> +extern int ENOMEM_OR_SIGKILL[];

This machinery looks a bit too generic to me. IMHO simple bit flag for
oom() that would allow/disallow the child being killed as a PASS result
would suffice.

Or do you expect that we will need (many) more possible cases to add?

>  long overcommit;
> -void oom(int testcase, int lite);
> -void testoom(int mempolicy, int lite);
> +void oom(int testcase, int lite, int outcome[]);
> +void testoom(int mempolicy, int lite, int outcome[]);
>  
>  /* KSM */
>  
> diff --git a/testcases/kernel/mem/lib/mem.c b/testcases/kernel/mem/lib/mem.c
> index c7910db..f207c58 100644
> --- a/testcases/kernel/mem/lib/mem.c
> +++ b/testcases/kernel/mem/lib/mem.c
> @@ -25,52 +25,73 @@
>  #include "numa_helper.h"
>  
>  /* OOM */
> +int EXITED_ENOMEM[] = { EXITED(ENOMEM), 0 };
> +int ENOMEM_OR_SIGKILL[] =  { EXITED(ENOMEM), SIGNALLED(SIGKILL), 0 };
>  
>  static int alloc_mem(long int length, int testcase)
>  {
> -	void *s;
> +	char *s;
> +	long i;
> +	int pagesz = getpagesize();
>  
>  	tst_resm(TINFO, "allocating %ld bytes.", length);
> +
>  	s = mmap(NULL, length, PROT_READ | PROT_WRITE,
>  		 MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
> -	if (s == MAP_FAILED) {
> -		if (testcase == OVERCOMMIT && errno == ENOMEM)
> -			return 1;
> -		else
> -			tst_brkm(TBROK | TERRNO, cleanup, "mmap");
> -	}
> +	if (s == MAP_FAILED)
> +		return errno;
> +
>  	if (testcase == MLOCK && mlock(s, length) == -1)
> -		tst_brkm(TBROK | TERRNO, cleanup, "mlock");
> +		return errno;
>  #ifdef HAVE_MADV_MERGEABLE
>  	if (testcase == KSM && madvise(s, length, MADV_MERGEABLE) == -1)
> -		tst_brkm(TBROK | TERRNO, cleanup, "madvise");
> +		return errno;
>  #endif
> -	memset(s, '\a', length);
> +	for (i = 0; i < length; i += pagesz)
> +		s[i] = '\a';

This is good optimalization but should be either mentioned in the commit
message or ideally commited separately.

-- 
Cyril Hrubis
ch...@su...