Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Camelia Alexandra Groza <camelia.groza@nxp.com>
To:     Sean Anderson <sean.anderson@seco.com>,
        Leo Li <leoyang.li@nxp.com>,
        "linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>
CC:     Scott Wood <oss@buserror.net>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "David S . Miller" <davem@davemloft.net>,
        Claudiu Manoil <claudiu.manoil@nxp.com>,
        Roy Pledge <roy.pledge@nxp.com>,
        Vladimir Oltean <vladimir.oltean@nxp.com>,
        Sean Anderson <sean.anderson@seco.com>
Subject: RE: [PATCH 1/2] soc: fsl: qbman: Always disable interrupts when
 taking cgr_lock
Thread-Topic: [PATCH 1/2] soc: fsl: qbman: Always disable interrupts when
 taking cgr_lock
Thread-Index: AQHZYOJtsilz0/ZTyk+UEV4EcVW4Za8Uuv6g
Date:   Fri, 31 Mar 2023 10:58:44 +0000
Message-ID: <AM6PR04MB57997C946E6AE7F2D87B5C78F28F9@AM6PR04MB5799.eurprd04.prod.outlook.com>
References: <20230327192841.952688-1-sean.anderson@seco.com>
In-Reply-To: <20230327192841.952688-1-sean.anderson@seco.com>
Accept-Language: en-GB, ro-RO, en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?OldC7kW4hpBFuSDn51RIdQo/9ASAI67h1YokPQy5l6JZ/1bPObkADFb7mnIp?=
 =?us-ascii?Q?765SfPlCnfVA/c1e5STtyDeGa0PTo6XRw9dV5aU6zo67VIZ4UfyyyU0luQYq?=
 =?us-ascii?Q?XE25Dtq0uG9u1NCfom/9yrRrL/bMySTCTV2rpZg34yqHNpk09ZMMrAccz9mf?=
 =?us-ascii?Q?P10mGAlLIfdmGQkHJ6mdrorsEi73b8Zz5fm70knIIOSBTW3vwZgxgGsXdpnE?=
 =?us-ascii?Q?wVtsRm7AwRDEi/NFeEG+JfVA9QZ6xiUwC5ayAfxDbgOuyStoGeKLs54qmqmw?=
 =?us-ascii?Q?Q4cbBS4FLtYN2LHlaT1B8qBsz1MTOcJalmbgBnCU8QdtTRkrUd5p7i5UG5ep?=
 =?us-ascii?Q?ZMqgorKOmGe4MzGT5nJZ/Si1gV7cr80hmsoRTptrDk2ogutYjFsmt94ze34L?=
 =?us-ascii?Q?xSaHcMMfyzdI62SWKsGBEDA+CtcFs+KzFk+CpG2v1diDJGsNtjF9Y2w6YEne?=
 =?us-ascii?Q?hPySHZ0fm8rdNUFiV4NnrhkND2ouYYCUd5EtIxjcr5DPuqO4Qo/0QJ1AWoUy?=
 =?us-ascii?Q?JuJxlRgbl5LWKXD/ys75UPsu4r/hvYkFZpOXODop8Ity/14K9fbtNf1kX+fU?=
 =?us-ascii?Q?hIhmBuU/owc0pEm1N/x0UJk+vTJDoJBimb81k7yKZ1OovXeCdX2fWVC8f6pL?=
 =?us-ascii?Q?A/avBQtZJ2eXIuHI1ebKeaEgENQRYxepqGx2Me+mZrBgJOzI+1sNdysdlyQk?=
 =?us-ascii?Q?4jE5LgolRoKwI/Fg3hi+mpw3l+Qq08C+WR/C0GDgE30ta8hhIR3XbrT5Pvrh?=
 =?us-ascii?Q?NtSmc+ala5FKchqbdABU+U3y4SRJb7BuAH6ONhoQjs3ePBM5mwbukzVRzxu5?=
 =?us-ascii?Q?SXwezasfe81EheParKGZixKput17KX8HXsDHEU2J8wDLLHMBb8hZLAgUqRCy?=
 =?us-ascii?Q?M+g+vaaqswKZT15pjDSMzGwiSdaMa6GU1K/AB15IwaEMlMiMTk9Kh/hUGBJA?=
 =?us-ascii?Q?ffPpdO7jHfhZ9pa/BU/FEfqoRsBrF4+2jrvVxxKSLs2AZzLOcFApIfPEado5?=
 =?us-ascii?Q?qtD0g265c8F7exjOlISoQwKGCJQyrlK41otKzKwcj4uiPbucMPMh5CcG6mEN?=
 =?us-ascii?Q?xnWN0DhaMVR6vosJfKoL893WAGxPa2c+ImJjDyN/vUiwVSTt0tRhi0v0wsgm?=
 =?us-ascii?Q?Yt8tkkBaZo/Ac9n2Mo8C/n3wt76/DQxPVKkN39R9KBcXpJM6lmaUGfGmKN0S?=
 =?us-ascii?Q?02fvBopT/w5thwuNeDzm8Gr85GANs8/iOMaiWnX6GRe8kvuxY/+gGINCmdh9?=
 =?us-ascii?Q?ao8pBE+lDALtFGy3iP8YK8Bp8tF/H7mnwAJhl2mKdtnjXG4fTjX7iAGla0xX?=
 =?us-ascii?Q?MW9qtq7HhMfdBjFyICOYOhCyTaMOGv74DkKJ3z6AdeChVmpgX4o4vGnHbCu/?=
 =?us-ascii?Q?gtuRkjM7Bz2i6npx8Mqgjo7TxJ9pXOg+lJQc9OdV3A6g025KUFGU6Nljz9wa?=
 =?us-ascii?Q?MJaAGbVhSNDD8uwHgNQZV1Ix1GWtSMHBkL9NpwhaHoJY0BeI7bAYw4tZxgXf?=
 =?us-ascii?Q?gXk8/mu3iVZNi1vFItmMYtRPu527imKNfZHil91z/K6KpVP+oEyvRhm/BBXA?=
 =?us-ascii?Q?h09iUFP9EJfap3EUGi3jkICdCq5UstVMjq4mAKIr?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: AM6PR04MB5799.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 0e1e1a7b-5575-4ace-5a05-08db31d6e55b
