kernel/linux.git/include/linux/tty.h, branch v3.12.17

tty: Fix lock order in tty_do_resize()

2013-07-24T22:12:53+00:00

Commits 6a1c0680cf3ba94356ecd58833e1540c93472a57 and 9356b535fcb71db494fc434acceb79f56d15bda2, respectively 'tty: Convert termios_mutex to termios_rwsem' and 'n_tty: Access termios values safely' introduced a circular lock dependency with console_lock and termios_rwsem. The lockdep report [1] shows that n_tty_write() will attempt to claim console_lock while holding the termios_rwsem, whereas tty_do_resize() may already hold the console_lock while claiming the termios_rwsem. Since n_tty_write() and tty_do_resize() do not contend over the same data -- the tty->winsize structure -- correct the lock dependency by introducing a new lock which specifically serializes access to tty->winsize only. [1] Lockdep report ====================================================== [ INFO: possible circular locking dependency detected ] 3.10.0-0+tip-xeon+lockdep #0+tip Not tainted ------------------------------------------------------- modprobe/277 is trying to acquire lock: (&tty->termios_rwsem){++++..}, at: [] tty_do_resize+0x36/0xe0 but task is already holding lock: ((fb_notifier_list).rwsem){.+.+.+}, at: [] __blocking_notifier_call_chain+0x56/0xc0 which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #2 ((fb_notifier_list).rwsem){.+.+.+}: [] lock_acquire+0x92/0x1f0 [] down_read+0x47/0x5c [] __blocking_notifier_call_chain+0x56/0xc0 [] blocking_notifier_call_chain+0x16/0x20 [] fb_notifier_call_chain+0x1b/0x20 [] register_framebuffer+0x1e2/0x320 [] drm_fb_helper_initial_config+0x371/0x540 [drm_kms_helper] [] nouveau_fbcon_init+0x105/0x140 [nouveau] [] nouveau_drm_load+0x43f/0x610 [nouveau] [] drm_get_pci_dev+0x17e/0x2a0 [drm] [] nouveau_drm_probe+0x25a/0x2a0 [nouveau] [] local_pci_probe+0x4b/0x80 [] pci_device_probe+0x111/0x120 [] driver_probe_device+0x8b/0x3a0 [] __driver_attach+0xab/0xb0 [] bus_for_each_dev+0x5d/0xa0 [] driver_attach+0x1e/0x20 [] bus_add_driver+0x111/0x290 [] driver_register+0x77/0x170 [] __pci_register_driver+0x64/0x70 [] drm_pci_init+0x11a/0x130 [drm] [] nouveau_drm_init+0x4d/0x1000 [nouveau] [] do_one_initcall+0xea/0x1a0 [] load_module+0x123b/0x1bf0 [] SyS_init_module+0xd7/0x120 [] system_call_fastpath+0x16/0x1b -> #1 (console_lock){+.+.+.}: [] lock_acquire+0x92/0x1f0 [] console_lock+0x77/0x80 [] con_flush_chars+0x31/0x50 [] n_tty_write+0x1ec/0x4d0 [] tty_write+0x159/0x2e0 [] redirected_tty_write+0xb5/0xc0 [] vfs_write+0xc5/0x1f0 [] SyS_write+0x55/0xa0 [] system_call_fastpath+0x16/0x1b -> #0 (&tty->termios_rwsem){++++..}: [] __lock_acquire+0x1c43/0x1d30 [] lock_acquire+0x92/0x1f0 [] down_write+0x44/0x70 [] tty_do_resize+0x36/0xe0 [] vc_do_resize+0x3e1/0x4c0 [] vc_resize+0x1f/0x30 [] fbcon_init+0x385/0x5a0 [] visual_init+0xbc/0x120 [] do_bind_con_driver+0x163/0x320 [] do_take_over_console+0x61/0x70 [] do_fbcon_takeover+0x63/0xc0 [] fbcon_event_notify+0x715/0x820 [] notifier_call_chain+0x5d/0x110 [] __blocking_notifier_call_chain+0x6c/0xc0 [] blocking_notifier_call_chain+0x16/0x20 [] fb_notifier_call_chain+0x1b/0x20 [] register_framebuffer+0x1e2/0x320 [] drm_fb_helper_initial_config+0x371/0x540 [drm_kms_helper] [] nouveau_fbcon_init+0x105/0x140 [nouveau] [] nouveau_drm_load+0x43f/0x610 [nouveau] [] drm_get_pci_dev+0x17e/0x2a0 [drm] [] nouveau_drm_probe+0x25a/0x2a0 [nouveau] [] local_pci_probe+0x4b/0x80 [] pci_device_probe+0x111/0x120 [] driver_probe_device+0x8b/0x3a0 [] __driver_attach+0xab/0xb0 [] bus_for_each_dev+0x5d/0xa0 [] driver_attach+0x1e/0x20 [] bus_add_driver+0x111/0x290 [] driver_register+0x77/0x170 [] __pci_register_driver+0x64/0x70 [] drm_pci_init+0x11a/0x130 [drm] [] nouveau_drm_init+0x4d/0x1000 [nouveau] [] do_one_initcall+0xea/0x1a0 [] load_module+0x123b/0x1bf0 [] SyS_init_module+0xd7/0x120 [] system_call_fastpath+0x16/0x1b other info that might help us debug this: Chain exists of: &tty->termios_rwsem --> console_lock --> (fb_notifier_list).rwsem Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock((fb_notifier_list).rwsem); lock(console_lock); lock((fb_notifier_list).rwsem); lock(&tty->termios_rwsem); *** DEADLOCK *** 7 locks held by modprobe/277: #0: (&__lockdep_no_validate__){......}, at: [] __driver_attach+0x5b/0xb0 #1: (&__lockdep_no_validate__){......}, at: [] __driver_attach+0x69/0xb0 #2: (drm_global_mutex){+.+.+.}, at: [] drm_get_pci_dev+0xbd/0x2a0 [drm] #3: (registration_lock){+.+.+.}, at: [] register_framebuffer+0x25/0x320 #4: (&fb_info->lock){+.+.+.}, at: [] lock_fb_info+0x26/0x60 #5: (console_lock){+.+.+.}, at: [] register_framebuffer+0x1d4/0x320 #6: ((fb_notifier_list).rwsem){.+.+.+}, at: [] __blocking_notifier_call_chain+0x56/0xc0 stack backtrace: CPU: 0 PID: 277 Comm: modprobe Not tainted 3.10.0-0+tip-xeon+lockdep #0+tip Hardware name: Dell Inc. Precision WorkStation T5400 /0RW203, BIOS A11 04/30/2012 ffffffff8213e5e0 ffff8802aa2fb298 ffffffff81755f19 ffff8802aa2fb2e8 ffffffff8174f506 ffff8802aa2fa000 ffff8802aa2fb378 ffff8802aa2ea8e8 ffff8802aa2ea910 ffff8802aa2ea8e8 0000000000000006 0000000000000007 Call Trace: [] dump_stack+0x19/0x1b [] print_circular_bug+0x1fb/0x20c [] __lock_acquire+0x1c43/0x1d30 [] ? mark_held_locks+0xae/0x120 [] ? trace_hardirqs_on_caller+0x105/0x1d0 [] lock_acquire+0x92/0x1f0 [] ? tty_do_resize+0x36/0xe0 [] down_write+0x44/0x70 [] ? tty_do_resize+0x36/0xe0 [] tty_do_resize+0x36/0xe0 [] vc_do_resize+0x3e1/0x4c0 [] vc_resize+0x1f/0x30 [] fbcon_init+0x385/0x5a0 [] visual_init+0xbc/0x120 [] do_bind_con_driver+0x163/0x320 [] do_take_over_console+0x61/0x70 [] do_fbcon_takeover+0x63/0xc0 [] fbcon_event_notify+0x715/0x820 [] notifier_call_chain+0x5d/0x110 [] __blocking_notifier_call_chain+0x6c/0xc0 [] blocking_notifier_call_chain+0x16/0x20 [] fb_notifier_call_chain+0x1b/0x20 [] register_framebuffer+0x1e2/0x320 [] drm_fb_helper_initial_config+0x371/0x540 [drm_kms_helper] [] ? kmemleak_alloc+0x5b/0xc0 [] ? kmem_cache_alloc_trace+0x104/0x290 [] ? drm_fb_helper_single_add_all_connectors+0x81/0xf0 [drm_kms_helper] [] nouveau_fbcon_init+0x105/0x140 [nouveau] [] nouveau_drm_load+0x43f/0x610 [nouveau] [] drm_get_pci_dev+0x17e/0x2a0 [drm] [] nouveau_drm_probe+0x25a/0x2a0 [nouveau] [] ? _raw_spin_unlock_irqrestore+0x42/0x80 [] local_pci_probe+0x4b/0x80 [] pci_device_probe+0x111/0x120 [] driver_probe_device+0x8b/0x3a0 [] __driver_attach+0xab/0xb0 [] ? driver_probe_device+0x3a0/0x3a0 [] bus_for_each_dev+0x5d/0xa0 [] driver_attach+0x1e/0x20 [] bus_add_driver+0x111/0x290 [] ? 0xffffffffa0229fff [] driver_register+0x77/0x170 [] ? 0xffffffffa0229fff [] __pci_register_driver+0x64/0x70 [] drm_pci_init+0x11a/0x130 [drm] [] ? 0xffffffffa0229fff [] ? 0xffffffffa0229fff [] nouveau_drm_init+0x4d/0x1000 [nouveau] [] do_one_initcall+0xea/0x1a0 [] load_module+0x123b/0x1bf0 [] ? ddebug_proc_open+0xb0/0xb0 [] ? trace_hardirqs_on_thunk+0x3a/0x3f [] SyS_init_module+0xd7/0x120 [] system_call_fastpath+0x16/0x1b Signed-off-by: Peter Hurley Signed-off-by: Greg Kroah-Hartman

