2001-12-30 13:53:13

by Russell King

[permalink] [raw]
Subject: [PATCH] fix serial close hang

Hi,

2.4/2.5 kernels suffer from an infinitely long hang when a serial tty device
is closed, and there are characters waiting to be sent. The hang occurs in
tty_wait_until_sent.

There is a timeout 'closing_wait' which defines how long to wait for the TX
buffers to empty; the problem is that the serial layer totally ignores it.
It is stored in two structures, 'info' and 'state'. It is initialised in
the 'state' structure, but used from the 'info' structure.

It turns out that 'hub6' was also missing.

I'm not currently clear what the expected behaviour should be when the
timeout is changed via setserial, and others have the port open - I've
opted to preserve the timeout until all users close the port. It's
trivial to change this behaviour though.

Comments welcome.

--- orig/include/linux/serialP.h Sat Jul 21 10:47:15 2001
+++ linux/include/linux/serialP.h Sun Dec 30 13:09:27 2001
@@ -70,7 +70,7 @@
int x_char; /* xon/xoff character */
int close_delay;
unsigned short closing_wait;
- unsigned short closing_wait2;
+ unsigned short closing_wait2; /* obsolete */
int IER; /* Interrupt Enable Register */
int MCR; /* Modem control register */
int LCR; /* Line control register */
--- orig/drivers/char/serial.c Sun Dec 30 13:37:23 2001
+++ linux/drivers/char/serial.c Sun Dec 30 13:49:27 2001
@@ -3095,36 +3095,52 @@

sstate = rs_table + line;
sstate->count++;
- if (sstate->info) {
- *ret_info = sstate->info;
- return 0;
- }
+ info = sstate->info;
+
+ /*
+ * If the async_struct is already allocated, do the fastpath.
+ */
+ if (info)
+ goto out;
+
info = kmalloc(sizeof(struct async_struct), GFP_KERNEL);
if (!info) {
sstate->count--;
return -ENOMEM;
}
+
memset(info, 0, sizeof(struct async_struct));
init_waitqueue_head(&info->open_wait);
init_waitqueue_head(&info->close_wait);
init_waitqueue_head(&info->delta_msr_wait);
info->magic = SERIAL_MAGIC;
info->port = sstate->port;
+ info->hub6 = sstate->hub6;
info->flags = sstate->flags;
- info->io_type = sstate->io_type;
- info->iomem_base = sstate->iomem_base;
- info->iomem_reg_shift = sstate->iomem_reg_shift;
info->xmit_fifo_size = sstate->xmit_fifo_size;
+ info->state = sstate;
info->line = line;
+ info->iomem_base = sstate->iomem_base;
+ info->iomem_reg_shift = sstate->iomem_reg_shift;
+ info->io_type = sstate->io_type;
info->tqueue.routine = do_softint;
info->tqueue.data = info;
- info->state = sstate;
+
if (sstate->info) {
kfree(info);
- *ret_info = sstate->info;
- return 0;
+ info = state->info;
+ } else {
+ sstate->info = info;
+ }
+
+out:
+ /*
+ * If this is the first open, copy over some timeouts.
+ */
+ if (sstate->count == 1) {
+ info->closing_wait = sstate->closing_wait;
}
- *ret_info = sstate->info = info;
+ *ret_info = info;
return 0;
}



--
Russell King ([email protected]) The developer of ARM Linux
http://www.arm.linux.org.uk/personal/aboutme.html


2001-12-30 14:17:08

by Heinz Diehl

[permalink] [raw]
Subject: Re: [PATCH] fix serial close hang

On Sun Dec 30 2001, Russell King wrote:

> Comments welcome.

Patch applies flawlessly to 2.4.17, however compilation fails:

