This fixes a race with the release-timer by adding acquire/release barrier semantics.
I noticed that contacts were sometimes sticking, even with the "sticky fingers" quirk enabled. This fixes that problem.
Cc: stable@vger.kernel.org Signed-off-by: Andri Yngvason andri@yngvason.is --- drivers/hid/hid-multitouch.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c index 2e72922e36f5..91a4d3fc30e0 100644 --- a/drivers/hid/hid-multitouch.c +++ b/drivers/hid/hid-multitouch.c @@ -1186,7 +1186,7 @@ static void mt_touch_report(struct hid_device *hid, int contact_count = -1;
/* sticky fingers release in progress, abort */ - if (test_and_set_bit(MT_IO_FLAGS_RUNNING, &td->mt_io_flags)) + if (test_and_set_bit_lock(MT_IO_FLAGS_RUNNING, &td->mt_io_flags)) return;
scantime = *app->scantime; @@ -1267,7 +1267,7 @@ static void mt_touch_report(struct hid_device *hid, del_timer(&td->release_timer); }
- clear_bit(MT_IO_FLAGS_RUNNING, &td->mt_io_flags); + clear_bit_unlock(MT_IO_FLAGS_RUNNING, &td->mt_io_flags); }
static int mt_touch_input_configured(struct hid_device *hdev, @@ -1699,11 +1699,11 @@ static void mt_expired_timeout(struct timer_list *t) * An input report came in just before we release the sticky fingers, * it will take care of the sticky fingers. */ - if (test_and_set_bit(MT_IO_FLAGS_RUNNING, &td->mt_io_flags)) + if (test_and_set_bit_lock(MT_IO_FLAGS_RUNNING, &td->mt_io_flags)) return; if (test_bit(MT_IO_FLAGS_PENDING_SLOTS, &td->mt_io_flags)) mt_release_contacts(hdev); - clear_bit(MT_IO_FLAGS_RUNNING, &td->mt_io_flags); + clear_bit_unlock(MT_IO_FLAGS_RUNNING, &td->mt_io_flags); }
static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
On Wed, Aug 17, 2022 at 11:32:48AM +0000, Andri Yngvason wrote:
This fixes a race with the release-timer by adding acquire/release barrier semantics.
What race?
I noticed that contacts were sometimes sticking, even with the "sticky fingers" quirk enabled. This fixes that problem.
Cc: stable@vger.kernel.org Signed-off-by: Andri Yngvason andri@yngvason.is
drivers/hid/hid-multitouch.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c index 2e72922e36f5..91a4d3fc30e0 100644 --- a/drivers/hid/hid-multitouch.c +++ b/drivers/hid/hid-multitouch.c @@ -1186,7 +1186,7 @@ static void mt_touch_report(struct hid_device *hid, int contact_count = -1; /* sticky fingers release in progress, abort */
- if (test_and_set_bit(MT_IO_FLAGS_RUNNING, &td->mt_io_flags))
- if (test_and_set_bit_lock(MT_IO_FLAGS_RUNNING, &td->mt_io_flags))
So this is now a lock?
Why not just use a real lock instead?
thanks,
greg k-h
Hi Greg,
mið., 17. ágú. 2022 kl. 11:59 skrifaði Greg KH gregkh@linuxfoundation.org:
On Wed, Aug 17, 2022 at 11:32:48AM +0000, Andri Yngvason wrote:
This fixes a race with the release-timer by adding acquire/release barrier semantics.
What race?
The race is between the release timer and processing of hid input. It is quite certain that these atomic checks are broken as is because they're lacking memory barriers and this patch does fix an actual problem for me.
I must admit that I don't know exactly where the execution reordering occurs and I haven't checked the generated assembly code, but if you look at e.g. mt_expired_timeout(), you'll see that there's nothing to keep the compiler or CPU from hoisting the test_bit() call above the test_and_set_bit() call.
I noticed that contacts were sometimes sticking, even with the "sticky fingers" quirk enabled. This fixes that problem.
Cc: stable@vger.kernel.org Signed-off-by: Andri Yngvason andri@yngvason.is
drivers/hid/hid-multitouch.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c index 2e72922e36f5..91a4d3fc30e0 100644 --- a/drivers/hid/hid-multitouch.c +++ b/drivers/hid/hid-multitouch.c @@ -1186,7 +1186,7 @@ static void mt_touch_report(struct hid_device *hid, int contact_count = -1;
/* sticky fingers release in progress, abort */
if (test_and_set_bit(MT_IO_FLAGS_RUNNING, &td->mt_io_flags))
if (test_and_set_bit_lock(MT_IO_FLAGS_RUNNING, &td->mt_io_flags))So this is now a lock?
Why not just use a real lock instead?
I don't know. This stuff was introduced in 9609827458c37d7b2c37f2a9255631c603a5004c by Benjamin Tissoires.
Cheers, Andri
On Wed, Aug 17, 2022 at 04:14:20PM +0000, Andri Yngvason wrote:
Hi Greg,
mið., 17. ágú. 2022 kl. 11:59 skrifaði Greg KH gregkh@linuxfoundation.org:
On Wed, Aug 17, 2022 at 11:32:48AM +0000, Andri Yngvason wrote:
This fixes a race with the release-timer by adding acquire/release barrier semantics.
What race?
The race is between the release timer and processing of hid input. It is quite certain that these atomic checks are broken as is because they're lacking memory barriers and this patch does fix an actual problem for me.
Perhaps you should say that in this changelog then.
I must admit that I don't know exactly where the execution reordering occurs and I haven't checked the generated assembly code, but if you look at e.g. mt_expired_timeout(), you'll see that there's nothing to keep the compiler or CPU from hoisting the test_bit() call above the test_and_set_bit() call.
I noticed that contacts were sometimes sticking, even with the "sticky fingers" quirk enabled. This fixes that problem.
Cc: stable@vger.kernel.org Signed-off-by: Andri Yngvason andri@yngvason.is
drivers/hid/hid-multitouch.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c index 2e72922e36f5..91a4d3fc30e0 100644 --- a/drivers/hid/hid-multitouch.c +++ b/drivers/hid/hid-multitouch.c @@ -1186,7 +1186,7 @@ static void mt_touch_report(struct hid_device *hid, int contact_count = -1;
/* sticky fingers release in progress, abort */
if (test_and_set_bit(MT_IO_FLAGS_RUNNING, &td->mt_io_flags))
if (test_and_set_bit_lock(MT_IO_FLAGS_RUNNING, &td->mt_io_flags))So this is now a lock?
Why not just use a real lock instead?
I don't know. This stuff was introduced in 9609827458c37d7b2c37f2a9255631c603a5004c by Benjamin Tissoires.
Then this needs a "Fixes:" tag?
thanks,
greg k-h
linux-stable-mirror@lists.linaro.org
-
Andri Yngvason -
Greg KH