[xserver] input: fix VT switch disabling devices

[Hans de Goede]

Hi,

On 24-09-16 19:55, Mihail Konev wrote:
> Make sure device removes are processed before doing a VT switch,
> so that no removes are "overwritten" with attachments afterwards
> (before the main thread releases the input lock, letting them be
> processed), which would leave affected devices disabled.
>
> Pass a timeout to input poll instead of telling it to wait forever,
> so that no deadlock before VT switch is possible if main thread
> waits for release processing only by the time input thread
> is already done and does another poll.
>
> When preparing a VT switch, temprorarily decrease polling
> timeout, so there is (likely) no noticeable stall.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97880
> Regressed-in: 52d6a1e832a5e62289dd4f32824ae16a78dfd7e8
> Signed-off-by: Mihail Konev <k.mvc at ya.ru>

Thank you for the patch and you're right that there is an issue here.

I've posted a much simpler fix yesterday, and that has been
favorably reviewed, so I think we're going to go with that fix
instead, see:

https://patchwork.freedesktop.org/patch/113763/

Regards,

Hans


> ---
>
> The patch could be wrong with regard to namings (InputThreadWait vs. input_wait)
> and multiple input threads (struct InputThreadInfo).
>
>  hw/xfree86/common/xf86Events.c |  4 ++++
>  include/input.h                |  4 ++++
>  os/inputthread.c               | 39 ++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 46 insertions(+), 1 deletion(-)
>
> diff --git a/hw/xfree86/common/xf86Events.c b/hw/xfree86/common/xf86Events.c
> index 9a8f432a0556..8290c117116e 100644
> --- a/hw/xfree86/common/xf86Events.c
> +++ b/hw/xfree86/common/xf86Events.c
> @@ -430,6 +430,7 @@ xf86VTLeave(void)
>       * Keep the order: Disable Device > LeaveVT
>       *                        EnterVT > EnableDevice
>       */
> +    InputThreadPollTimeoutSmall();
>      for (ih = InputHandlers; ih; ih = ih->next) {
>          if (ih->is_input)
>              xf86DisableInputHandler(ih);
> @@ -439,6 +440,9 @@ xf86VTLeave(void)
>      for (pInfo = xf86InputDevs; pInfo; pInfo = pInfo->next)
>          xf86DisableInputDeviceForVTSwitch(pInfo);
>
> +    InputThreadWaitForProcessingAllRemoves();
> +    InputThreadPollTimeoutNormal();
> +
>      input_lock();
>      for (i = 0; i < xf86NumScreens; i++)
>          xf86Screens[i]->LeaveVT(xf86Screens[i]);
> diff --git a/include/input.h b/include/input.h
> index e0f6b9b01d97..1afb3f89a33a 100644
> --- a/include/input.h
> +++ b/include/input.h
> @@ -719,6 +719,10 @@ extern _X_EXPORT void input_lock(void);
>  extern _X_EXPORT void input_unlock(void);
>  extern _X_EXPORT void input_force_unlock(void);
>
> +extern _X_EXPORT void InputThreadPollTimeoutSmall(void);
> +extern _X_EXPORT void InputThreadPollTimeoutNormal(void);
> +extern _X_EXPORT void InputThreadWaitForProcessingAllRemoves(void);
> +
>  extern void InputThreadPreInit(void);
>  extern void InputThreadInit(void);
>  extern void InputThreadFini(void);
> diff --git a/os/inputthread.c b/os/inputthread.c
> index 6aa0a9ce6fb5..fab3c30954e5 100644
> --- a/os/inputthread.c
> +++ b/os/inputthread.c
> @@ -90,6 +90,12 @@ static pthread_mutex_t input_mutex;
>  static Bool input_mutex_initialized;
>  #endif
>
> +int input_poll_timeout_ms;
> +
> +static Bool waiting_for_processing_all_removes = FALSE;
> +static int unprocessed_removes_count = 0;
> +static pthread_barrier_t all_removes_processed_barrier;
> +
>  void
>  input_lock(void)
>  {
> @@ -125,6 +131,25 @@ input_force_unlock(void)
>      }
>  }
>
> +void
> +InputThreadPollTimeoutSmall(void)
> +{
> +    input_poll_timeout_ms = 4;
> +}
> +
> +void
> +InputThreadPollTimeoutNormal(void)
> +{
> +    input_poll_timeout_ms = 200;
> +}
> +
> +void
> +InputThreadWaitForProcessingAllRemoves(void)
> +{
> +    waiting_for_processing_all_removes = TRUE;
> +    pthread_barrier_wait(&all_removes_processed_barrier);
> +}
> +
>  /**
>   * Notify a thread about the availability of new asynchronously enqueued input
>   * events.
> @@ -267,6 +292,7 @@ InputThreadUnregisterDev(int fd)
>
>      dev->state = device_state_removed;
>      inputThreadInfo->changed = TRUE;
> +    unprocessed_removes_count++;
>
>      input_unlock();
>
> @@ -348,13 +374,19 @@ InputThreadDoWork(void *arg)
>                      ospoll_remove(inputThreadInfo->fds, dev->fd);
>                      xorg_list_del(&dev->node);
>                      free(dev);
> +                    unprocessed_removes_count--;
>                      break;
>                  }
>              }
>              input_unlock();
>          }
>
> -        if (ospoll_wait(inputThreadInfo->fds, -1) < 0) {
> +        if (waiting_for_processing_all_removes && !unprocessed_removes_count) {
> +            waiting_for_processing_all_removes = FALSE;
> +            pthread_barrier_wait(&all_removes_processed_barrier);
> +        }
> +
> +        if (ospoll_wait(inputThreadInfo->fds, input_poll_timeout_ms) < 0) {
>              if (errno == EINVAL)
>                  FatalError("input-thread: %s (%s)", __func__, strerror(errno));
>              else if (errno != EINTR)
> @@ -400,6 +432,8 @@ InputThreadPreInit(void)
>      if (!inputThreadInfo)
>          FatalError("input-thread: could not allocate memory");
>
> +    pthread_barrier_init(&all_removes_processed_barrier, NULL, 2);
> +
>      inputThreadInfo->thread = 0;
>      xorg_list_init(&inputThreadInfo->devs);
>      inputThreadInfo->fds = ospoll_create();
> @@ -434,6 +468,7 @@ InputThreadPreInit(void)
>      pthread_setname_np ("MainThread");
>  #endif
>
> +    InputThreadPollTimeoutNormal();
>  }
>
>  /**
> @@ -517,6 +552,8 @@ int xthread_sigmask(int how, const sigset_t *set, sigset_t *oldset)
>
>  Bool InputThreadEnable = FALSE;
>
> +void input_wait_for_processing_all_removes(void) {};
> +
>  void input_lock(void) {}
>  void input_unlock(void) {}
>  void input_force_unlock(void) {}
>