[Intel-gfx] [PATCH i-g-t] overlay/intel-gpu-overlay

[Vlad, Marius C]

-----Original Message-----
From: Chris Wilson [mailto:chris at chris-wilson.co.uk] 
Sent: Wednesday, November 18, 2015 2:59 PM
To: Vlad, Marius C <marius.c.vlad at intel.com>
Cc: intel-gfx at lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH i-g-t] overlay/intel-gpu-overlay

On Wed, Nov 18, 2015 at 02:40:29PM +0200, marius.c.vlad at intel.com wrote:
> From: Marius Vlad <marius.c.vlad at intel.com>
> 
> Use sigaction() instead of signal() and add SIGINT, SIGTERM to close 
> the overlay window. With this change the overlay window will be 
> destroyed.
> 
> Signed-off-by: Marius Vlad <marius.c.vlad at intel.com>
> ---
>  overlay/overlay.c | 42 ++++++++++++++++++++++++++++++++++++------
>  1 file changed, 36 insertions(+), 6 deletions(-)
> 
> diff --git a/overlay/overlay.c b/overlay/overlay.c index 
> 3c0dbb4..48ba67c 100644
> --- a/overlay/overlay.c
> +++ b/overlay/overlay.c
> @@ -804,11 +804,19 @@ static void show_gem_objects(struct 
> overlay_context *ctx, struct overlay_gem_obj
>  
>  static int take_snapshot;
>  
> -static void signal_snapshot(int sig)
> +static void
> +signal_snapshot(int sig, siginfo_t *si, void *__unused)
>  {
>  	take_snapshot = sig;
>  }
>  
> +static void
> +signal_x11_destroy(int sig, siginfo_t *si, void *__unused) {
> +	x11_overlay_stop();

is not signalsafe.

Indeed. Any ideas then? I'm hitting a weird system hang if the overlay window is open
for a period of time (although period ranges to minutes it seems to happen each time). If the window
is closed and process exits I don't see this behavior. This is entirely different bug but meanwhile 
wanted to keep my machine running.

> +	if (ctx.surface == NULL) {
> +		fprintf(stderr, "Failed to create X11 overlay.\n");

Spurious changes.

Right, just debugging. 

>  		ctx.surface = x11_window_create(&config, &ctx.width, &ctx.height);
> -	if (ctx.surface == NULL)
> +	}
> +	if (ctx.surface == NULL) {
> +		fprintf(stderr, "Failed to create X11 window.\n");
>  		ctx.surface = kms_overlay_create(&config, &ctx.width, &ctx.height);
> -	if (ctx.surface == NULL)
> +	}
> +	if (ctx.surface == NULL) {
> +		fprintf(stderr, "Failed to create KMS overlay.\n");
>  		return ENXIO;
> +	}
>  
>  	if (daemonize && daemon(0, 0))
>  		return EINVAL;
> @@ -913,7 +930,20 @@ int main(int argc, char **argv)
>  	if (renice && (nice(renice) == -1))
>  		fprintf(stderr, "Could not renice: %s\n", strerror(errno));
>  
> -	signal(SIGUSR1, signal_snapshot);
> +	sa.sa_flags = SA_SIGINFO;
> +	sigemptyset(&sa.sa_mask);
> +	sa.sa_sigaction = &signal_snapshot;
> +
> +	if (sigaction(SIGUSR1, &sa, NULL) == -1) {

Any particular reason for a fondness here for sigaction?

Well signal(2) states to avoid it and instead use sigaction(). I'm not that fond of it :)

-Chris

--
Chris Wilson, Intel Open Source Technology Centre