On 08/19/2013 08:18 PM, Viresh Kumar wrote:
Hi Peter,
On 19 August 2013 21:01, Peter Hurley peter@hurleysoftware.com wrote:
Please review commit 677fe555cbfb188af58cce105f4dae9505e58c31 'serial: imx: Fix recursive locking bug' to see if that is actually the problem you're having with the samsung serial driver.
No, this doesn't seem to be the same issue... The issue here is:
s3c24xx_serial_rx_chars() -> spin_lock_irqsave(&port->lock, flags); -> tty_flip_buffer_push() -> flush_to_ldisc() -> n_tty_receive_buf2() -> __receive_buf() -> uart_start() -> spin_lock_irqsave(&port->lock, flags);
Nope.
First, you have two stack traces from your previous message. The BUG report is the second stack trace:
worker_thread process_one_work flush_to_ldisc n_tty_receive_buf2 __receive_buf uart_start raw_spin_lock_irqsave(&port->lock)
IOW, no lock recursion because it's running on a separate kworker thread, and did not arrive from s3c24xx_serial_rx_chars() (other than indirectly via schedule_work()).
It's BUGing because the uart_port spinlock cannot be acquired _even though apparently there is no owning CPU_.
The key to why the flush_to_disc() worker thread cannot acquire the uart_port spinlock is in the first stack trace:
s3c24xx_serial_rx_chars raw_spin_unlock_irqrestore do_raw_spin_unlock dump_stack
So the real question here is why do_raw_spin_unlock() is doing a dump_stack()? Is one of the SPIN_BUG_ON() asserts in debug_spin_unlock() triggering?
This seems to be a generic enough problem as many drivers are already doing the right thing. They release lock before calling tty_flip_buffer_push()..
No. Those drivers are carrying vestige code from the original 2.6 tree import.
As documented in tty_flip_buffer_push():
* Queue a push of the terminal flip buffers to the line discipline. This * function must not be called from IRQ context if port->low_latency is * set.
IOW, s3c24xx_serial_rx_chars() executing in IRQ context cannot call flush_to_ldisc() -- the work must be scheduled instead. Since flush_to_ldisc() will be scheduled on a separate kworker thread, the uart_port spin lock will not be recursively claimed.
So, recursive locking of the uart_port lock is not the problem (at least, not wrt the flush_to_ldisc() call chain).
Regards, Peter Hurley