| Message ID | 1447850429-8303-1-git-send-email-marius.c.vlad@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Nov 18, 2015 at 02:40:29PM +0200, marius.c.vlad@intel.com wrote: > From: Marius Vlad <marius.c.vlad@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@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. > + if (ctx.surface == NULL) { > + fprintf(stderr, "Failed to create X11 overlay.\n"); Spurious changes. > 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? -Chris
-----Original Message----- From: Chris Wilson [mailto:chris@chris-wilson.co.uk] Sent: Wednesday, November 18, 2015 2:59 PM To: Vlad, Marius C <marius.c.vlad@intel.com> Cc: intel-gfx@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@intel.com wrote: > From: Marius Vlad <marius.c.vlad@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@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
On Wed, Nov 18, 2015 at 01:51:57PM +0000, Vlad, Marius C wrote: > > > -----Original Message----- > From: Chris Wilson [mailto:chris@chris-wilson.co.uk] > Sent: Wednesday, November 18, 2015 2:59 PM > To: Vlad, Marius C <marius.c.vlad@intel.com> > Cc: intel-gfx@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@intel.com wrote: > > From: Marius Vlad <marius.c.vlad@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@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. Yes, it is a known hw (on ivb/hsw) issue with concurrent access to registers. We need the i915 perf interface to avoid it (as then we can serialise all register access). -Chris
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(); + exit(EXIT_SUCCESS); +} + static int get_sample_period(struct config *config) { const char *value; @@ -854,6 +862,7 @@ int main(int argc, char **argv) }; struct overlay_context ctx; struct config config; + struct sigaction sa; int index, sample_period; int daemonize = 1, renice = 0; int i; @@ -898,14 +907,22 @@ int main(int argc, char **argv) ctx.width = 640; ctx.height = 236; ctx.surface = NULL; - if (ctx.surface == NULL) + + if (ctx.surface == NULL) { ctx.surface = x11_overlay_create(&config, &ctx.width, &ctx.height); - if (ctx.surface == NULL) + } + if (ctx.surface == NULL) { + fprintf(stderr, "Failed to create X11 overlay.\n"); 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) { + x11_overlay_stop(); + return EXIT_FAILURE; + } + + sa.sa_sigaction = &signal_x11_destroy; + if (sigaction(SIGINT | SIGTERM, &sa, NULL) == -1) { + x11_overlay_stop(); + return EXIT_FAILURE; + } debugfs_init();