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