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:   Sunil Kovvuri Goutham <sgoutham@marvell.com>
To:     Tejun Heo <tj@kernel.org>,
        "jiangshanlai@gmail.com" <jiangshanlai@gmail.com>
CC:     "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "kernel-team@meta.com" <kernel-team@meta.com>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>
Subject: RE: [EXT] [PATCH 06/22] net: thunderx: Use alloc_ordered_workqueue()
 to create ordered workqueues
Thread-Topic: [EXT] [PATCH 06/22] net: thunderx: Use alloc_ordered_workqueue()
 to create ordered workqueues
Thread-Index: AQHZc/wfNvpO3cXK6kORNJjVFXR2Iq81ShaA
Date:   Fri, 21 Apr 2023 06:19:03 +0000
Message-ID: <BY3PR18MB4737E75C2FB751C64935220FC6609@BY3PR18MB4737.namprd18.prod.outlook.com>
References: <20230421025046.4008499-1-tj@kernel.org>
 <20230421025046.4008499-7-tj@kernel.org>
In-Reply-To: <20230421025046.4008499-7-tj@kernel.org>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?CcnDFosyqq2l6b8HuJu+rVi5gnXeP4dSVZpoTlRHSaLHie2DUrh7VD8Yo4SD?=
 =?us-ascii?Q?W+7WGjwhnfGCuoQXF/fezI+ezPFDUU73MrGdZqML/P4X2rJaDxZSF6Fsk6Yj?=
 =?us-ascii?Q?3vdkmyjNiXVgj+u92LToIwsoY4kFSnZByZIpFKzTYYtB/3uroMf8sDnazJ9D?=
 =?us-ascii?Q?FIOLJYX/G/MLbnAdpM9OxXH6CTXODvcsoOND7Ji3VrVWPGIsTyvhmtkP/2z1?=
 =?us-ascii?Q?78nALt6QW4hDY7V8FLRksPJFYGP12fh5nLhCBLe9tgb0EeBOoxyGdw8TGycu?=
 =?us-ascii?Q?c2G/KsFb5jCF9Y6iX1DhBkhpka/uDtrfLo76/9ucwRPhjQKLSvkfmPscrWHG?=
 =?us-ascii?Q?W3dvwGHqOlZsmuWYhNN0WrBxL6J/EgiJgoEZQ9wzyrW3g77jqL6s77IbRm6i?=
 =?us-ascii?Q?JO63dCWiOk3yiP+lHEdr1yXggS6TU6A1OY9q3V4johyldZD8Hj1WGrPfJqfo?=
 =?us-ascii?Q?aPDMc/FSRdWy457yvZwT7WBsfRWln3wD+iIeTmUIWBDV5z2/J9xoePTHQSW4?=
 =?us-ascii?Q?4z1LlcI73DhzWdei/qrV83zZ454PycdG9GjiDPIjYr2KuM7BIPwPc92SL0ux?=
 =?us-ascii?Q?Agr0GFCGzCnYWMHqGPcCkelUf5+5xjfbzsGj+P01U0IkS57O9oJGlJkW4/yW?=
 =?us-ascii?Q?rmgl7ADWsxC+Q2cUG1XkwbyqAMToiDjG/dbyqi5VZZukMar5O/PY3XWcd5wC?=
 =?us-ascii?Q?IfJ8/PLS2CswlAK+Vu0hrMewbjrGeOdmhPeIPdRduBTC5A4+7zcSjwl06I7s?=
 =?us-ascii?Q?Hf3t/YGTF5LXYLL1TrSKnmI0eFcHwrptTo5uFoMnifOQOoI/P9ZQA11YhQl3?=
 =?us-ascii?Q?VyfQIhubDxsnicDGSrlk1RUKP4jyaG/GhTX9xeF0QH5osiHtQy+iqf0/0lSQ?=
 =?us-ascii?Q?/JsrFoku5cU9EEbKPzRPCyk6a1PDC0sVmPx8uV2JzwLWtjK1Jttm2P31/cQh?=
 =?us-ascii?Q?mnLY3ZYu/nHg8PeBLxVgmrrhFhZxreES7vgS8gP6HhGMgLz9HWl6EVtycqmS?=
 =?us-ascii?Q?2FNRROG3JCQwEbl+MRQEQ2qB46zdhFHUjUfuODYvv9nQSI7rVPxh4b199DtG?=
 =?us-ascii?Q?e3j2Oeac4cGBq9yL8/G0Heg26LZLNHXU9Z9Y59pGpl9HEdmdzEIEyIVStFu+?=
 =?us-ascii?Q?eXeY8rCsGEwoSHeL/5lrDlud29VekhEsOvjBP8ToL0w0hxX7CHDO49gfkN+j?=
 =?us-ascii?Q?JRgX2OhSUnAfhdaqk28Azl2Zy6VHyMLEf/wD2xa+SuIHgPMr7+hGDrkZq0Xm?=
 =?us-ascii?Q?125Y4W4jWVBsebITUvZOHPBArl+VOXrrvKCL3m2V3p6/CuQL/ZeRvM9QALTO?=
 =?us-ascii?Q?VHyitXIRvew3eGXVeXrunkQM+NeenqZMdaxt5Ms2jH1KA3c+ZtBXrF2mgo0Q?=
 =?us-ascii?Q?c1t1GbKz6jJBVnoLr51j6dH+xCiZNJaBzMtzzkxtQxhsi4ZNcC7txGjUxoxV?=
 =?us-ascii?Q?9w+/dQWNP7Wna+JC4BBlGfoLFcnz38Vpkyo7W0FZ49k3h+4v6YVnrDrj8/QN?=
 =?us-ascii?Q?2DiUef6gF+Gbyn+oRUYcqDcN3oxvTNynFH6EEWG59Kwa1dt6KniaDIf5cNqk?=
 =?us-ascii?Q?noxhmgmwg2Y/s3NWclJSjZKIqvD0sOcFRXObtg6z?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: marvell.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BY3PR18MB4737.namprd18.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 4ad34393-9889-4fd8-1b5f-08db42304dbc
