Skip to contentSkip to conversation

fix serial close hang

PATCH In review Russell King 2 participants 4 messages Dec 30, 2001 – Dec 31, 2001 Char and misc drivers

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 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.

include/linux/serialP.h +1 -1
@@ -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 */
drivers/char/serial.c +27 -11
@@ -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;
 }
 
Signature · 2 lines
Russell King ([email protected])                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html
#1 lore raw
Quoted 1 line · Russell King

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 #

Signature · 1 line
# Heinz Diehl, 68259 Mannheim, Germany
#2 lore raw
In reply to Russell King
Quoted 1 line · Heinz Diehl

serial.c:3131: state' undeclared (first use in this function)

Whoops. This should fix it.

include/linux/serialP.h +1 -1
@@ -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 */
drivers/char/serial.c +27 -11
@@ -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;
 }
 
Signature · 2 lines
Russell King ([email protected])                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html
#3 lore raw
In reply to Russell King
Quoted 1 line · Russell King

Whoops. This should fix it.

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

Signature · 1 line
# Heinz Diehl, 68259 Mannheim, Germany