Re: [LTP] [PATCH v1] Refactor fork12 using new LTP API

[Richard Palethorpe <rpalethorpe@suse.de>]

Hello,

Richard Palethorpe <rpalethorpe@suse.de> writes:

> Hello,
>
> Andrea Cervesato <andrea.cervesato@suse.de> writes:
>
>> -	cleanup();
>> -	tst_exit();
>> +	tst_res(TINFO, "Number of processes forked is %d", forks);
>> +	TST_EXP_EXPR(TST_ERR == EAGAIN, "last fork() failed with
>> EAGAIN");
>
> I think it should at least accept ENOMEM as well.
>
>>  }
>>  
>> -static void setup(void)
>> +static void run(void)
>>  {
>> -	tst_sig(FORK, fork12_sigs, cleanup);
>> -	TEST_PAUSE;
>> +	int status;
>> +
>> +	child_pid = SAFE_FORK();
>> +	if (!child_pid) {
>
> Maybe here we should start a new process group which we can kill later
> in cleanup.
>
> I guess you would call SAFE_SETPGID(0, 0) here.
>
>> +		start_forking();
>> +		return;
>> +	}
>> +
>> +	SAFE_WAIT(&status);
>>  }
>>  
>>  static void cleanup(void)
>>  {
>> -	int waitstatus;
>> +	kill(child_pid, 0);
>>  
>> -	/* collect our kids */
>> -	kill(0, SIGQUIT);
>> -	while (wait(&waitstatus) > 0) ;
>> +	if (errno != ESRCH)
>
> Possibly errno is undefined if the call to kill was successful. Also
> there is a race here. You probably should just do an unconditional
> SIGKILL to all the children.
>
> If you have a process group as mentioned above, then you should be able
> to call kill(-child_pid, SIGKILL).
>
>> +		SAFE_KILL(child_pid, SIGKILL);
>>  }
>>  
>> -static void fork12_sigs(int signum)
>> -{
>> -	if (signum == SIGQUIT) {
>> -		/* Children will continue, parent will ignore */
>> -	} else {
>> -		tst_brkm(TBROK, cleanup,
>> -			 "Unexpected signal %d received.", signum);
>> -	}
>> -}
>> +static struct tst_test test = {
>> +	.test_all = run,
>> +	.cleanup = cleanup,
>> +	.forks_child = 1,
>> +	.max_runtime = 600,
>> +};
>> -- 
>> 2.35.3
>
> BTW, tests like this seem to cause containers to become unresponsive. If
> Kirk or LTX are running in the same container without any further
> isolation then we could use all the resources assigned to a container on
> forking. We can lock LTX in memory, but that still leaves CPU.
>
> Perhaps the test could reduce its own process and memory limit? e.g. with prlimit

This test also randomly fails outside of a container. Also other tests
that are testing the limits. This makes me think more that setting lower
prlimits is needed. Also this rewrite gets higher priority.

-- 
Thank you,
Richard.

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