Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH] tty: serial: samsung_tty: remove spinlock flags in
 interrupt handlers
To:     Johan Hovold <johan@kernel.org>
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Jiri Slaby <jirislaby@kernel.org>,
        linux-arm-kernel@lists.infradead.org,
        linux-samsung-soc@vger.kernel.org, linux-serial@vger.kernel.org,
        linux-kernel@vger.kernel.org, marcan@marcan.st, arnd@kernel.org,
        Andy Shevchenko <andy.shevchenko@gmail.com>
References: <20210315181212.113217-1-krzysztof.kozlowski@canonical.com>
 <YFB0OcBg3Vj555eA@hovoldconsulting.com>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
Message-ID: <7f348e4c-3051-13cf-d461-eeda0ef53fdd@canonical.com>
Date:   Tue, 16 Mar 2021 10:47:53 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.7.1
MIME-Version: 1.0
In-Reply-To: <YFB0OcBg3Vj555eA@hovoldconsulting.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 16/03/2021 10:02, Johan Hovold wrote:
> On Mon, Mar 15, 2021 at 07:12:12PM +0100, Krzysztof Kozlowski wrote:
>> 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.

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