| Message ID | 20170802102919.13340-1-maarten.lankhorst@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 2017-08-02 at 12:29 +0200, Maarten Lankhorst wrote: > Export 2 functions, igt_signal_helper_get_num and > igt_signal_helper_get_hz. > > This will allow tests to measure how much time in a test was spent > in a uninterruptible state, which is useful when testing whether > certain ioctl's can be interrupted or not. > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > --- > lib/igt_aux.c | 30 +++++++++++++++++++++++++++--- > lib/igt_aux.h | 2 ++ > 2 files changed, 29 insertions(+), 3 deletions(-) > > diff --git a/lib/igt_aux.c b/lib/igt_aux.c > index 86a213c2032f..265e43f399e7 100644 > --- a/lib/igt_aux.c > +++ b/lib/igt_aux.c > @@ -275,7 +275,7 @@ bool __igt_sigiter_continue(struct __igt_sigiter > *iter, bool enable) > } > > static struct igt_helper_process signal_helper; > -long long int sig_stat; > +static int64_t sig_stat; > static void __attribute__((noreturn)) signal_helper_process(pid_t > pid) > { > /* Interrupt the parent process at 500Hz, just to be > annoying */ > @@ -314,6 +314,9 @@ void igt_fork_signal_helper(void) > if (igt_only_list_subtests()) > return; > > + /* Reset number of signalscaught */ > + sig_stat = 0; > + > /* We pick SIGCONT as it is a "safe" signal - if we send > SIGCONT to > * an unexpecting process it spuriously wakes up and does > nothing. > * Most other signals (e.g. SIGUSR1) cause the process to > die if they > @@ -348,8 +351,29 @@ void igt_stop_signal_helper(void) > return; > > igt_stop_helper(&signal_helper); > +} > > - sig_stat = 0; > +/** > + * igt_signal_helper_get_num: > + * > + * Return the amount of signals generated since the last time > + * igt_fork_signal_helper() was called. > + * > + * This is reset to 0 on every call to igt_fork_signal_helper. > + */ > +int64_t igt_signal_helper_get_num(void) > +{ > + return sig_stat; > +} > + > +/** > + * igt_signal_helper_get_hz: > + * > + * Return the approximate amount of signals generated per second. > + */ > +int igt_signal_helper_get_hz(void) > +{ > + return 50; > } I wonder if this is really necessary? The function returns hardcoded value and it is only used in one place i.e. third patch of this series. > > static struct igt_helper_process shrink_helper; > @@ -357,7 +381,7 @@ static void __attribute__((noreturn)) > shrink_helper_process(int fd, pid_t pid) > { > while (1) { > igt_drop_caches_set(fd, DROP_SHRINK_ALL); > - usleep(1000 * 1000 / 50); > + usleep(1000 * 1000 / igt_signal_helper_get_hz()); > if (kill(pid, 0)) /* Parent has died, so must we. */ > exit(0); > } > diff --git a/lib/igt_aux.h b/lib/igt_aux.h > index 499a16796ebb..7e080089dcbc 100644 > --- a/lib/igt_aux.h > +++ b/lib/igt_aux.h > @@ -55,6 +55,8 @@ extern int num_trash_bos; > /* generally useful helpers */ > void igt_fork_signal_helper(void); > void igt_stop_signal_helper(void); > +int64_t igt_signal_helper_get_num(void); > +int igt_signal_helper_get_hz(void); > > void igt_fork_shrink_helper(int fd); > void igt_stop_shrink_helper(void);
Quoting Maarten Lankhorst (2017-08-02 11:29:17) > Export 2 functions, igt_signal_helper_get_num and > igt_signal_helper_get_hz. > > This will allow tests to measure how much time in a test was spent > in a uninterruptible state, which is useful when testing whether > certain ioctl's can be interrupted or not. Use sig_ioctl, the purpose of that wrapper is to measure whether or not it was interrupted. Refine it to suit your purposes, it is the surgical scalpel compared to the shotgun of signal_helper. -Chris
Op 04-08-17 om 09:50 schreef Chris Wilson: > Quoting Maarten Lankhorst (2017-08-02 11:29:17) >> Export 2 functions, igt_signal_helper_get_num and >> igt_signal_helper_get_hz. >> >> This will allow tests to measure how much time in a test was spent >> in a uninterruptible state, which is useful when testing whether >> certain ioctl's can be interrupted or not. > Use sig_ioctl, the purpose of that wrapper is to measure whether or not it > was interrupted. Refine it to suit your purposes, it is the surgical > scalpel compared to the shotgun of signal_helper. > -Chris I've been using the igt display helpers now so I don't have to worry about the ioctl's. Using sig_ioctl would mean having to perform the ioctl's myself, and that makes the code a lot more verbose..
Quoting Maarten Lankhorst (2017-08-07 10:45:38) > Op 04-08-17 om 09:50 schreef Chris Wilson: > > Quoting Maarten Lankhorst (2017-08-02 11:29:17) > >> Export 2 functions, igt_signal_helper_get_num and > >> igt_signal_helper_get_hz. > >> > >> This will allow tests to measure how much time in a test was spent > >> in a uninterruptible state, which is useful when testing whether > >> certain ioctl's can be interrupted or not. > > Use sig_ioctl, the purpose of that wrapper is to measure whether or not it > > was interrupted. Refine it to suit your purposes, it is the surgical > > scalpel compared to the shotgun of signal_helper. > > -Chris > > I've been using the igt display helpers now so I don't have to worry about the ioctl's. Using sig_ioctl would mean having to perform the ioctl's myself, and that makes the code a lot more verbose.. You know that we igt uses igt_ioctl so that we can switch the ioctl wrapper. What you have here is sig_ioctl so rather than painting the shotgun signal helper a different colour, use the interface that actually supports what you have in mind. -Chris >
On Mon, Aug 07, 2017 at 10:59:55AM +0100, Chris Wilson wrote: > Quoting Maarten Lankhorst (2017-08-07 10:45:38) > > Op 04-08-17 om 09:50 schreef Chris Wilson: > > > Quoting Maarten Lankhorst (2017-08-02 11:29:17) > > >> Export 2 functions, igt_signal_helper_get_num and > > >> igt_signal_helper_get_hz. > > >> > > >> This will allow tests to measure how much time in a test was spent > > >> in a uninterruptible state, which is useful when testing whether > > >> certain ioctl's can be interrupted or not. > > > Use sig_ioctl, the purpose of that wrapper is to measure whether or not it > > > was interrupted. Refine it to suit your purposes, it is the surgical > > > scalpel compared to the shotgun of signal_helper. > > > -Chris > > > > I've been using the igt display helpers now so I don't have to worry about the ioctl's. Using sig_ioctl would mean having to perform the ioctl's myself, and that makes the code a lot more verbose.. > > You know that we igt uses igt_ioctl so that we can switch the ioctl > wrapper. What you have here is sig_ioctl so rather than painting the > shotgun signal helper a different colour, use the interface that > actually supports what you have in mind. Might be good to spicy the docs up a bit to make this clearer, maybe even with an example and some links between the different bits? Mika/Maarten? -Daniel
diff --git a/lib/igt_aux.c b/lib/igt_aux.c index 86a213c2032f..265e43f399e7 100644 --- a/lib/igt_aux.c +++ b/lib/igt_aux.c @@ -275,7 +275,7 @@ bool __igt_sigiter_continue(struct __igt_sigiter *iter, bool enable) } static struct igt_helper_process signal_helper; -long long int sig_stat; +static int64_t sig_stat; static void __attribute__((noreturn)) signal_helper_process(pid_t pid) { /* Interrupt the parent process at 500Hz, just to be annoying */ @@ -314,6 +314,9 @@ void igt_fork_signal_helper(void) if (igt_only_list_subtests()) return; + /* Reset number of signalscaught */ + sig_stat = 0; + /* We pick SIGCONT as it is a "safe" signal - if we send SIGCONT to * an unexpecting process it spuriously wakes up and does nothing. * Most other signals (e.g. SIGUSR1) cause the process to die if they @@ -348,8 +351,29 @@ void igt_stop_signal_helper(void) return; igt_stop_helper(&signal_helper); +} - sig_stat = 0; +/** + * igt_signal_helper_get_num: + * + * Return the amount of signals generated since the last time + * igt_fork_signal_helper() was called. + * + * This is reset to 0 on every call to igt_fork_signal_helper. + */ +int64_t igt_signal_helper_get_num(void) +{ + return sig_stat; +} + +/** + * igt_signal_helper_get_hz: + * + * Return the approximate amount of signals generated per second. + */ +int igt_signal_helper_get_hz(void) +{ + return 50; } static struct igt_helper_process shrink_helper; @@ -357,7 +381,7 @@ static void __attribute__((noreturn)) shrink_helper_process(int fd, pid_t pid) { while (1) { igt_drop_caches_set(fd, DROP_SHRINK_ALL); - usleep(1000 * 1000 / 50); + usleep(1000 * 1000 / igt_signal_helper_get_hz()); if (kill(pid, 0)) /* Parent has died, so must we. */ exit(0); } diff --git a/lib/igt_aux.h b/lib/igt_aux.h index 499a16796ebb..7e080089dcbc 100644 --- a/lib/igt_aux.h +++ b/lib/igt_aux.h @@ -55,6 +55,8 @@ extern int num_trash_bos; /* generally useful helpers */ void igt_fork_signal_helper(void); void igt_stop_signal_helper(void); +int64_t igt_signal_helper_get_num(void); +int igt_signal_helper_get_hz(void); void igt_fork_shrink_helper(int fd); void igt_stop_shrink_helper(void);
Export 2 functions, igt_signal_helper_get_num and igt_signal_helper_get_hz. This will allow tests to measure how much time in a test was spent in a uninterruptible state, which is useful when testing whether certain ioctl's can be interrupted or not. Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- lib/igt_aux.c | 30 +++++++++++++++++++++++++++--- lib/igt_aux.h | 2 ++ 2 files changed, 29 insertions(+), 3 deletions(-)