n_tty: Fix EOF push handling

2013-07-24T00:08:40+00:00

In canonical mode, an EOF which is not the first character of the line causes read() to complete and return the number of characters read so far (commonly referred to as EOF push). However, if the previous read() returned because the user buffer was full _and_ the next character is an EOF not at the beginning of the line, read() must not return 0, thus mistakenly indicating the end-of-file condition. The TTY_PUSH flag is used to indicate an EOF was received which is not at the beginning of the line. Because the EOF push condition is evaluated by a thread other than the read(), multiple EOF pushes can cause a premature end-of-file to be indicated. Instead, discover the 'EOF push as first read character' condition from the read() thread itself, and restart the i/o loop if detected. Signed-off-by: Peter Hurley Signed-off-by: Greg Kroah-Hartman

tty: Remove private constant from global namespace

2013-07-23T23:47:10+00:00

TTY_BUFFER_PAGE is only used within drivers/tty/tty_buffer.c; relocate to that file scope. Signed-off-by: Peter Hurley Signed-off-by: Greg Kroah-Hartman

tty: Fix unsafe vt paste_selection()

2013-07-23T23:47:10+00:00

Convert the tty_buffer_flush() exclusion mechanism to a public interface - tty_buffer_lock/unlock_exclusive() - and use the interface to safely write the paste selection to the line discipline. Signed-off-by: Peter Hurley Signed-off-by: Greg Kroah-Hartman

