- 28 Mar, 2013 4 commits
-
-
Michael Spang authored
The no_console_suspend option allows the console to write to the UART during early resume, before the serial port itself is resumed. Such writes hang indefinitely waiting for TX ready. This adds a check to the console putchar function to drop characters instead of hanging if the port hasn't been initialized. That way, we can use no_console_suspend without risk of hanging. Signed-off-by: Michael Spang <spang@chromium.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
Michael Spang authored
This closes a window where the system may hang in resume as soon as the UART interrupt is enabled and before the mask is restored. The hang occurs because the driver can't handle IRQs it thinks are masked. Signed-off-by: Michael Spang <spang@chromium.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
Tomasz Figa authored
Since the interrupt mask register is not preserved across system suspend and it defaults to all interrupts enabled, it is not enough to disable UART interrupt. This patch adds free_irq to port shutdown and mask setting to port startup to handle IRQ disabling in a suspend-friendly way. In addition, a bug caused by multiple request_irq calls in port startup callback is fixed. Signed-off-by: Tomasz Figa <t.figa@samsung.com> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
Karthik Manamcheri authored
Consider a situation where I have an ARM based system and therefore no legacy ports. Say, I have two memory-mapped ports. I use device tree to describe the ports. What would be the config options I set so that I get only the two ports in my system? I do not want legacy ports being created automatically and I want it to be flexible enough that it creates the devices based only on the device tree. I expected setting SERIAL_8250_RUNTIME_UARTS = 0 to work because the description said, "Set this to the maximum number of serial ports you want the kernel to register at boot time." Unfortunately, even though SERIAL_8250_NR_UARTS was set to the default value of 4, I did not get any device nodes (because SERIAL_8250_RUNTIME_UARTS was 0). This is what this change is addressing. SERIAL_8250_NR_UARTS controls the maximum number of ports you can support. SERIAL_8250_RUNTIME_UARTS specifies the number of ports you want to create automatically for legacy ports at boot time. All other ports will be created when serial8250_register_port is called (and if does not exceed the total number of supported ports as specified by SERIAL_8250_NR_UARTS). Signed-off-by: Karthik Manamcheri <karthik.manamcheri@ni.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 25 Mar, 2013 8 commits
-
-
Paul Bolle authored
Support for the WindRiver SBC8560 board was removed in v3.6. But there are still a few lines depending on its obsolete Kconfig macro. Remove these now. Signed-off-by: Paul Bolle <pebolle@tiscali.nl> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
Heikki Krogerus authored
Automatic Flow Control capability is not tied to this property. This is only one way of detecting it. The property is limited to be used only with 8250 driver. Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> Acked-by: Arnd Bergmann <arnd@arndb.de> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
Changlong Xie authored
Reported-by: Fengguang Wu <fengguang.wu@intel.com> Acked-by: Peter Hurley <peter@hurleysoftware.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
Jiri Slaby authored
After commit 6865ff22 ("TTY: do not warn about setting speed via SPD_*"), we are seeing a build error when DEBUG_FS is enabled: drivers/net/caif/caif_serial.c: In function 'update_tty_status': drivers/net/caif/caif_serial.c:94:11: error: 'struct tty_struct' has no member named 'warned' Fix the error by removing the reference to the variable as it was not set in the last decade (as documented in the changelog of the commit above). Signed-off-by: Jiri Slaby <jslaby@suse.cz> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> Cc: Sjur Braendeland <sjur.brandeland@stericsson.com> Cc: netdev@vger.kernel.org Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
Steffen Trumtrar authored
The datesheet clearly states, that writing low bits to the XUARTPS_IDR register have no effect. Remove the write. Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
Paul Bolle authored
Support for SH7367 and SH7377 got removed in v3.8. Now remove their last Kconfig macros. Signed-off-by: Paul Bolle <pebolle@tiscali.nl> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
Heikki Krogerus authored
This will reduce the need for extra types in 8250.c just in case the fifo size differs from the standard. Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> Acked-by: Arnd Bergmann <arnd@arndb.de> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
Heikki Krogerus authored
In most cases the tx_loadsz is the same as fifosize. This will store the fifosize in it if it was not separately delivered from the driver. Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 21 Mar, 2013 2 commits
-
-
Greg Kroah-Hartman authored
This reverts commit 27b351c5. It caused problems with the build. Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> Cc: Bill Pemberton <wfp5p@viridian.itc.virginia.edu> Cc: Jiri Slaby <jslaby@suse.cz> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
Greg Kroah-Hartman authored
-
- 19 Mar, 2013 3 commits
-
-
Zhang Yanfei authored
remove cast for kmalloc return value. Signed-off-by: Zhang Yanfei <zhangyanfei@cn.fujitsu.com> Cc: Jiri Slaby <jslaby@suse.cz> Cc: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
Zhang Yanfei authored
remove cast for kzalloc return value. Signed-off-by: Zhang Yanfei <zhangyanfei@cn.fujitsu.com> Cc: Jiri Slaby <jslaby@suse.cz> Cc: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
Jiri Slaby authored
The following commits: * 6732c8bb (TTY: switch tty_schedule_flip) * 2e124b4a (TTY: switch tty_flip_buffer_push) * 05c7cd39 (TTY: switch tty_insert_flip_string) * 92a19f9c (TTY: switch tty_insert_flip_char) * 227434f8 (TTY: switch tty_buffer_request_room to tty_port) introduced a potential NULL dereference to some drivers. In particular, when the device is used as a console, incoming bytes can kill the box. This is caused by removed checks for TTY against NULL. It happened because it was unclear to me why the checks were there. I assumed them superfluous because the interrupts were unbound or otherwise stopped. But this is not the case for consoles for these drivers, as was pointed out by David Miller. Now, this patch re-introduces the checks (at this point we check port->state, not the tty proper, as we do not care about tty pointers anymore). For both of the drivers, we place the check below the handling of break signal so that sysrq can actually work. (One needs to issue a break and then sysrq key within the following 5 seconds.) We do not change sc26xx, sunhv, and sunsu here because they behave the same as before. People having that hardware should fix the driver eventually, however. They always could unconditionally dereference tty in receive_chars, port->state in uart_handle_dcd_change, and up->port.state->port.tty. There is perhaps more to fix in all those drivers, but they are at least in a state they were before. Signed-off-by: Jiri Slaby <jslaby@suse.cz> Cc: "David S. Miller" <davem@davemloft.net> Cc: Grant Likely <grant.likely@secretlab.ca> Cc: Rob Herring <rob.herring@calxeda.com> Cc: sparclinux@vger.kernel.org Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 18 Mar, 2013 23 commits
-
-
Peter Hurley authored
tty_perform_flush() can deadlock when called while holding a line discipline reference. By definition, all ldisc drivers hold a ldisc reference, so calls originating from ldisc drivers must not block for a ldisc reference. The deadlock can occur when: CPU 0 | CPU 1 | tty_ldisc_ref(tty) | .... | <line discipline halted> tty_ldisc_ref_wait(tty) | | CPU 0 cannot progess because it cannot obtain an ldisc reference with the line discipline has been halted (thus no new references are granted). CPU 1 cannot progress because an outstanding ldisc reference has not been released. An in-tree call-tree audit of tty_perform_flush() [1] shows 5 ldisc drivers calling tty_perform_flush() indirectly via n_tty_ioctl_helper() and 2 ldisc drivers calling directly. A single tty driver safely uses the function. [1] Recursive usage: /* These functions are line discipline ioctls and thus * recursive wrt line discipline references */ tty_perform_flush() - ./drivers/tty/tty_ioctl.c n_tty_ioctl_helper() hci_uart_tty_ioctl(default) - drivers/bluetooth/hci_ldisc.c (N_HCI) n_hdlc_tty_ioctl(default) - drivers/tty/n_hdlc.c (N_HDLC) gsmld_ioctl(default) - drivers/tty/n_gsm.c (N_GSM0710) n_tty_ioctl(default) - drivers/tty/n_tty.c (N_TTY) gigaset_tty_ioctl(default) - drivers/isdn/gigaset/ser-gigaset.c (N_GIGASET_M101) ppp_synctty_ioctl(TCFLSH) - drivers/net/ppp/pps_synctty.c ppp_asynctty_ioctl(TCFLSH) - drivers/net/ppp/ppp_async.c Non-recursive use: tty_perform_flush() - drivers/tty/tty_ioctl.c ipw_ioctl(TCFLSH) - drivers/tty/ipwireless/tty.c /* This function is a tty i/o ioctl method, which * is invoked by tty_ioctl() */ Signed-off-by: Peter Hurley <peter@hurleysoftware.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
Peter Hurley authored
tty_ioctl() already waits until sent. Signed-off-by: Peter Hurley <peter@hurleysoftware.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
Peter Hurley authored
Signed-off-by: Peter Hurley <peter@hurleysoftware.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
Peter Hurley authored
Now that tty_ldisc_assign() is a one-line file-scoped function, remove it and perform the simple assignment at its call sites. Signed-off-by: Peter Hurley <peter@hurleysoftware.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
Peter Hurley authored
Merge get_ldisc() into its only call site. Note how, after merging, the unsafe acquire of an ldisc reference is obvious. CPU 0 in tty_ldisc_try() | CPU 1 in tty_ldisc_halt() | test_bit(TTY_LDISC, &tty_flags) | if (true) | clear_bit(TTY_LDISC, &tty_flags) tty->ldisc != 0? | atomic_read(&tty->ldisc->users) if (true) | ret_val == 1? atomic_inc(&tty->ldisc->users) | if (false) | wait | <goes on assuming safe ldisc use> | <doesn't wait - proceeds w/ close> | The spin lock in tty_ldisc_try() does nothing wrt synchronizing the ldisc halt since it's not acquired as part of halting. Signed-off-by: Peter Hurley <peter@hurleysoftware.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
Peter Hurley authored
tty_ldisc_ref()/tty_ldisc_unref() have usage semantics equivalent to down_read_trylock()/up_read(). Only callers of tty_ldisc_put() are performing the additional operations necessary for proper ldisc teardown, and then only after ensuring no outstanding 'read lock' remains. Thus, tty_ldisc_unref() should never be the last reference; WARN if it is. Conversely, tty_ldisc_put() should never be destructing if the use count != 1. Signed-off-by: Peter Hurley <peter@hurleysoftware.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
Peter Hurley authored
test_bit() is already atomic; drop mutex lock/unlock. Signed-off-by: Peter Hurley <peter@hurleysoftware.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
Peter Hurley authored
Expected typical debug log: [ 582.721965] tty_open: opening pts3... [ 582.721970] tty_open: opening pts3... [ 582.721977] tty_release: pts3 (tty count=3)... [ 582.721980] tty_release: ptm3 (tty count=1)... [ 582.722015] pts3 vhangup... [ 582.722020] tty_ldisc_hangup: pts3: closing ldisc: ffff88007a920540 [ 582.724128] tty_release: pts3 (tty count=2)... [ 582.724217] tty_ldisc_hangup: pts3: re-opened ldisc: ffff88007a920580 [ 582.724221] tty_release: ptm3: final close [ 582.724234] tty_ldisc_release: ptm3: closing ldisc: ffff88007a920a80 [ 582.724238] tty_ldisc_release: ptm3: ldisc closed [ 582.724241] tty_release: ptm3: freeing structure... [ 582.724741] tty_open: opening pts3... Signed-off-by: Peter Hurley <peter@hurleysoftware.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
Peter Hurley authored
Expected typical log output: [ 2.437211] tty_open: opening pts1... [ 2.443376] tty_open: opening pts5... [ 2.447830] tty_release: ptm0 (tty count=1)... [ 2.447849] pts0 vhangup... [ 2.447865] tty_release: ptm0: final close [ 2.447876] tty_release: ptm0: freeing structure... [ 2.451634] tty_release: tty1 (tty count=1)... [ 2.451638] tty_release: tty1: final close [ 2.451654] tty_release: tty1: freeing structure... [ 2.452505] tty_release: pts5 (tty count=2)... [ 2.453029] tty_open: opening pts0... Signed-off-by: Peter Hurley <peter@hurleysoftware.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
Peter Hurley authored
This comment is a victim of code migration from "tty: Fix the ldisc hangup race"; re-parent it. Signed-off-by: Peter Hurley <peter@hurleysoftware.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
Peter Hurley authored
The tty core relies on the ldisc layer for synchronizing destruction of the tty. Instead, the final tty release must wait for any pending tty work to complete prior to tty destruction. Signed-off-by: Peter Hurley <peter@hurleysoftware.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
Peter Hurley authored
Waiting for buffer work to complete is not required for safely performing changes to the line discipline, once the line discipline is halted. The buffer work routine, flush_to_ldisc(), will be unable to acquire an ldisc ref and all existing references were waited until released (so it can't already have one). Ensure running buffer work which may reference the soon-to-be-gone tty completes and any buffer work running after this point retrieves a NULL tty. Also, ensure all buffer work is cancelled on port destruction. Signed-off-by: Peter Hurley <peter@hurleysoftware.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
Peter Hurley authored
tty_ldisc_hangup() guarantees the ldisc is enabled (or that there is no ldisc). Since __tty_hangup() was the only user, re-define tty_ldisc_enable() in file-scope. Signed-off-by: Peter Hurley <peter@hurleysoftware.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
Peter Hurley authored
Buffer work may already be pending when the n_tty ldisc is re-opened, eg., when setting the ldisc (via TIOCSETD ioctl) and when hanging up the tty. Since n_tty_set_room() may restart buffer work, first ensure the ldisc is completely initialized. Factor n_tty_set_room() out of reset_buffer_flags() (only 2 callers) and reorganize n_tty_open() to set termios last; buffer work will be restarted there if necessary, after the char_map is properly initialized. Fixes this WARNING: [ 549.561769] ------------[ cut here ]------------ [ 549.598755] WARNING: at drivers/tty/n_tty.c:160 n_tty_set_room+0xff/0x130() [ 549.604058] scheduling buffer work for halted ldisc [ 549.607741] Pid: 9417, comm: trinity-child28 Tainted: G D W 3.7.0-next-20121217-sasha-00023-g8689ef9 #219 [ 549.652580] Call Trace: [ 549.662754] [<ffffffff81c432cf>] ? n_tty_set_room+0xff/0x130 [ 549.665458] [<ffffffff8110cae7>] warn_slowpath_common+0x87/0xb0 [ 549.668257] [<ffffffff8110cb71>] warn_slowpath_fmt+0x41/0x50 [ 549.671007] [<ffffffff81c432cf>] n_tty_set_room+0xff/0x130 [ 549.673268] [<ffffffff81c44597>] reset_buffer_flags+0x137/0x150 [ 549.675607] [<ffffffff81c45b71>] n_tty_open+0x131/0x1c0 [ 549.677699] [<ffffffff81c47824>] tty_ldisc_open.isra.5+0x54/0x70 [ 549.680147] [<ffffffff81c482bf>] tty_ldisc_hangup+0x11f/0x1e0 [ 549.682409] [<ffffffff81c3fa17>] __tty_hangup+0x137/0x440 [ 549.684634] [<ffffffff81c3fd49>] tty_vhangup+0x9/0x10 [ 549.686443] [<ffffffff81c4a42c>] pty_close+0x14c/0x160 [ 549.688446] [<ffffffff81c41225>] tty_release+0xd5/0x490 [ 549.690460] [<ffffffff8127d8a2>] __fput+0x122/0x250 [ 549.692577] [<ffffffff8127d9d9>] ____fput+0x9/0x10 [ 549.694534] [<ffffffff811348c2>] task_work_run+0xb2/0xf0 [ 549.696349] [<ffffffff81113c6d>] do_exit+0x36d/0x580 [ 549.698286] [<ffffffff8107d964>] ? syscall_trace_enter+0x24/0x2e0 [ 549.702729] [<ffffffff81113f4a>] do_group_exit+0x8a/0xc0 [ 549.706775] [<ffffffff81113f92>] sys_exit_group+0x12/0x20 [ 549.711088] [<ffffffff83cfab18>] tracesys+0xe1/0xe6 [ 549.728001] ---[ end trace 73eb41728f11f87e ]--- 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
The driver is no longer unthrottled on buffer reset, so remove comments that claim it is. Signed-off-by: Peter Hurley <peter@hurleysoftware.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
Peter Hurley authored
SAK work may schedule hangup work (if TTY_SOFT_SAK is defined), thus SAK work must be flushed before hangup work. Signed-off-by: Peter Hurley <peter@hurleysoftware.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
Peter Hurley authored
The pty driver does not obtain an ldisc reference to the linked tty when writing. When the ldiscs are sequentially halted, it is possible for one ldisc to be halted, and before the second ldisc can be halted, a concurrent write schedules buffer work on the first ldisc. This can lead to an access-after-free error when the scheduled buffer work starts on the closed ldisc. Prevent subsequent use after halt by performing each stage of the halt on both ttys. Signed-off-by: Peter Hurley <peter@hurleysoftware.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
Peter Hurley authored
In preparation for destructing and freeing the tty, the line discipline must first be brought to an inactive state before it can be destructed. This line discipline shutdown must: - disallow new users of the ldisc - wait for existing ldisc users to finish - only then, cancel/flush their pending/running work Factor tty_ldisc_wait_idle() from tty_set_ldisc() and tty_ldisc_kill() to ensure this shutdown order. Failure to provide this guarantee can result in scheduled work running after the tty has already been freed, as indicated in the following log message: [ 88.331234] WARNING: at drivers/tty/tty_buffer.c:435 flush_to_ldisc+0x194/0x1d0() [ 88.334505] Hardware name: Bochs [ 88.335618] tty is bad=-1 [ 88.335703] Modules linked in: netconsole configfs bnep rfcomm bluetooth ...... [ 88.345272] Pid: 39, comm: kworker/1:1 Tainted: G W 3.7.0-next-20121129+ttydebug-xeon #20121129+ttydebug [ 88.347736] Call Trace: [ 88.349024] [<ffffffff81058aff>] warn_slowpath_common+0x7f/0xc0 [ 88.350383] [<ffffffff81058bf6>] warn_slowpath_fmt+0x46/0x50 [ 88.351745] [<ffffffff81432bd4>] flush_to_ldisc+0x194/0x1d0 [ 88.353047] [<ffffffff816f7fe1>] ? _raw_spin_unlock_irq+0x21/0x50 [ 88.354190] [<ffffffff8108a809>] ? finish_task_switch+0x49/0xe0 [ 88.355436] [<ffffffff81077ad1>] process_one_work+0x121/0x490 [ 88.357674] [<ffffffff81432a40>] ? __tty_buffer_flush+0x90/0x90 [ 88.358954] [<ffffffff81078c84>] worker_thread+0x164/0x3e0 [ 88.360247] [<ffffffff81078b20>] ? manage_workers+0x120/0x120 [ 88.361282] [<ffffffff8107e230>] kthread+0xc0/0xd0 [ 88.362284] [<ffffffff816f0000>] ? cmos_do_probe+0x2eb/0x3bf [ 88.363391] [<ffffffff8107e170>] ? flush_kthread_worker+0xb0/0xb0 [ 88.364797] [<ffffffff816fff6c>] ret_from_fork+0x7c/0xb0 [ 88.366087] [<ffffffff8107e170>] ? flush_kthread_worker+0xb0/0xb0 [ 88.367266] ---[ end trace 453a7c9f38fbfec0 ]--- Signed-off-by: Peter Hurley <peter@hurleysoftware.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
Peter Hurley authored
tty_ldisc_halt() will use the file-scoped function, tty_ldisc_wait_idle(), in the following patch. Signed-off-by: Peter Hurley <peter@hurleysoftware.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
Peter Hurley authored
Flip buffer work cannot be cancelled until all outstanding ldisc references have been released. Convert the ldisc ref wait into a full ldisc halt with buffer work cancellation. Note that the legacy mutex is not held while cancelling. Signed-off-by: Peter Hurley <peter@hurleysoftware.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
Peter Hurley authored
Since the tty->ldisc is prevented from being changed by tty_set_ldisc() when a tty is being hung up, re-testing the ldisc user count is unnecessary -- ie, it cannot be a different ldisc and the user count cannot have increased (assuming the caller meets the precondition that TTY_LDISC flag is cleared) Removal of the 'early-out' locking optimization is necessary for the subsequent patch 'tty: Fix ldisc halt sequence on hangup'. Signed-off-by: Peter Hurley <peter@hurleysoftware.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
Peter Hurley authored
Refactor tty_ldisc_hangup() to extract standalone function, tty_ldisc_hangup_wait_idle(), to wait for ldisc references to be released. Signed-off-by: Peter Hurley <peter@hurleysoftware.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
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>
-