X-MS-Exchange-CrossTenant-originalarrivaltime: 21 Apr 2023 06:19:03.2555
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 70e1fb47-1155-421d-87fc-2e58f638b6e0
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 9L0dsjinLpUJlw6OW7fpAUtB71Edx3KDnKWU6xnrQ+PmPYBFhBVlYFpZbEh/SSyghuxuxeuQ5zcYFnXdAfJVGw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR18MB3040
X-Proofpoint-GUID: jnMFXYuKVpreljZQtcJl59pq3r8TATgh
X-Proofpoint-ORIG-GUID: jnMFXYuKVpreljZQtcJl59pq3r8TATgh
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.254,Aquarius:18.0.942,Hydra:6.0.573,FMLib:17.11.170.22
 definitions=2023-04-20_17,2023-04-20_01,2023-02-09_01
X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,RCVD_IN_DNSWL_LOW,SPF_HELO_NONE,SPF_NONE,
        T_SCC_BODY_TEXT_LINE autolearn=ham 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: Tejun Heo <htejun@gmail.com> On Behalf Of Tejun Heo
> Sent: Friday, April 21, 2023 8:21 AM
> To: jiangshanlai@gmail.com
> Cc: linux-kernel@vger.kernel.org; kernel-team@meta.com; Tejun Heo
> <tj@kernel.org>; Sunil Kovvuri Goutham <sgoutham@marvell.com>; David S.
> Miller <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub
> Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; linux-arm-
> kernel@lists.infradead.org; netdev@vger.kernel.org
> Subject: [EXT] [PATCH 06/22] net: thunderx: Use alloc_ordered_workqueue()=
 to
> create ordered workqueues
>=20
> External Email
>=20
> ----------------------------------------------------------------------
> BACKGROUND
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>=20
> When multiple work items are queued to a workqueue, their execution order
> doesn't match the queueing order. They may get executed in any order and
> simultaneously. When fully serialized execution - one by one in the queue=
ing
> order - is needed, an ordered workqueue should be used which can be creat=
ed
> with alloc_ordered_workqueue().
>=20
> However, alloc_ordered_workqueue() was a later addition. Before it, an or=
dered
> workqueue could be obtained by creating an UNBOUND workqueue with
> @max_active=3D=3D1. This originally was an implementation side-effect whi=
ch was
> broken by 4c16bd327c74 ("workqueue: restore WQ_UNBOUND/max_active=3D=3D1
> to be ordered"). Because there were users that depended on the ordered
> execution,
> 5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active=3D=3D1 to be
> ordered") made workqueue allocation path to implicitly promote UNBOUND
> workqueues w/
> @max_active=3D=3D1 to ordered workqueues.
>=20
> While this has worked okay, overloading the UNBOUND allocation interface =
this
> way creates other issues. It's difficult to tell whether a given workqueu=
e actually
> needs to be ordered and users that legitimately want a min concurrency le=
vel wq
> unexpectedly gets an ordered one instead. With planned UNBOUND workqueue
> updates to improve execution locality and more prevalence of chiplet desi=
gns
> which can benefit from such improvements, this isn't a state we wanna be =
in
> forever.
>=20
> This patch series audits all callsites that create an UNBOUND workqueue w=
/
> @max_active=3D=3D1 and converts them to alloc_ordered_workqueue() as
> necessary.
>=20
> WHAT TO LOOK FOR
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>=20
> The conversions are from
>=20
>   alloc_workqueue(WQ_UNBOUND | flags, 1, args..)
>=20
> to
>=20
>   alloc_ordered_workqueue(flags, args...)
>=20
> which don't cause any functional changes. If you know that fully ordered
> execution is not ncessary, please let me know. I'll drop the conversion a=
nd
> instead add a comment noting the fact to reduce confusion while conversio=
n is
> in progress.
>=20
> If you aren't fully sure, it's completely fine to let the conversion thro=
ugh. The
> behavior will stay exactly the same and we can always reconsider later.
>=20
> As there are follow-up workqueue core changes, I'd really appreciate if t=
he
> patch can be routed through the workqueue tree w/ your acks. Thanks.
>=20
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: Sunil Goutham <sgoutham@marvell.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Paolo Abeni <pabeni@redhat.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: netdev@vger.kernel.org
> ---
>  drivers/net/ethernet/cavium/thunder/thunder_bgx.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>=20
> diff --git a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
> b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
> index 7eb2ddbe9bad..a317feb8decb 100644
> --- a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
> +++ b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
> @@ -1126,8 +1126,7 @@ static int bgx_lmac_enable(struct bgx *bgx, u8
> lmacid)
>  	}
>=20
>  poll:
> -	lmac->check_link =3D alloc_workqueue("check_link", WQ_UNBOUND |
> -					   WQ_MEM_RECLAIM, 1);
> +	lmac->check_link =3D alloc_ordered_workqueue("check_link",
> +WQ_MEM_RECLAIM);
>  	if (!lmac->check_link)
>  		return -ENOMEM;
>  	INIT_DELAYED_WORK(&lmac->dwork, bgx_poll_for_link);
> --
> 2.40.0

Reviewed-by: Sunil Goutham <sgoutham@marvell.com>