On Wednesday 07 June 2006 6:11 am, Franck Bui-Huu wrote:
> When closing the device, the driver acquires/release twice the
> port lock before/after waiting for the data to be completely
> sent. Therefore it will dead lock.
> 
> This patch fixes it and also uses the generic scheduler services
> for waiting for an event.

I don't recall what Al's reasons were for his custom versions,
but they haven't caught on (or is it:  core routines now fixed?)
so maybe it's right to remove them.


One question though:

> +#define GS_WRITE_FINISHED_EVENT_SAFELY(p)                    \
> +({                                                           \
> +     unsigned long flags;                                    \
> +     int cond;                                               \
> +                                                             \
> +     spin_lock_irqsave(&(p)->port_lock, flags);              \
> +     cond = !(p)->port_dev || !gs_buf_data_avail((p)->port_write_buf); \
> +     spin_unlock_irqrestore(&(p)->port_lock, flags);         \

Wouldn't spin_lock_irq()/spin_unlock_irq() be better here?

After all, if wait_event_interruptible_timeout() is permitted,
you know IRQs are never blocked so that the save/restore of
those flags will be pointless.  That observation would seem
to apply througout the gs_close() routine.  It's called from
drivers/char/tty_io.c AFAICT, which even gets semaphores.

So the cheaper calls would seem appropriate here.

- Dave


> +     cond;                                                   \
> +})   
> +
>  static void gs_close(struct tty_struct *tty, struct file *file)
>  {
>       unsigned long flags;
> @@ -888,10 +828,9 @@ static void gs_close(struct tty_struct *
>       /* at most GS_CLOSE_TIMEOUT seconds */
>       if (gs_buf_data_avail(port->port_write_buf) > 0) {
>               spin_unlock_irqrestore(&port->port_lock, flags);
> -             wait_cond_interruptible_timeout(port->port_write_wait,
> -             port->port_dev == NULL
> -             || gs_buf_data_avail(port->port_write_buf) == 0,
> -             &port->port_lock, flags, GS_CLOSE_TIMEOUT * HZ);
> +             wait_event_interruptible_timeout(port->port_write_wait,
> +                                     GS_WRITE_FINISHED_EVENT_SAFELY(port),
> +                                     GS_CLOSE_TIMEOUT * HZ);
>               spin_lock_irqsave(&port->port_lock, flags);
>       }
> 


_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to