[xserver] input: fix VT switch disabling devices
Hans de Goede
hdegoede at redhat.com
Thu Oct 6 11:04:07 UTC 2016
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) {}
>
More information about the xorg-devel
mailing list