Re: [PATCH igt] core/sighelper: Interrupt everyone in the process group

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, Jan 08, 2016 at 08:44:29AM +0000, Chris Wilson wrote:
> Some stress tests create both the signal helper and a lot of competing
> processes. In these tests, the parent is just waiting upon the children,
> and the intention is not to keep waking up the waiting parent, but to
> keep interrupting the children (as we hope to trigger races in our
> kernel code). kill(-pid) sends the signal to all members of the process
> group, not just the target pid.

I don't really have any clue about unix pgroups, but the -pid disappeared
compared to the previous version.
> 
> We also switch from using SIGUSR1 to SIGCONT to paper over a race
> condition when forking children that saw the default signal action being
> run (and thus killing the child).

I thought I fixed that race by first installing the new signal handler,
then forking. Ok, rechecked and it's the SYS_getpid stuff, so another
race. Still I thought signal handlers would survive a fork?
-Daniel

> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  lib/igt_aux.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/igt_aux.c b/lib/igt_aux.c
> index 4d08d68..f6b5792 100644
> --- a/lib/igt_aux.c
> +++ b/lib/igt_aux.c
> @@ -79,7 +79,7 @@ static void __attribute__((noreturn)) signal_helper_process(pid_t pid)
>  	/* Interrupt the parent process at 500Hz, just to be annoying */
>  	while (1) {
>  		usleep(1000 * 1000 / 500);
> -		if (kill(pid, SIGUSR1)) /* Parent has died, so must we. */
> +		if (kill(pid, SIGCONT)) /* Parent has died, so must we. */
>  			exit(0);
>  	}
>  }
> @@ -93,7 +93,7 @@ static void sig_handler(int i)
>   * igt_fork_signal_helper:
>   *
>   * Fork a child process using #igt_fork_helper to interrupt the parent process
> - * with a SIGUSR1 signal at regular quick intervals. The corresponding dummy
> + * with a SIGCONT signal at regular quick intervals. The corresponding dummy
>   * signal handler is installed in the parent process.
>   *
>   * This is useful to exercise ioctl error paths, at least where those can be
> @@ -108,10 +108,12 @@ void igt_fork_signal_helper(void)
>  	if (igt_only_list_subtests())
>  		return;
>  
> -	signal(SIGUSR1, sig_handler);
> +	signal(SIGCONT, sig_handler);
> +	setpgrp(); /* define a new process group for ourselves */
>  
>  	igt_fork_helper(&signal_helper) {
> -		signal_helper_process(getppid());
> +		signal(SIGCONT, SIG_IGN);
> +		signal_helper_process(-getppid());
>  	}
>  }
>  
> -- 
> 2.7.0.rc3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx