Hi,
Sorry, please ignore the patch below.
The following lines should be in the reverse order (first we should
store the prev_msr_value and
later the current state):
> + set_current_state(TASK_INTERRUPTIBLE);
> prev_msr_value = port_extra->shadowMSR & QT2_SERIAL_MSR_MASK;
Because of the above 2 lines, we might not be able to detect the
change in line 2 while
the task state might change to running.
Result: we would return from this function with -EIO.
Of course, the usermode app could do another ioctl for waiting for
change in status, but
this would be an error in my logic.
I will post another patch for this tomorrow.
On Tue, Sep 13, 2011 at 11:23 AM, Kautuk Consul <consul.kautuk@xxxxxxxxx> wrote:
> If the usermode app does an ioctl over this serial device by
> using TIOCMIWAIT, then the code will wait by setting the current
> task state to TASK_INTERRUPTIBLE and then calling schedule().
> This will be woken up by the qt2_process_modem_status on URB
> completion when the port_extra->shadowMSR is set to the new
> modem status.
>
> However, this could result in a lost wakeup scenario due to a race
> in the logic in the qt2_ioctl(TIOCMIWAIT) loop and the URB completion
> for new modem status in qt2_process_modem_status.
> Due to this, the usermode app's task will continue to sleep despite a
> change in the modem status.
>
> Signed-off-by: Kautuk Consul <consul.kautuk@xxxxxxxxx>
> ---
> drivers/staging/quatech_usb2/quatech_usb2.c | 8 ++++++--
> 1 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/quatech_usb2/quatech_usb2.c b/drivers/staging/quatech_usb2/quatech_usb2.c
> index ca098ca..af7cba2 100644
> --- a/drivers/staging/quatech_usb2/quatech_usb2.c
> +++ b/drivers/staging/quatech_usb2/quatech_usb2.c
> @@ -915,10 +915,10 @@ static int qt2_ioctl(struct tty_struct *tty,
> } else if (cmd == TIOCMIWAIT) {
> dbg("%s() port %d, cmd == TIOCMIWAIT enter",
> __func__, port->number);
> + set_current_state(TASK_INTERRUPTIBLE);
> prev_msr_value = port_extra->shadowMSR & QT2_SERIAL_MSR_MASK;
> while (1) {
> add_wait_queue(&port_extra->wait, &wait);
> - set_current_state(TASK_INTERRUPTIBLE);
> schedule();
> dbg("%s(): port %d, cmd == TIOCMIWAIT here\n",
> __func__, port->number);
> @@ -926,9 +926,12 @@ static int qt2_ioctl(struct tty_struct *tty,
> /* see if a signal woke us up */
> if (signal_pending(current))
> return -ERESTARTSYS;
> + set_current_state(TASK_INTERRUPTIBLE);
> msr_value = port_extra->shadowMSR & QT2_SERIAL_MSR_MASK;
> - if (msr_value == prev_msr_value)
> + if (msr_value == prev_msr_value) {
> + __set_current_state(TASK_RUNNING);
> return -EIO; /* no change - error */
> + }
> if ((arg & TIOCM_RNG &&
> ((prev_msr_value & QT2_SERIAL_MSR_RI) ==
> (msr_value & QT2_SERIAL_MSR_RI))) ||
> @@ -941,6 +944,7 @@ static int qt2_ioctl(struct tty_struct *tty,
> (arg & TIOCM_CTS &&
> ((prev_msr_value & QT2_SERIAL_MSR_CTS) ==
> (msr_value & QT2_SERIAL_MSR_CTS)))) {
> + __set_current_state(TASK_RUNNING);
> return 0;
> }
> } /* end inifinite while */
> --
> 1.7.6
>
>
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
