Messages in this thread | | | Date | Fri, 28 Aug 2015 10:51:47 -0700 | Subject | Re: Potential data race in psmouse_interrupt | From | Dmitry Torokhov <> |
| |
On Fri, Aug 28, 2015 at 10:34 AM, Dmitry Vyukov <dvyukov@google.com> wrote: > Hello, > > I am looking at this code in __ps2_command again: > > /* > * The reset command takes a long time to execute. > */ > timeout = msecs_to_jiffies(command == PS2_CMD_RESET_BAT ? 4000 : 500); > > timeout = wait_event_timeout(ps2dev->wait, > !(READ_ONCE(ps2dev->flags) & PS2_FLAG_CMD1), timeout); > > if (smp_load_acquire(&ps2dev->cmdcnt) && > !(smp_load_acquire(&ps2dev->flags) & PS2_FLAG_CMD1)) { > timeout = ps2_adjust_timeout(ps2dev, command, timeout); > wait_event_timeout(ps2dev->wait, > !(smp_load_acquire(&ps2dev->flags) & > PS2_FLAG_CMD), timeout); > } > > if (param) > for (i = 0; i < receive; i++) > param[i] = ps2dev->cmdbuf[(receive - 1) - i]; > > > Here are two moments I don't understand: > 1. The last parameter of ps2_adjust_timeout is timeout in jiffies (it > is compared against 100ms). However, timeout is assigned to result of > wait_event_timeout, which returns 0 or 1. This does not make sense to > me. What am I missing?
The fact that wait_event_timeout can return value greater than one:
* Returns: * 0 if the @condition evaluated to %false after the @timeout elapsed, * 1 if the @condition evaluated to %true after the @timeout elapsed, * or the remaining jiffies (at least 1) if the @condition evaluated ^^^^^^^^^^^^^^^^^^^^^^^^^
> 2. This code pays great attention to timeouts, but in the end I don't > see how it handles timeouts. That is, if a timeout is happened, we > still copyout (garbage) from cmdbuf. What am I missing here?
Once upon a time wait_event() did not return positive value when timeout expired and then condition satisfied. So we just examine the final state (psmpouse->cmdcnt should be 0 if command actually succeeded) and even if we copy in garbage nobody should care since we'll return error in this case.
Thanks.
-- Dmitry
| |