Re: [Intel-gfx] [PATCH i-g-t 1/3] lib/igt_aux: Export statistics of signal helper.

[Daniel Vetter]

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
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t 1/3] lib/igt_aux: Export statistics of signal helper.

[Chris Wilson]

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
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t 1/3] lib/igt_aux: Export statistics of signal helper.

[Maarten Lankhorst]

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..

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t 1/3] lib/igt_aux: Export statistics of signal helper.

[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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t 1/3] lib/igt_aux: Export statistics of signal helper.

[Mika Kahola]

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 
> ---
>  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(_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);
-- 
Mika Kahola - Intel OTC

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx