[igt-dev] [PATCH i-g-t v21 09/35] lib/intel_allocator: Try to stop smoothly instead of deinit

[Chris Wilson]

Quoting Zbigniew Kempczyński (2021-03-01 16:13:37)
> Avoid race when stop was send to allocator thread. We wait around
> 100 ms to give thread chance to stop smoothly instead of removing
> queue and enforcing exiting all blocked message syscalls.
> 
> Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
> Cc: Dominik Grzegorzek <dominik.grzegorzek at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>  lib/intel_allocator.c | 26 +++++++++++++++++++++++---
>  1 file changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/intel_allocator.c b/lib/intel_allocator.c
> index 114f9dc78..725a5c9d1 100644
> --- a/lib/intel_allocator.c
> +++ b/lib/intel_allocator.c
> @@ -105,6 +105,7 @@ static pthread_mutex_t map_mutex = PTHREAD_MUTEX_INITIALIZER;
>  
>  static bool multiprocess;
>  static pthread_t allocator_thread;
> +static bool allocator_thread_running;
>  
>  static bool warn_if_not_empty;
>  
> @@ -700,6 +701,8 @@ static void *allocator_thread_loop(void *data)
>                    (long) allocator_pid, (long) gettid());
>         alloc_info("Entering allocator loop\n");
>  
> +       WRITE_ONCE(allocator_thread_running, true);
> +
>         while (1) {
>                 ret = recv_req(channel, &req);
>  
> @@ -733,6 +736,8 @@ static void *allocator_thread_loop(void *data)
>                 }
>         }
>  
> +       WRITE_ONCE(allocator_thread_running, false);
> +
>         return NULL;
>  }
>  
> @@ -770,15 +775,30 @@ void intel_allocator_multiprocess_start(void)
>   * Function turns off intel_allocator multiprocess mode what means means
>   * stopping allocator thread and deinitializing its data.
>   */
> +#define STOP_TIMEOUT_MS 100
>  void intel_allocator_multiprocess_stop(void)
>  {
> +       int time_left = STOP_TIMEOUT_MS;
> +
>         alloc_info("allocator multiprocess stop\n");
>  
>         if (multiprocess) {
>                 send_alloc_stop(channel);
> -               /* Deinit, this should stop all blocked syscalls, if any */
> -               channel->deinit(channel);
> -               pthread_join(allocator_thread, NULL);
> +
> +               /* We prefer joining thread when it is stopped */
> +               while (time_left-- > 0 && READ_ONCE(allocator_thread_running))
> +                       usleep(1000); /* coarse calculation */
> +
> +               /* Thread has stuck somewhere */
> +               if (READ_ONCE(allocator_thread_running)) {
> +                       /* Deinit, this should stop all blocked syscalls, if any */
> +                       channel->deinit(channel);
> +                       pthread_join(allocator_thread, NULL);
> +               } else {
> +                       pthread_join(allocator_thread, NULL);
> +                       channel->deinit(channel);

If the allocator thread was known to not be running, wouldn't it be safe
to call channel->deinit() anyway?
-Chris