On 10.09.2015 22:41, Robert Baldyga wrote:

This patch introduces s3c24xx_serial_rx_drain_fifo() which reads data
from RX FIFO and writes it to tty buffer. It also checks for special
conditions (such as 'break') and handles it. This function has been
separated from s3c24xx_serial_rx_chars_pio() as it contains code which
can be used also in DMA mode.

Much better, thanks! Now it is also easier to spot the difference (see
below).

Here is a difference - previously this was outside of spinlock. I think
moving it inside spin lock is okay, just the interrupts won't be
disabled before unlock and queue_work() from tty_flip_buffer_push().

However after testing this patchset (entire) on:
next-20150910 + my dt-for-next branch (dma for serial) + this patchset
you can see quite complicated lockdep warning:

[    3.568657] =========================================================
[    3.575079] [ INFO: possible irq lock inversion dependency detected ]
[    3.581506] 4.2.0-next-20150910-00009-g65fd5a9cff54 #218 Not tainted
[    3.587838] ---------------------------------------------------------
[    3.594263] swapper/0/0 just changed the state of lock:
[    3.599470]  (&port_lock_key){..-...}, at: [<c02a8fac>]
s3c24xx_serial_tx_dma_complete+0x8c/0xfc
[    3.608237] but this lock took another, SOFTIRQ-unsafe lock in the past:
[    3.614919]  (&(&ctx->lock)->rlock){+.+...}
[    3.614919]
[    3.614919] and interrupts could create inverse lock ordering between
them.
[    3.614919]
[    3.625076]
[    3.625076] other info that might help us debug this:
[    3.631586]  Possible interrupt unsafe locking scenario:
[    3.631586]
[    3.638356]        CPU0                    CPU1
[    3.642870]        ----                    ----
[    3.647382]   lock(&(&ctx->lock)->rlock);
[    3.651376]                                local_irq_disable();
[    3.657278]                                lock(&port_lock_key);
[    3.663267]                                lock(&(&ctx->lock)->rlock);
[    3.669777]   <Interrupt>
[    3.672381]     lock(&port_lock_key);


Config: exynos, disabled MMC_CLKGATE, enabled usual testing stuff
Board: Trats2

Didn't you notice it?


Additionally the SysRq "Show backtrace of all active CPUs" on this
linux-next (without additional patches, pure next) has significant delay
(like 5 seconds) and a:
[  169.221223] s3c-i2c 138d0000.i2c: timeout waiting for bus idle

That's weird. But as I said this occurs on pure next as well.

Dmesgs and config attached.

Best regards,
Krzysztof

+}
+
+static irqreturn_t s3c24xx_serial_rx_chars_pio(void *dev_id)
+{
+	struct s3c24xx_uart_port *ourport = dev_id;
+	struct uart_port *port = &ourport->port;
+	unsigned long flags;
+
+	spin_lock_irqsave(&port->lock, flags);
+	s3c24xx_serial_rx_drain_fifo(ourport);
+	spin_unlock_irqrestore(&port->lock, flags);
 
-out:
 	return IRQ_HANDLED;
 }