RE: [PATCH] tty: hold lock across tty buffer finding and bufferfilling

[Du, Alek]

Alan,

I agree with you for the case of multiple writers to the tty buffer.

But there is another case that cause kernel hang: driver is writing data to tty, while app shutdown the port or flush through IOCTL. The buf.tail is set to NULL, here is the log:


<1>[ 4713.451954] BUG: unable to handle kernel NULL pointer dereference at
00000004
<1>[ 4713.452424] IP: [<c14ff59c>] tty_insert_flip_string_fixed_flag+0x4c/0x90
<4>[ 4713.452832] *pde = 00000000 
<0>[ 4713.453017] Oops: 0000 [#1] PREEMPT SMP 
<0>[ 4713.453280] last sysfs file:
/sys/devices/pci0000:00/0000:00:03.5/gpio/gpio172/value
<4>[ 4713.453709] Modules linked in: ipv6 atomisp lm3554 mt9m114 mt9e013
videobuf_vmalloc videobuf_dma_contig videobuf_core wl12xx_sdio wl12xx mac80211
cfg80211 compat btwilink st_drv
<4>[ 4713.454768] 
<4>[ 4713.454875] Pid: 0, comm: swapper Not tainted 2.6.35.3-84779-g68baa8f #1
/
<4>[ 4713.455258] EIP: 0060:[<c14ff59c>] EFLAGS: 00010002 CPU: 0
<4>[ 4713.455573] EIP is at tty_insert_flip_string_fixed_flag+0x4c/0x90
<4>[ 4713.455914] EAX: e01a6800 EBX: 00000000 ECX: 00000001 EDX: 00010002
<4>[ 4713.456264] ESI: 00000001 EDI: 00000000 EBP: c1a8be50 ESP: c1a8be38
<4>[ 4713.456614]  DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068
<0>[ 4713.456921] Process swapper (pid: 0, ti=c1a8a000 task=c1aa02a0
task.ti=c1a8a000)
<0>[ 4713.457324] Stack:
<4>[ 4713.457447]  00000000 f5abf800 e01a6800 f6dec358 f6df877c e01a6800
c1a8be78 c1544b13
<4>[ 4713.457958] <0> 00000001 000e0100 f79a785c 00000000 00000001 f6dec358
000000dc 00000001
<4>[ 4713.458511] <0> c1a8be98 c1544d0c 00000002 00000082 f6de0000 f7880f40
c1a908fc 00000000
<0>[ 4713.459087] Call Trace:
<4>[ 4713.459255]  [<c1544b13>] ? hsu_dma_rx+0xa3/0x190
<4>[ 4713.459534]  [<c1544d0c>] ? dma_irq+0x10c/0x160
<4>[ 4713.459806]  [<c12981f4>] ? handle_IRQ_event+0x44/0x1a0
<4>[ 4713.460109]  [<c129a798>] ? handle_fasteoi_irq+0x68/0xd0
<4>[ 4713.460418]  [<c120569d>] ? handle_irq+0x1d/0x30
<4>[ 4713.460695]  [<c18744cf>] ? do_IRQ+0x4f/0xc0
<4>[ 4713.460953]  [<c120356e>] ? common_interrupt+0x2e/0x34
<4>[ 4713.461257]  [<c124007b>] ? pre_schedule_rt+0x1b/0x30
<4>[ 4713.461557]  [<c14ede14>] ? soc_s0ix_idle+0x144/0x290
<4>[ 4713.461859]  [<c16544a0>] ? cpuidle_idle_call+0x90/0x180
<4>[ 4713.462168]  [<c1201d61>] ? cpu_idle+0x51/0xb0
<4>[ 4713.462431]  [<c1853e6b>] ? rest_init+0xab/0xc0
<4>[ 4713.462702]  [<c1b07991>] ? start_kernel+0x342/0x348
<4>[ 4713.462992]  [<c1b07460>] ? unknown_bootoption+0x0/0x1a0
<4>[ 4713.463301]  [<c1b070de>] ? i386_start_kernel+0xde/0xe6
<0>[ 4713.463593] Code: e8 8b 55 08 b8 00 07 00 00 29 fa 81 fa 00 07 00 00 0f
47
d0 8b 45 f0 e8 33 fd ff ff 89 c6 8b 45 f0 85 f6 8b 98 d4 00 00 00 74 2a <8b> 43
04 89 f1 01 f7 8b 55 ec 03 43 0c e8 72 99 fb ff 8b 43 08 
<0>[ 4713.465460] EIP: [<c14ff59c>] tty_insert_flip_string_fixed_flag+0x4c/0x90
SS:ESP 0068:c1a8be3





-----Original Message-----
From: Alan Cox [mailto:alan@xxxxxxxxxxxxxxxxxxx] 
Sent: Thursday, March 15, 2012 6:08 PM
To: Tu, Xiaobing
Cc: linux-kernel@xxxxxxxxxxxxxxx; gregkh@xxxxxxxxxxxxxxxxxxx; Zhang, Yanmin; Du, Alek; Zuo, Jiao
Subject: Re: [PATCH] tty: hold lock across tty buffer finding and buffer filling

On Thu, 15 Mar 2012 02:06:43 +0000
"Tu, Xiaobing" <xiaobing.tu@xxxxxxxxx> wrote:

> From: Xiaobing Tu <xiaobing.tu@xxxxxxxxx>
> 
> tty_buffer_request_room is well protected, but while after it returns,  
> it releases the port->lock. tty->buf.tail might be modified

This can only occur in a way that matters if you have multiple writers to the tty buffer. 

It is a design requirement of the tty layer that your input is single threaded paths and this is true for existing drivers. There is a good reason it is true as well - ordering of bytes is defined for serial data, any parallel writer breaks that ordering even if you have locks in tty_insert_flip_*.

In practice that means that the tail pointer has a single incrementer and the reader is the ldisc processing queue. 

So NAK pending a lot more explanation and an actual case where you can show it is required.

Under what situations with what in tree driver do you see a problem ?


Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/