tty: Use non-atomic state to signal flip buffer flush pending

2013-07-23T23:47:09+00:00

Atomic bit ops are no longer required to indicate a flip buffer flush is pending, as the flush_mutex is sufficient barrier. Remove the unnecessary port .iflags field and localize flip buffer state to struct tty_bufhead. Signed-off-by: Peter Hurley Signed-off-by: Greg Kroah-Hartman

tty: Avoid false-sharing flip buffer ptrs

2013-07-23T23:47:09+00:00

Separate the head and tail ptrs to avoid cache-line contention (so called 'false-sharing') between concurrent threads. Signed-off-by: Peter Hurley Signed-off-by: Greg Kroah-Hartman

tty: Only perform flip buffer flush from tty_buffer_flush()

2013-07-23T23:47:09+00:00

Now that dropping the buffer lock is not necessary (as result of converting the spin lock to a mutex), the flip buffer flush no longer needs to be handled by the buffer work. Simply signal a flush is required; the buffer work will exit the i/o loop, which allows tty_buffer_flush() to proceed. Signed-off-by: Peter Hurley Signed-off-by: Greg Kroah-Hartman

tty: Ensure single-threaded flip buffer consumer with mutex

2013-07-23T23:47:09+00:00

The buffer work may race with parallel tty_buffer_flush. Use a mutex to guarantee exclusive modify access to the head flip buffer. Remove the unneeded spin lock. Signed-off-by: Peter Hurley Signed-off-by: Greg Kroah-Hartman

tty: Track flip buffer memory limit atomically

2013-07-23T23:47:08+00:00

Lockless flip buffers require atomically updating the bytes-in-use watermark. The pty driver also peeks at the watermark value to limit memory consumption to a much lower value than the default; query the watermark with new fn, tty_buffer_space_avail(). Signed-off-by: Peter Hurley Signed-off-by: Greg Kroah-Hartman

tty: Simplify flip buffer list with 0-sized sentinel

2013-07-23T23:47:08+00:00

Use a 0-sized sentinel to avoid assigning the head ptr from the driver side thread. This also eliminates testing head/tail for NULL. When the sentinel is first 'consumed' by the buffer work (or by tty_buffer_flush()), it is detached from the list but not freed nor added to the free list. Both buffer work and tty_buffer_flush() continue to preserve at least 1 flip buffer to which head & tail is pointed. Signed-off-by: Peter Hurley Signed-off-by: Greg Kroah-Hartman