X-MS-Exchange-CrossTenant-originalarrivaltime: 31 Mar 2023 10:58:44.3399
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: kMzQHCXl4eaq2L5wua16+L74AhTVK0YhrXVjUmiTXvFmPcl76yOMZKbbsIX2Dji4FpcgJcCGwJtNBOhXlHKfow==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PAXPR04MB9325
X-Spam-Status: No, score=-0.2 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
        DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,SPF_HELO_PASS,
        T_SPF_PERMERROR autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> -----Original Message-----
> From: Sean Anderson <sean.anderson@seco.com>
> Sent: Monday, March 27, 2023 22:29
> To: Leo Li <leoyang.li@nxp.com>; linuxppc-dev@lists.ozlabs.org; linux-arm=
-
> kernel@lists.infradead.org
> Cc: Scott Wood <oss@buserror.net>; linux-kernel@vger.kernel.org; David S =
.
> Miller <davem@davemloft.net>; Claudiu Manoil <claudiu.manoil@nxp.com>;
> Roy Pledge <roy.pledge@nxp.com>; Vladimir Oltean
> <vladimir.oltean@nxp.com>; Camelia Alexandra Groza
> <camelia.groza@nxp.com>; Sean Anderson <sean.anderson@seco.com>
> Subject: [PATCH 1/2] soc: fsl: qbman: Always disable interrupts when taki=
ng
> cgr_lock
>=20
> smp_call_function_single disables IRQs when executing the callback. To
> prevent deadlocks, we must disable IRQs when taking cgr_lock elsewhere.
> This is already done by qman_update_cgr and qman_delete_cgr; fix the
> other lockers.
>=20
> Fixes: c535e923bb97 ("soc/fsl: Introduce DPAA 1.x QMan device driver")
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
> ---
>=20
>  drivers/soc/fsl/qbman/qman.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>=20
> diff --git a/drivers/soc/fsl/qbman/qman.c b/drivers/soc/fsl/qbman/qman.c
> index 739e4eee6b75..ff870ca07596 100644
> --- a/drivers/soc/fsl/qbman/qman.c
> +++ b/drivers/soc/fsl/qbman/qman.c
> @@ -1456,7 +1456,7 @@ static void qm_congestion_task(struct work_struct
> *work)
>  	union qm_mc_result *mcr;
>  	struct qman_cgr *cgr;
>=20
> -	spin_lock(&p->cgr_lock);
> +	spin_lock_irq(&p->cgr_lock);
>  	qm_mc_start(&p->p);
>  	qm_mc_commit(&p->p, QM_MCC_VERB_QUERYCONGESTION);
>  	if (!qm_mc_result_timeout(&p->p, &mcr)) {
> @@ -1476,7 +1476,7 @@ static void qm_congestion_task(struct work_struct
> *work)
>  	list_for_each_entry(cgr, &p->cgr_cbs, node)
>  		if (cgr->cb && qman_cgrs_get(&c, cgr->cgrid))
>  			cgr->cb(p, cgr, qman_cgrs_get(&rr, cgr->cgrid));
> -	spin_unlock(&p->cgr_lock);
> +	spin_unlock_irq(&p->cgr_lock);
>  	qman_p_irqsource_add(p, QM_PIRQ_CSCI);
>  }

There is one more spin_unlock call in qm_congestion_task on the error path =
that needs updating:

if (!qm_mc_result_timeout(&p->p, &mcr)) {
	spin_unlock(&p->cgr_lock);

Regards,
Camelia

> @@ -2440,7 +2440,7 @@ int qman_create_cgr(struct qman_cgr *cgr, u32
> flags,
>  	preempt_enable();
>=20
>  	cgr->chan =3D p->config->channel;
> -	spin_lock(&p->cgr_lock);
> +	spin_lock_irq(&p->cgr_lock);
>=20
>  	if (opts) {
>  		struct qm_mcc_initcgr local_opts =3D *opts;
> @@ -2477,7 +2477,7 @@ int qman_create_cgr(struct qman_cgr *cgr, u32
> flags,
>  	    qman_cgrs_get(&p->cgrs[1], cgr->cgrid))
>  		cgr->cb(p, cgr, 1);
>  out:
> -	spin_unlock(&p->cgr_lock);
> +	spin_unlock_irq(&p->cgr_lock);
>  	put_affine_portal();
>  	return ret;
>  }
> --
> 2.35.1.1320.gc452695387.dirty