Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) client-ip=2620:137:e000::3:3;
Date:   Thu, 28 Sep 2023 10:44:36 +0200
From:   Uwe =?utf-8?Q?Kleine-K=C3=B6nig?= <u.kleine-koenig@pengutronix.de>
To:     Vinod Koul <vkoul@kernel.org>
Cc:     festevam@gmail.com, linux-serial@vger.kernel.org,
        gregkh@linuxfoundation.org, s.hauer@pengutronix.de,
        linux-kernel@vger.kernel.org, Chengfeng Ye <dg573847474@gmail.com>,
        sorganov@gmail.com, kernel@pengutronix.de,
        ilpo.jarvinen@linux.intel.com, dmaengine@vger.kernel.org,
        shawnguo@kernel.org, jirislaby@kernel.org,
        linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH RESEND] serial: imx: Fix potential deadlock on
 sport->port.lock
Message-ID: <20230928084436.fftw5sk4sixaedck@pengutronix.de>
References: <20230927181939.60554-1-dg573847474@gmail.com>
 <20230928060749.qzs6qgcesstclqqk@pengutronix.de>
 <ZRUtZ/M3Ml6ltc2m@matsya>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="r6y5twzpb32i5htn"
Content-Disposition: inline
In-Reply-To: <ZRUtZ/M3Ml6ltc2m@matsya>
Precedence: bulk


--r6y5twzpb32i5htn
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Hello Vinod,

thanks for your quick answer!

On Thu, Sep 28, 2023 at 01:08:15PM +0530, Vinod Koul wrote:
> On 28-09-23, 08:07, Uwe Kleine-K=F6nig wrote:
> > [Cc +=3D Vinod Koul, dmaengine@vger.kernel.org]
> >=20
> > Hello,
> >=20
> > On Wed, Sep 27, 2023 at 06:19:39PM +0000, Chengfeng Ye wrote:
> > > As &sport->port.lock is acquired under irq context along the following
> > > call chain from imx_uart_rtsint(), other acquisition of the same lock
> > > inside process context or softirq context should disable irq avoid do=
uble
> > > lock.
> > >=20
> > > <deadlock #1>
> > >=20
> > > imx_uart_dma_rx_callback()
> > > --> spin_lock(&sport->port.lock)
> > > <interrupt>
> > >    --> imx_uart_rtsint()
> > >    --> spin_lock(&sport->port.lock)
> > >=20
> > > This flaw was found by an experimental static analysis tool I am
> > > developing for irq-related deadlock.
> >=20
> > Ah, I understood before that you really experienced that deadlock (or a
> > lockdep splat). I didn't test anything, but I think the
> > imx_uart_dma_rx_callback() is called indirectly by
> > sdma_update_channel_loop() which is called in irq context. I don't know
> > if this is the case for all dma drivers?!
> >=20
> > @Vinod: Maybe you can chime in here: Is a dma callback always called in
> > irq context?
>=20
> Not in callback but a tasklet context. The DMA irq handler is supposed
> to use a tasklet for invoking the callback

So drivers/dma/imx-sdma.c is bogous as it calls

	-> sdma_int_handler()
	  -> sdma_update_channel_loop()
	    -> dmaengine_desc_get_callback_invoke()

resulting in imx_uart_dma_rx_callback() (and others) being called in irq
context, right?

In that case:

Acked-by: Uwe Kleine-K=F6nig <u.kleine-koenig@pengutronix.de>

(for the imx-UART patch that stops assuming imx_uart_dma_rx_callback()
is called with irqs off).

Best regards
Uwe

--=20
Pengutronix e.K.                           | Uwe Kleine-K=F6nig            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

--r6y5twzpb32i5htn
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQEzBAABCgAdFiEEP4GsaTp6HlmJrf7Tj4D7WH0S/k4FAmUVPPIACgkQj4D7WH0S
/k7vNwgAhb4pKXKJbH9aBhcqanU4xb1tK0/f8VK6sZW0aeeWi7M4Ix1HQFdLg9va
AS2/DwTdV7PHtMX5NJhPkqyUupi2SIJ7fXQo5pkN4p0sHBCzNIYrgpCtvXi/azSC
qKXuOOTw8A9HvoeTKNbKQm6n2Va/AaPORzE/uzZq/m7ni4JHuCSqZMEIzRtJ05/O
m6c2cs6FDkGfPE3B8WRWK4ClF5hFZ7mhdQr118Y8tb/cAQ6eG8BzGNMQJt555Rrn
plV5TSK5mnbmW/B2psEEVmQcBrwZz+vfbsoAvd45B8Lnstey+e9DX+Fmvs8I3Pse
rJmz4QDc2grxnjlCvVSBHtj4jEBOKQ==
=p5nE
-----END PGP SIGNATURE-----

--r6y5twzpb32i5htn--