On 16/03/2021 10:02, Johan Hovold wrote:

On Mon, Mar 15, 2021 at 07:12:12PM +0100, Krzysztof Kozlowski wrote:

quoted

Since interrupt handler is called with disabled local interrupts, there
is no need to use the spinlock primitives disabling interrupts as well.

This isn't generally true due to "threadirqs" and that can lead to
deadlocks if the console code is called from hard irq context.

Now, this is *not* the case for this particular driver since it doesn't
even bother to take the port lock in console_write(). That should
probably be fixed instead.

See https://lore.kernel.org/r/X7kviiRwuxvPxC8O@localhost (local).

Thanks for the link, quite interesting! For one type of device we have
two interrupts (RX and TX) so I guess it's a valid point/risk. However
let me try to understand it more.

Assuming we had only one interrupt line, how this interrupt handler with
threadirqs could be called from hardirq context?

You wrote there:

For console drivers this can even happen for the same interrupt as the
generic interrupt code can call printk(), and so can any other handler
that isn't threaded (e.g. hrtimers or explicit IRQF_NO_THREAD).

However I replaced here only interrupt handler's spin lock to non-irq.
This code path will be executed only when interrupt is masked therefore
for one interrupt line there is *no possibility of*:

-> s3c64xx_serial_handle_irq
   - interrupts are masked
   - s3c24xx_serial_tx_irq
     - spin_lock()
                       -> hrtimers or other IRQF_NO_THREAD
                          - console_write() or something
                            - s3c64xx_serial_handle_irq
                              - s3c24xx_serial_tx_irq
                                - spin_lock()


Best regards,
Krzysztof