- 18 Mar, 2013 35 commits
-
-
Peter Hurley authored
A buffer flush is both undesirable and unnecessary when the ldisc is closing. A buffer flush performs the following: 1. resets ldisc data fields to their initial state 2. resets tty->receive_room to indicate more data can be sent 3. schedules buffer work to receive more data 4. signals a buffer flush has happened to linked pty in packet mode Since the ldisc has been halted and the tty may soon be destructed, buffer work must not be scheduled as that work might access an invalid tty and ldisc state. Also, the ldisc read buffer is about to be freed, so that's pointless. Resetting the ldisc data fields is pointless as well since that structure is about to be freed. Resetting tty->receive_room is unnecessary, as it will be properly reset if a new ldisc is reopened. Besides, resetting the original receive_room value would be wrong since the read buffer will be gone. Since the packet mode flush is observable from userspace, this behavior has been preserved. The test jig originally authored by Ilya Zykov <ilya@ilyx.ru> and signed off by him is included below. The test jig prompts the following warnings which this patch fixes. [ 38.051111] ------------[ cut here ]------------ [ 38.052113] WARNING: at drivers/tty/n_tty.c:160 n_tty_set_room.part.6+0x8b/0xa0() [ 38.053916] Hardware name: Bochs [ 38.054819] Modules linked in: netconsole configfs bnep rfcomm bluetooth parport_pc ppdev snd_hda_intel snd_hda_codec snd_hwdep snd_pcm snd_seq_midi snd_rawmidi snd_seq_midi_event snd_seq psmouse snd_timer serio_raw mac_hid snd_seq_device snd microcode lp parport virtio_balloon soundcore i2c_piix4 snd_page_alloc floppy 8139too 8139cp [ 38.059704] Pid: 1564, comm: pty_kill Tainted: G W 3.7.0-next-20121130+ttydebug-xeon #20121130+ttydebug [ 38.061578] Call Trace: [ 38.062491] [<ffffffff81058b4f>] warn_slowpath_common+0x7f/0xc0 [ 38.063448] [<ffffffff81058baa>] warn_slowpath_null+0x1a/0x20 [ 38.064439] [<ffffffff8142dc2b>] n_tty_set_room.part.6+0x8b/0xa0 [ 38.065381] [<ffffffff8142dc82>] n_tty_set_room+0x42/0x80 [ 38.066323] [<ffffffff8142e6f2>] reset_buffer_flags+0x102/0x160 [ 38.077508] [<ffffffff8142e76d>] n_tty_flush_buffer+0x1d/0x90 [ 38.078782] [<ffffffff81046569>] ? default_spin_lock_flags+0x9/0x10 [ 38.079734] [<ffffffff8142e804>] n_tty_close+0x24/0x60 [ 38.080730] [<ffffffff81431b61>] tty_ldisc_close.isra.2+0x41/0x60 [ 38.081680] [<ffffffff81431bbb>] tty_ldisc_kill+0x3b/0x80 [ 38.082618] [<ffffffff81432a07>] tty_ldisc_release+0x77/0xe0 [ 38.083549] [<ffffffff8142b781>] tty_release+0x451/0x4d0 [ 38.084525] [<ffffffff811950be>] __fput+0xae/0x230 [ 38.085472] [<ffffffff8119524e>] ____fput+0xe/0x10 [ 38.086401] [<ffffffff8107aa88>] task_work_run+0xc8/0xf0 [ 38.087334] [<ffffffff8105ea56>] do_exit+0x196/0x4b0 [ 38.088304] [<ffffffff8106c77b>] ? __dequeue_signal+0x6b/0xb0 [ 38.089240] [<ffffffff8105ef34>] do_group_exit+0x44/0xa0 [ 38.090182] [<ffffffff8106f43d>] get_signal_to_deliver+0x20d/0x4e0 [ 38.091125] [<ffffffff81016979>] do_signal+0x29/0x130 [ 38.092096] [<ffffffff81431a9e>] ? tty_ldisc_deref+0xe/0x10 [ 38.093030] [<ffffffff8142a317>] ? tty_write+0xb7/0xf0 [ 38.093976] [<ffffffff81193f53>] ? vfs_write+0xb3/0x180 [ 38.094904] [<ffffffff81016b20>] do_notify_resume+0x80/0xc0 [ 38.095830] [<ffffffff81700492>] int_signal+0x12/0x17 [ 38.096788] ---[ end trace 5f6f7a9651cd999b ]--- [ 2730.570602] ------------[ cut here ]------------ [ 2730.572130] WARNING: at drivers/tty/n_tty.c:160 n_tty_set_room+0x107/0x140() [ 2730.574904] scheduling buffer work for halted ldisc [ 2730.578303] Pid: 9691, comm: trinity-child15 Tainted: G W 3.7.0-rc8-next-20121205-sasha-00023-g59f0d85 #207 [ 2730.588694] Call Trace: [ 2730.590486] [<ffffffff81c41d77>] ? n_tty_set_room+0x107/0x140 [ 2730.592559] [<ffffffff8110c827>] warn_slowpath_common+0x87/0xb0 [ 2730.595317] [<ffffffff8110c8b1>] warn_slowpath_fmt+0x41/0x50 [ 2730.599186] [<ffffffff81c41d77>] n_tty_set_room+0x107/0x140 [ 2730.603141] [<ffffffff81c42fe7>] reset_buffer_flags+0x137/0x150 [ 2730.607166] [<ffffffff81c43018>] n_tty_flush_buffer+0x18/0x90 [ 2730.610123] [<ffffffff81c430af>] n_tty_close+0x1f/0x60 [ 2730.612068] [<ffffffff81c461f2>] tty_ldisc_close.isra.4+0x52/0x60 [ 2730.614078] [<ffffffff81c462ab>] tty_ldisc_reinit+0x3b/0x70 [ 2730.615891] [<ffffffff81c46db2>] tty_ldisc_hangup+0x102/0x1e0 [ 2730.617780] [<ffffffff81c3e537>] __tty_hangup+0x137/0x440 [ 2730.619547] [<ffffffff81c3e869>] tty_vhangup+0x9/0x10 [ 2730.621266] [<ffffffff81c48f1c>] pty_close+0x14c/0x160 [ 2730.622952] [<ffffffff81c3fd45>] tty_release+0xd5/0x490 [ 2730.624674] [<ffffffff8127fbe2>] __fput+0x122/0x250 [ 2730.626195] [<ffffffff8127fd19>] ____fput+0x9/0x10 [ 2730.627758] [<ffffffff81134602>] task_work_run+0xb2/0xf0 [ 2730.629491] [<ffffffff811139ad>] do_exit+0x36d/0x580 [ 2730.631159] [<ffffffff81113c8a>] do_group_exit+0x8a/0xc0 [ 2730.632819] [<ffffffff81127351>] get_signal_to_deliver+0x501/0x5b0 [ 2730.634758] [<ffffffff8106de34>] do_signal+0x24/0x100 [ 2730.636412] [<ffffffff81204865>] ? user_exit+0xa5/0xd0 [ 2730.638078] [<ffffffff81183cd8>] ? trace_hardirqs_on_caller+0x118/0x140 [ 2730.640279] [<ffffffff81183d0d>] ? trace_hardirqs_on+0xd/0x10 [ 2730.642164] [<ffffffff8106df78>] do_notify_resume+0x48/0xa0 [ 2730.643966] [<ffffffff83cdff6a>] int_signal+0x12/0x17 [ 2730.645672] ---[ end trace a40d53149c07fce0 ]--- /* * pty_thrash.c * * Based on original test jig by Ilya Zykov <ilya@ilyx.ru> * * Signed-off-by: Peter Hurley <peter@hurleysoftware.com> * Signed-off-by: Ilya Zykov <ilya@ilyx.ru> */ static int fd; static void error_exit(char *f, ...) { va_list va; va_start(va, f); vprintf(f, va); printf(": %s\n", strerror(errno)); va_end(va); if (fd >= 0) close(fd); exit(EXIT_FAILURE); } int main(int argc, char *argv[]) { int parent; char pts_name[24]; int ptn, unlock; while (1) { fd = open("/dev/ptmx", O_RDWR); if (fd < 0) error_exit("opening pty master"); unlock = 0; if (ioctl(fd, TIOCSPTLCK, &unlock) < 0) error_exit("unlocking pty pair"); if (ioctl(fd, TIOCGPTN, &ptn) < 0) error_exit("getting pty #"); snprintf(pts_name, sizeof(pts_name), "/dev/pts/%d", ptn); child_id = fork(); if (child_id == -1) error_exit("forking child"); if (parent) { int err, id, status; char buf[128]; int n; n = read(fd, buf, sizeof(buf)); if (n < 0) error_exit("master reading"); printf("%.*s\n", n-1, buf); close(fd); err = kill(child_id, SIGKILL); if (err < 0) error_exit("killing child"); id = waitpid(child_id, &status, 0); if (id < 0 || id != child_id) error_exit("waiting for child"); } else { /* Child */ close(fd); printf("Test cycle on slave pty %s\n", pts_name); fd = open(pts_name, O_RDWR); if (fd < 0) error_exit("opening pty slave"); while (1) { char pattern[] = "test\n"; if (write(fd, pattern, strlen(pattern)) < 0) error_exit("slave writing"); } } } /* never gets here */ return 0; } Reported-by:Sasha Levin <levinsasha928@gmail.com> Signed-off-by:
Peter Hurley <peter@hurleysoftware.com> Signed-off-by:
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
Peter Hurley authored
Factor the packet mode status change from n_tty_flush_buffer for use by follow-on patch. Signed-off-by:
-
Peter Hurley authored
Flip buffer work must not be scheduled by the line discipline after the line discipline has been halted; issue warning. Note: drivers can still schedule flip buffer work. Signed-off-by:
-
Lars-Peter Clausen authored
The ifx6x60 driver implements both legacy suspend/resume callbacks and dev_pm_ops. The SPI core is going to ignore legacy suspend/resume callbacks if a driver implements dev_pm_ops. Since the legacy suspend/resume callbacks are empty in this case it is safe to just remove them. Cc: Bi Chao <chao.bi@intel.com> Cc: Chen Jun <jun.d.chen@intel.com> Signed-off-by:
Lars-Peter Clausen <lars@metafoo.de> Signed-off-by:
-
Lars-Peter Clausen authored
Use dev_pm_ops instead of the deprecated legacy suspend/resume for the mrst_max3110 driver. Cc: Alan Cox <alan@linux.intel.com> Signed-off-by:
-
Lars-Peter Clausen authored
Use dev_pm_ops instead of the deprecated legacy suspend/resume for the max310x driver. Cc: Alexander Shiyan <shc_work@mail.ru> Signed-off-by:
-
Lars-Peter Clausen authored
Use dev_pm_ops instead of the deprecated legacy suspend/resume for the max3100 driver. Signed-off-by:
-
Johan Hovold authored
Make sure we do not make tty-driver callbacks or wait for port to drain on uninitialised ports (e.g. when open failed) in tty_port_close_start(). No callback, such as flush_buffer or wait_until_sent, needs to be made on a port that has never been opened. Neither does it make much sense to add drain delay for an uninitialised port. Currently a drain delay of up to two seconds could be added when a tty fails to open. Signed-off-by:
Johan Hovold <jhovold@gmail.com> Signed-off-by:
-
Johan Hovold authored
Move port drain-delay handling to a separate function. Signed-off-by:
-
Johan Hovold authored
Move HUPCL handling to port shutdown so that DTR is dropped also on hang up (tty_port_close is a noop for hung-up ports). Also do not try to drop DTR for uninitialised ports where it has never been raised (e.g. after a failed open). Note that this is also the current behaviour of serial-core. Nine drivers currently call tty_port_close_start directly (rather than through tty_port_close) and seven of them lower DTR as part of their close (if the port has been initialised). Fixup the remaining two drivers so that it continues to be lowered also on normal (non-HUP) close. [ Note that most of those other seven drivers did not expect DTR to have been dropped by tty_port_close_start in the first place. ] Signed-off-by:
-
Johan Hovold authored
Make sure to check ASYNC_INITIALISED before raising DTR when waking up from blocked open in tty_port_block_til_ready. Currently DTR could get raised at hang up as a blocked process would raise DTR unconditionally before checking for hang up and returning. Signed-off-by:
-
Johan Hovold authored
Move wake up of processes on blocked-open and modem-status wait queues to after port shutdown at hangup. This way the woken up processes can use the ASYNC_INITIALIZED flag to detect port shutdown. Note that this is the order currently used by serial-core. Signed-off-by:
-
Johan Hovold authored
Untangle port-shutdown logic and make sure the initialised flag is always cleared for non-console ports. Signed-off-by:
-
Jiri Slaby authored
The same as flags, convert to using open/close counts from tty_port. Signed-off-by:
Jiri Slaby <jslaby@suse.cz> Signed-off-by:
-
Jiri Slaby authored
The same as flags, convert to using *_wait queues from tty_port. Signed-off-by:
-
Jiri Slaby authored
The same as flags, convert to using close delays from tty_port. Signed-off-by:
-
Jiri Slaby authored
Well, all those are unused. They were perhaps copied from generic serial structure ages ago. Remove them for good. Signed-off-by:
-
Jiri Slaby authored
The return value is not used anywhere, so no need to return anything. Signed-off-by:
-
Jiri Slaby authored
First, remove STD_FLAGS as the value, or its subvalues (ASYNC_BOOT_AUTOCONF | ASYNC_SKIP_TEST) is not tested anywhere -- there is no point to initialize flags to that. Second, use flags member from tty_port when we have it now. So that we do not waste space. Signed-off-by:
-
Jiri Slaby authored
tty->hw_stopped is set only by drivers to remember HW state. If it is never set to 1 in a particular driver, there is no need to check it in the driver at all. Remove such checks. Signed-off-by:
-
Jiri Slaby authored
Signed-off-by:
-
Jiri Slaby authored
There are no (and never were any) kgdb fields in uart_ops. Setting them produces a build error: drivers/tty/serial/bfin_uart.c:1054:2: error: unknown field 'kgdboc_port_startup' specified in initializer drivers/tty/serial/bfin_uart.c:1054:2: warning: initialization from incompatible pointer type [enabled by default] drivers/tty/serial/bfin_uart.c:1054:2: warning: (near initialization for 'bfin_serial_pops.ioctl') [enabled by default] drivers/tty/serial/bfin_uart.c:1055:2: error: unknown field 'kgdboc_port_shutdown' specified in initializer drivers/tty/serial/bfin_uart.c:1055:2: warning: initialization from incompatible pointer type [enabled by default] drivers/tty/serial/bfin_uart.c:1055:2: warning: (near initialization for 'bfin_serial_pops.poll_init') [enabled by default] Remove them. Signed-off-by:
Sonic Zhang <sonic.zhang@analog.com> Signed-off-by:
-
Jiri Slaby authored
It allows for cleaning up on a considerable amount of places. They did port_get, hangup, kref_put. Now the only thing needed is to call tty_port_tty_hangup which does exactly that. And they can also decide whether to consider CLOCAL or completely ignore that. Signed-off-by:
-
Jiri Slaby authored
tty->ops->break_ctl cannot be called outside the gap between open and close. So there is no need to check whether the port is open in break_ctl in quatech2. Remove the check and also that member completely. Signed-off-by:
-
Jiri Slaby authored
It allows for cleaning up on a considerable amount of places. They did port_get, wakeup, kref_put. Now the only thing needed is to call tty_port_tty_wakeup which does exactly that. One exception is ifx6x60 where tty_wakeup was open-coded. We now call tty_wakeup properly there. Signed-off-by:
-
Jiri Slaby authored
Do not dig struct smd_tty_info out of tty_struct using tty_port_tty_get. It is unnecessarily too complicated, use simple container_of instead. Signed-off-by:
-
Jiri Slaby authored
The warning is there since 2.1.69 and we have not seen anybody reporting it in the past decade. Remove the warning now. tty_get_baud_rate can now be inline. This gives us one less EXPORT_SYMBOL. Signed-off-by:
-
Jiri Slaby authored
info is obtained by container_of. It can never be NULL. So do not test that. Signed-off-by:
-
Jiri Slaby authored
data_len in jsm_input cannot be zero as we would jump out early in the function. It also cannot be negative because it is an int and we do bitwise and with 8192. So remove the check. Signed-off-by:
-
Peter Hurley authored
Signed-off-by:
-
Peter Hurley authored
Concurrent access to tty->pgrp must be protected with tty->ctrl_lock. Also, as noted in the comments, reading current->signal->tty is safe because either, 1) current->signal->tty is assigned by current, or 2) current->signal->tty is set to NULL. NB: for reference, tty_check_change() implements a similar POSIX check for the ioctls corresponding to tcflush(), tcdrain(), tcsetattr(), tcsetpgrp(), tcflow() and tcsendbreak(). Signed-off-by:
-
Peter Hurley authored
An ldisc reference is insufficient guarantee the foreground process group is not in the process of being signalled from a hangup. 1) Reads of tty->pgrp must be locked with ctrl_lock 2) The group pid must be referenced for the duration of signalling. Because the driver-side is not process-context, a pid reference must be acquired. Signed-off-by:
-
Peter Hurley authored
As noted in the following comment: /* FIXME: there is a tiny race here if the receive room check runs before the other work executes and empties the buffer (upping the receiving room and unthrottling. We then throttle and get stuck. This has been observed and traced down by Vincent Pillet/ We need to address this when we sort out out the rx path locking */ Use new safe throttle/unthrottle functions to re-evaluate conditions if interrupted by the complement flow control function. Reported-by:Vincent Pillet <vincentx.pillet@intel.com> Signed-off-by:
-
Peter Hurley authored
The tty driver can become stuck throttled due to race conditions between throttle and unthrottle, when the decision to throttle or unthrottle is conditional. The following example helps to illustrate the race: CPU 0 | CPU 1 | if (condition A) | | <processing such that A not true> | if (!condition A) | unthrottle() throttle() | | Note the converse is also possible; ie., CPU 0 | CPU 1 | | if (!condition A) <processing such that A true> | if (condition A) | throttle() | | unthrottle() | Add new throttle/unthrottle functions based on the familiar model of task state and schedule/wake. For example, while (1) { tty_set_flow_change(tty, TTY_THROTTLE_SAFE); if (!condition) break; if (!tty_throttle_safe(tty)) break; } __tty_set_flow_change(tty, 0); In this example, if an unthrottle occurs after the condition is evaluated but before tty_throttle_safe(), then tty_throttle_safe() will return non-zero, looping and forcing the re-evaluation of condition. Reported-by: -
Peter Hurley authored
2-line function check_unthrottle() is now only called from n_tty_read(); merge into caller. Signed-off-by:
-
- 15 Mar, 2013 5 commits
-
-
zhangwei(Jovi) authored
Currently help message of /proc/sysrq-trigger highlight its upper-case characters, like below: SysRq : HELP : loglevel(0-9) reBoot Crash terminate-all-tasks(E) memory-full-oom-kill(F) kill-all-tasks(I) ... this would confuse user trigger sysrq by upper-case character, which is inconsistent with the real lower-case character registed key. This inconsistent help message will also lead more confused when 26 upper-case letters put into use in future. This patch fix it. Thanks the comments from Andrew and Randy. Signed-off-by:zhangwei(Jovi) <jovi.zhangwei@huawei.com> Cc: Andrew Morton <akpm@linux-foundation.org> Acked-by:
Randy Dunlap <rdunlap@infradead.org> Signed-off-by:
-
Peter Hurley authored
An exiting session leader can hang if a foreground process is blocking for line discipline i/o, eg. in n_tty_read(). This happens because the blocking reader is holding an ldisc reference (indicating the line discipline is in-use) which prevents __tty_hangup() from recycling the line discipline. Although waiters are woken before attempting to gain exclusive access for changing the ldisc, the blocking reader in this case will not exit the i/o loop since it has not yet received SIGHUP (because it has not been sent). Instead, perform signalling first, then recycle the line discipline. Fixes: INFO: task init:1 blocked for more than 120 seconds. "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. init D 00000000001d7180 2688 1 0 0x00000002 ffff8800b9acfba8 0000000000000002 00000000001d7180 ffff8800b9b10048 ffff8800b94cb000 ffff8800b9b10000 00000000001d7180 00000000001d7180 ffff8800b9b10000 ffff8800b9acffd8 00000000001d7180 00000000001d7180 Call Trace: [<ffffffff83db9909>] __schedule+0x2e9/0x3b0 [<ffffffff83db9b35>] schedule+0x55/0x60 [<ffffffff83db74ba>] schedule_timeout+0x3a/0x370 [<ffffffff81182349>] ? mark_held_locks+0xf9/0x130 [<ffffffff83dbab38>] ? down_failed+0x108/0x200 [<ffffffff83dbb7ab>] ? _raw_spin_unlock_irq+0x2b/0x80 [<ffffffff81182608>] ? trace_hardirqs_on_caller+0x128/0x160 [<ffffffff83dbab61>] down_failed+0x131/0x200 [<ffffffff83dbbfad>] ? tty_ldisc_lock_pair_timeout+0xcd/0x120 [<ffffffff83dbae03>] ldsem_down_write+0xd3/0x113 [<ffffffff83dbbfad>] ? tty_ldisc_lock_pair_timeout+0xcd/0x120 [<ffffffff8118264d>] ? trace_hardirqs_on+0xd/0x10 [<ffffffff83dbbfad>] tty_ldisc_lock_pair_timeout+0xcd/0x120 [<ffffffff81c3df60>] tty_ldisc_hangup+0xd0/0x220 [<ffffffff81c35bd7>] __tty_hangup+0x137/0x4f0 [<ffffffff81c37c7c>] disassociate_ctty+0x6c/0x230 [<ffffffff8111290c>] do_exit+0x41c/0x590 [<ffffffff8107ad34>] ? syscall_trace_enter+0x24/0x2e0 [<ffffffff81112b4a>] do_group_exit+0x8a/0xc0 [<ffffffff81112b92>] sys_exit_group+0x12/0x20 [<ffffffff83dc49d8>] tracesys+0xe1/0xe6 1 lock held by init/1: #0: (&tty->ldisc_sem){++++++}, at: [<ffffffff83dbbfad>] tty_ldisc_lock_pair_timeout+0xcd/0x120 Reported-by: -
Peter Hurley authored
When the session leader is exiting, signal the foreground group processes as part of the hangup sequence, instead of after the hangup is complete. This prepares for hanging up the line discipline _after_ signalling processes which may be blocking on ldisc i/o. Parameterize __tty_hangup() to distinguish between when the session leader is exiting and all other hangups; signal the foreground group after signalling the session leader and its process group, which preserves the original signal order. Signed-off-by:
-
Peter Hurley authored
The interrupt state does not need to be saved, disabled and restored here; interrupts are already off because this lock is bracketed by spin_lock_irq/spin_unlock_irq. Signed-off-by:
-
Peter Hurley authored
__tty_hangup() and tty_vhangup() cannot be called from atomic context, so locks do not need to preserve the interrupt state (although, still disable interrupts). Signed-off-by:
-