[....]
make[2]: Leaving directory /usr/src/linux/drivers/cdrom'
make -C char modules
make[2]: Entering directory /usr/src/linux/drivers/char'
gcc -D__KERNEL__ -I/usr/src/linux/include -Wall -Wstrict-prototypes
-Wno-trigraphs -O2 -fomit-frame-pointer -fno-strict-aliasing -fno-common
-pipe -mpreferred-stack-boundary=2 -march=k6 -DMODULE -DEXPORT_SYMTAB -c
serial.c
serial.c: In function get_async_struct':
serial.c:3131: state' undeclared (first use in this function)
serial.c:3131: (Each undeclared identifier is reported only once
serial.c:3131: for each function it appears in.)
make[2]: *** [serial.o] Error 1
make[2]: Leaving directory /usr/src/linux/drivers/char'
make[1]: *** [_modsubdir_char] Error 2
make[1]: Leaving directory /usr/src/linux/drivers'
make: *** [_mod_drivers] Error 2
elfie:/usr/src/linux #

--
# Heinz Diehl, 68259 Mannheim, Germany

2001-12-30 14:43:10

by Russell King

[permalink] [raw]
Subject: Re: [PATCH] fix serial close hang

On Sun, Dec 30, 2001 at 03:17:31PM +0100, Heinz Diehl wrote:
> serial.c:3131: state' undeclared (first use in this function)

Whoops. This should fix it.

--- orig/include/linux/serialP.h Sat Jul 21 10:47:15 2001
+++ linux/include/linux/serialP.h Sun Dec 30 13:09:27 2001
@@ -70,7 +70,7 @@
int x_char; /* xon/xoff character */
int close_delay;
unsigned short closing_wait;
- unsigned short closing_wait2;
+ unsigned short closing_wait2; /* obsolete */
int IER; /* Interrupt Enable Register */
int MCR; /* Modem control register */
int LCR; /* Line control register */
--- orig/drivers/char/serial.c Sun Dec 30 13:37:23 2001
+++ linux/drivers/char/serial.c Sun Dec 30 13:49:27 2001
@@ -3095,36 +3095,52 @@

sstate = rs_table + line;
sstate->count++;
- if (sstate->info) {
- *ret_info = sstate->info;
- return 0;
- }
+ info = sstate->info;
+
+ /*
+ * If the async_struct is already allocated, do the fastpath.
+ */
+ if (info)
+ goto out;
+
info = kmalloc(sizeof(struct async_struct), GFP_KERNEL);
if (!info) {
sstate->count--;
return -ENOMEM;
}
+
memset(info, 0, sizeof(struct async_struct));
init_waitqueue_head(&info->open_wait);
init_waitqueue_head(&info->close_wait);
init_waitqueue_head(&info->delta_msr_wait);
info->magic = SERIAL_MAGIC;
info->port = sstate->port;
+ info->hub6 = sstate->hub6;
info->flags = sstate->flags;
- info->io_type = sstate->io_type;
- info->iomem_base = sstate->iomem_base;
- info->iomem_reg_shift = sstate->iomem_reg_shift;
info->xmit_fifo_size = sstate->xmit_fifo_size;
+ info->state = sstate;
info->line = line;
+ info->iomem_base = sstate->iomem_base;
+ info->iomem_reg_shift = sstate->iomem_reg_shift;
+ info->io_type = sstate->io_type;
info->tqueue.routine = do_softint;
info->tqueue.data = info;
- info->state = sstate;
+
if (sstate->info) {
kfree(info);
- *ret_info = sstate->info;
- return 0;
+ info = sstate->info;
+ } else {
+ sstate->info = info;
+ }
+
+out:
+ /*
+ * If this is the first open, copy over some timeouts.
+ */
+ if (sstate->count == 1) {
+ info->closing_wait = sstate->closing_wait;
}
- *ret_info = sstate->info = info;
+ *ret_info = info;
return 0;
}


--
Russell King ([email protected]) The developer of ARM Linux
http://www.arm.linux.org.uk/personal/aboutme.html

2001-12-31 12:14:10

by Heinz Diehl

[permalink] [raw]
Subject: Re: [PATCH] fix serial close hang

On Sun Dec 30 2001, Russell King wrote:

> Whoops. This should fix it.

Yes, it does. The patch runs perfectly under 2.4.17.

--
# Heinz Diehl, 68259 Mannheim, Germany