diff options
author | David Herrmann <dh.herrmann@gmail.com> | 2014-07-29 11:14:15 -0400 |
---|---|---|
committer | Jiri Kosina <jkosina@suse.cz> | 2014-08-25 04:28:05 -0400 |
commit | 0e0d7520064c9f5668c030afafdbcab242176195 (patch) | |
tree | 065b9a6d5e0cd495b315040941cc93df32a7617f | |
parent | cee5aa1f81b1e88d8de05df226bfd207041954c2 (diff) |
HID: uhid: simplify report-cb shutdown
The report-query is blocking, so when user-space destroys a device we have
to wake up any blocking kernel context that is currently in the report-cb.
We used some broken correlation between @report_done and @running so far.
Replace it by a much more obvious use.
We now wake up the report-cb if either @report_done or @running is set.
wake_up() and wait_event() serve as implicit barriers (as they always do)
so no need to use smp_rmb/wmb directly.
Note that @report_done is never reset by anyone but the report-cb, thus
it cannot flip twice while we wait for it. And whenever we set @running,
we afterwards synchronously remove the HID device. Therefore, we wait for
all report-cbs to finish before we return. This way, @running can never
flip to true while we wait for it.
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
-rw-r--r-- | drivers/hid/uhid.c | 11 |
1 files changed, 2 insertions, 9 deletions
diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c index 0cb92e347258..16af4d382391 100644 --- a/drivers/hid/uhid.c +++ b/drivers/hid/uhid.c | |||
@@ -172,13 +172,9 @@ static int uhid_hid_get_raw(struct hid_device *hid, unsigned char rnum, | |||
172 | spin_unlock_irqrestore(&uhid->qlock, flags); | 172 | spin_unlock_irqrestore(&uhid->qlock, flags); |
173 | 173 | ||
174 | ret = wait_event_interruptible_timeout(uhid->report_wait, | 174 | ret = wait_event_interruptible_timeout(uhid->report_wait, |
175 | atomic_read(&uhid->report_done), 5 * HZ); | 175 | atomic_read(&uhid->report_done) || !uhid->running, |
176 | 5 * HZ); | ||
176 | 177 | ||
177 | /* | ||
178 | * Make sure "uhid->running" is cleared on shutdown before | ||
179 | * "uhid->report_done" is set. | ||
180 | */ | ||
181 | smp_rmb(); | ||
182 | if (!ret || !uhid->running) { | 178 | if (!ret || !uhid->running) { |
183 | ret = -EIO; | 179 | ret = -EIO; |
184 | } else if (ret < 0) { | 180 | } else if (ret < 0) { |
@@ -493,10 +489,7 @@ static int uhid_dev_destroy(struct uhid_device *uhid) | |||
493 | if (!uhid->running) | 489 | if (!uhid->running) |
494 | return -EINVAL; | 490 | return -EINVAL; |
495 | 491 | ||
496 | /* clear "running" before setting "report_done" */ | ||
497 | uhid->running = false; | 492 | uhid->running = false; |
498 | smp_wmb(); | ||
499 | atomic_set(&uhid->report_done, 1); | ||
500 | wake_up_interruptible(&uhid->report_wait); | 493 | wake_up_interruptible(&uhid->report_wait); |
501 | 494 | ||
502 | hid_destroy_device(uhid->hid); | 495 | hid_destroy_device(uhid->hid); |