Re: [LTP] [PATCH v1] Check for maximum available pids in dio_sparse.c

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
>  static void setup(void)
>  {
>  	struct stat sb;
> +	int max_pids;
>  
>  	numchildren = 1000;
>  	writesize = 1024;
> @@ -69,6 +70,13 @@ static void setup(void)
>  	if (tst_parse_int(str_numchildren, &numchildren, 1, INT_MAX))
>  		tst_brk(TBROK, "Invalid number of children '%s'", str_numchildren);
>  
> +	max_pids = tst_get_free_pids();
> +	if (numchildren > max_pids) {
> +		numchildren = max_pids;
> +
> +		tst_res(TCONF, "Number of children reduced to %d due to system limitations", numchildren);
                          ^
			  If we are going to limit the number of
			  children this should be TINFO

And if we are going to skip the test it should be tst_brk(TCONF, ...)

Either way tst_res(TCONF, ...) does not make much sense in this case.

> +	}

I guess that we should do a similar check in all the io tests that fork
children, so we may as well put it into some kind of library function.

Maybe just the common.h with something as:

static inline void check_children(unsigned int numchildren)
{
	if (numchildren > tst_get_free_pids)
		tst_brk(TCONF, "....");
}

or:

static inline void check_children(unsigned int *numchilren)
{
	...
}

In case that we want to print the info message and modify the value.

>  	if (tst_parse_filesize(str_writesize, &writesize, 1, LLONG_MAX))
>  		tst_brk(TBROK, "Invalid write blocks size '%s'", str_writesize);
>  
> @@ -129,10 +137,10 @@ static struct tst_test test = {
>  	.needs_tmpdir = 1,
>  	.forks_child = 1,
>  	.options = (struct tst_option[]) {
> -		{"n:", &str_numchildren, "Number of threads (default 1000)"},
> -		{"w:", &str_writesize, "Size of writing blocks (default 1K)"},
> -		{"s:", &str_filesize, "Size of file (default 100M)"},
> -		{"o:", &str_offset, "File offset (default 0)"},
> -		{}
> +		{"n:", &str_numchildren, "-n\t Number of threads (default 1000)"},
> +		{"w:", &str_writesize, "-w\t Size of writing blocks (default 1K)"},
> +		{"s:", &str_filesize, "-s\t Size of file (default 100M)"},
> +		{"o:", &str_offset, "-o\t File offset (default 0)"},
> +		{},

This part is certainly wrong.

-- 
Cyril Hrubis
chrubis@suse.cz

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