2004-04-05 04:47:06

by Robert White

[permalink] [raw]
Subject: [PATCH] (linux 2.4.25) hangup on disconnect for usbserial module

This is "reasonably well tested" on the x86 platform.


This patch fixes a problem where the usbserial code would not notify
connected programs that the serial port was going away.


--- linux-2.4.25-orig/drivers/usb/serial/usbserial.c 2003-11-28
10:26:20.000000000 -0800
+++ linux-2.4.25/drivers/usb/serial/usbserial.c 2004-04-04
21:26:34.000000000 -0700
@@ -14,6 +14,10 @@
*
* See Documentation/usb/usb-serial.txt for more information on using this
driver
*
+ * (04/04/2004) [email protected]
+ * usb_serial_disconnect() now calls tty_hangup() so that programs
+ * using the device can/will notice that the device is going away.
+ *
* (10/10/2001) gkh
* usb_serial_disconnect() now sets the serial->dev pointer is to NULL
to
* help prevent child drivers from accessing the device since it is now
@@ -1404,9 +1408,11 @@ static void usb_serial_disconnect(struct
for (i = 0; i < serial->num_ports; ++i) {
port = &serial->port[i];
down (&port->sem);
- if (port->tty != NULL)
+ if (port->tty != NULL) {
+ tty_hangup(port->tty);
while (port->open_count > 0)
__serial_close(port, NULL);
+ }
up (&port->sem);
}




2004-04-05 05:14:41

by Pete Zaitcev

[permalink] [raw]
Subject: Re: [linux-usb-devel] [PATCH] (linux 2.4.25) hangup on disconnect for usbserial module

On Sun, 4 Apr 2004 21:46:52 -0700
"Robert White" <[email protected]> wrote:

> This is "reasonably well tested" on the x86 platform.
>
> This patch fixes a problem where the usbserial code would not notify
> connected programs that the serial port was going away.

> @@ -1404,9 +1408,11 @@ static void usb_serial_disconnect(struct
> for (i = 0; i < serial->num_ports; ++i) {
> port = &serial->port[i];
> down (&port->sem);
> - if (port->tty != NULL)
> + if (port->tty != NULL) {
> + tty_hangup(port->tty);
> while (port->open_count > 0)
> __serial_close(port, NULL);
> + }

I'll think about it.

If Greg approves and takes, it's fine, too.

What is the actual symptom? Did you expect a SIGHUP?

-- Pete

2004-04-05 05:50:22

by Robert White

[permalink] [raw]
Subject: RE: [linux-usb-devel] [PATCH] (linux 2.4.25) hangup on disconnect for usbserial module

Pasting the whole program is impractical, so here is the psudocode

Open /dev/usb/ttyUSB0
Build Poll structure with events = POLLIN for this descriptor
Call poll(&structure,1,-1)

(Without the patch) If you pull the usb cable out of the computer, the program above
will never return from the poll. I was quite surprised, especially since this didn't
match the behavior of the ACM device that we alternately use in our box. The hang is
semi-terminal too, as nothing can reattach to the file descriptor from below.

If you use /dev/usb/ttyACM0 in the identical program (obviously also against a
different device 8-) when you pull the plug poll returns immediately and revents has
the (I think) POLLHUP bit set. (I am not certain this is the value, the actual
program considers all three of POLLHUP, POLLNVAL and POLLERR equally terminal in this
case and doesn't emit the actual value, but it is at least one of those. 8-)

The hangup semantic with all its ramifications (sighup if the device is a controlling
terminal etc.) is "good for me", and matches other kinds of (USB and non-USB)
devices, but I have no opinion on particular apps. I considered adding an IOCTL and
flag to control this but decided against for the following reasons:

The poll management happens at the far side of the tty layer, and this was the only
non-intrusive and portable way I could figure out to pass the disconnect event
through the that layer.

The CLOCAL flag already acts as a means to enable/disable tty_hangup()'s effects and
is mature and well documented as an interface feature.

There was no clear way to propagate this event that wasn't either far more delicate,
far reaching, or disruptive.


Rob.


-----Original Message-----
From: Pete Zaitcev [mailto:[email protected]]
Sent: Sunday, April 04, 2004 10:15 PM
To: Robert White
Cc: [email protected]; [email protected]
Subject: Re: [linux-usb-devel] [PATCH] (linux 2.4.25) hangup on disconnect for
usbserial module

On Sun, 4 Apr 2004 21:46:52 -0700
"Robert White" <[email protected]> wrote:

> This is "reasonably well tested" on the x86 platform.
>
> This patch fixes a problem where the usbserial code would not notify
> connected programs that the serial port was going away.

> @@ -1404,9 +1408,11 @@ static void usb_serial_disconnect(struct
> for (i = 0; i < serial->num_ports; ++i) {
> port = &serial->port[i];
> down (&port->sem);
> - if (port->tty != NULL)
> + if (port->tty != NULL) {
> + tty_hangup(port->tty);
> while (port->open_count > 0)
> __serial_close(port, NULL);
> + }

I'll think about it.

If Greg approves and takes, it's fine, too.

What is the actual symptom? Did you expect a SIGHUP?

-- Pete



2004-04-05 08:11:07

by Oliver Neukum

[permalink] [raw]
Subject: Re: [linux-usb-devel] [PATCH] (linux 2.4.25) hangup on disconnect for usbserial module

Am Montag, 5. April 2004 07:49 schrieb Robert White:
> Pasting the whole program is impractical, so here is the psudocode
>
> Open /dev/usb/ttyUSB0
> Build Poll structure with events = POLLIN for this descriptor
> Call poll(&structure,1,-1)
>
> (Without the patch) If you pull the usb cable out of the computer, the
> program above will never return from the poll. I was quite surprised,
> especially since this didn't match the behavior of the ACM device that we
> alternately use in our box. The hang is semi-terminal too, as nothing can
> reattach to the file descriptor from below.

This is clearly unacceptable behavior.

[..]
> The hangup semantic with all its ramifications (sighup if the device is a
> controlling terminal etc.) is "good for me", and matches other kinds of
> (USB and non-USB) devices, but I have no opinion on particular apps. I
> considered adding an IOCTL and flag to control this but decided against for
> the following reasons:

Unless you want to go for reattaching, which I wouldn't consider a good idea,
I see no other choice.
I did a quick look at the drivers and the other driver hanging there is usb-midi.
I haven't checked the serial drivers not using generic_disconnect() though.

Regards
Oliver