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);
> }
>
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
