From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 9672843B57; Tue, 20 Feb 2024 19:32:00 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 66FA7402A7; Tue, 20 Feb 2024 19:32:00 +0100 (CET) Received: from NAM02-SN1-obe.outbound.protection.outlook.com (mail-sn1nam02on2064.outbound.protection.outlook.com [40.107.96.64]) by mails.dpdk.org (Postfix) with ESMTP id 3CB4A40289 for ; Tue, 20 Feb 2024 19:31:58 +0100 (CET) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=gcRS1dEvBdJWrB/d2tehmDMba7PCt6dgSRyhxa0jUP54CQIoONZ2cUWQFv+WGUfjk6EwZRkU3GheEi2f9PoZ+ynlBKwThqmQ1v3uJBcwDynwzZa2+W6GjvPhzmbtZHC35gcLI9bqoz8weOrXmh82CPLobn+xWgLPSRr+688kuLgxuRcMZsb99QjYMbMDEPOTxLaw4pwNsK32GLJXP/jQaXWLUZlkxXryDEsK7nK7YVgZuBK+bn7nafLJqoH2UmZX6VrSQgnsqIPABYOpZz+0pGfSDKledWJuwi1iZ4XH/Ham3xK6Hhx1eeeqEuizAxsT+uRq2qtfZA3sPbnTZc2NkQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=pEQn+OtezqZIK5oHagjSxmKCcZp/H1b2DZAbP2nALgI=; b=dMUCu+mZzy6prBj69L70nzre9aIEsA6VL9kbwnlmivcLrw8JDIzYCXR6kOJMs4GWguB7XkUPtH+x2BI2NKpxwAyTe66PfprbWGL1EmUfwrXF562EIpTWCptHAceOqbWuMHZqWfUpqazpIarw98KDTjaKFSvxSAo3GwBc8aF84k/qxNviexxIOpdaLG3ODJwJj27LPyqS//V/ag7dBA07mjH0DMxCv73vb+o8iy41v0ZZ1l05aZzX1vOQn6FpjzLSOeLYT+pmPtm9MeZO0fP+Wu/uFcehwloqD2UYiS0iESOw+30BOAuerK7XP2OUgg1yvCTiVhNupsBipI42i6+laA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=amd.com; dmarc=pass action=none header.from=amd.com; dkim=pass header.d=amd.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amd.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=pEQn+OtezqZIK5oHagjSxmKCcZp/H1b2DZAbP2nALgI=; b=Qz4llNnFVfL0oJ+iT5Er0pyrPAmD7IHCY8w4CIYBgNJdFyBtHHZDsfvsuv8VSMLRi3Q78atBpIGgSKYLEWebX+MUtBISeTtYiXGPRWTypjiIVRZ3aKRiR/9OjDHypgrPiUopd4cePBQpU/0GHvxl3HiaJY1kikw1ytmz+Z1QXmk= Received: from MW6PR12MB8999.namprd12.prod.outlook.com (2603:10b6:303:247::8) by CYYPR12MB8653.namprd12.prod.outlook.com (2603:10b6:930:c5::9) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7316.21; Tue, 20 Feb 2024 18:31:56 +0000 Received: from MW6PR12MB8999.namprd12.prod.outlook.com ([fe80::877d:a085:d9bc:2f7d]) by MW6PR12MB8999.namprd12.prod.outlook.com ([fe80::877d:a085:d9bc:2f7d%7]) with mapi id 15.20.7316.018; Tue, 20 Feb 2024 18:31:56 +0000 From: "Boyer, Andrew" To: Wathsala Wathawana Vithanage CC: "Boyer, Andrew" , "dev@dpdk.org" , "Patel, Neel" , nd , Honnappa Nagarahalli Subject: Re: [PATCH 2/3] net/ionic: remove duplicate barriers Thread-Topic: [PATCH 2/3] net/ionic: remove duplicate barriers Thread-Index: AQHaYPqtXZHuVgPwdkGqgeQmFui6VbESQKIAgAA/BgCAARR/gA== Date: Tue, 20 Feb 2024 18:31:56 +0000 Message-ID: <76FA87FC-B542-40C8-8F03-350B3A1175B9@amd.com> References: <20240216170704.55523-1-andrew.boyer@amd.com> <20240216170704.55523-3-andrew.boyer@amd.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=amd.com; x-ms-publictraffictype: Email x-ms-traffictypediagnostic: MW6PR12MB8999:EE_|CYYPR12MB8653:EE_ x-ms-office365-filtering-correlation-id: 17cbe956-be49-42e1-195b-08dc32423794 x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: 9FhqxJ9VznHJxLp0OiECv68qRXUNjPtcOpAPCf/CoDZQCnIq3Bm3xdNE1FXafrtQyPZg/pBlEZAjImYdRP+jfFjWxNIP7zVFRyQU5RSxtckZZFWllkeC6lXTZX1ariIyN9O4sAjAWmGVmDvh132Fqmph2jk+HXdKM2VvBg90Rng7PrdkQcfCGtJtDgYeR4y1nu9petU0VyQ3JtTU1lSdOInPQbjf6bGTOuTo141RgiadGzrOpCXTHBYrojZ+zDINO0O6lymGUUoeLkKpEjX1GIARHTnzOxvYFjSlDkC/E1AoqE3Cht+2qvJGBs2XdHwX1BUDs+xCulGDOsoxtevIMHmQzh2cb0YzvY0031hRUEnDj5haygk6Z3H8zyTDOz9S31VoqL6F29bbvS05uDu12nfDzitNPydM9tn8tkkyTmojIEO20QZcaE3S7GS3sVHi5fbK9GvGaeQLz7DE1pC70jSA8BDlhJKOKDZIPHQrDYsnwBGpDmv9gCXVxsxMXE6nTNHB+dmF+fU2J4QVnrB9KEJYgrAiWkhzuUnW3im13jZT+Qotu0by796N8ID3imdredQ75LmtjhuGVOALYnR31PL7sct9N7Rj6580lT4d0LTr9sOw92dpNJLV4jdggO3f x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:MW6PR12MB8999.namprd12.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230031)(230273577357003)(38070700009); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?RZqLLeSklYQhNH8XuZx8t4pL3lBVoJe5UnMgk/JoPUcw6ERKpEZ1bi+EJHJU?= =?us-ascii?Q?/davJ0CAZ9n8FroXzNjMlokwNPu6prgiqqghPuVIRo14OMFqcSOmnamj89hI?= =?us-ascii?Q?C4ueKCU7Fk8QAjgnprx6+gVOEZ8r7YlEO5EnJZtnZfMg9SHYM9YU2/yGk9+w?= =?us-ascii?Q?7lI7QCBzwcsy/LPvT8PJJrCnGWvQ5Mug1KHr+nzfC3+GWDF0QBa/Iu28bxFB?= =?us-ascii?Q?njOhuDLLsGEEthKWjuyDzOuH3FHglRXgSPW8doFnadKvqbhNJY7YnSDgNUVq?= =?us-ascii?Q?vagNasuH4tHKjphFMwN5Mskht5TKOYLzgDZXmGQzv5VtkUtv5B6h9dTEr5lQ?= =?us-ascii?Q?W5r0Ui+U89E+XPclQf3N70LXDIQ/IYop4+OfUp3lKTaqEGnzQfC9Tq9/NQKW?= =?us-ascii?Q?VeUaxxkBYkREi2znTxrdRBrQeGoJtV0nv+lryGBiTK6vzDoi+OXldwTQOROa?= =?us-ascii?Q?yaMVSB4D8IqHV6/i3pCGsz/hA/Cab3KXxxd+hFtdkyLwwpZgZiHXeNvHKbRd?= =?us-ascii?Q?PUPx+cHTmrNtP0/hlSpBXgOc+om3tmKTJipIAMa3QuymLqYhVzRU+wbjj/LH?= =?us-ascii?Q?T02XADPgEV5mvQ8UIGb+A/6dVH+OXw+wJFsD7Vbg/wVSeApiChcatUPxhWlA?= =?us-ascii?Q?wRBGhsHWlf4lObhMKDc9J7cXWFBLPtqtB8yG2KnTjVqbpYuK3JpmpH6qMEL7?= =?us-ascii?Q?hR3O0vzAqn8ID1wwiZZAHjXjvgPPEbJEHYb85dGySYNkwjqFfCiJzD+D6FB9?= =?us-ascii?Q?3Ch/g/tF4K4qQsSEjH1fW+Fy6n9hOnk7MJLPJPOt0/D8bglCKNd//WsngxxS?= =?us-ascii?Q?Ez5Zf6YS4n4hXPAESDotfcrjYsqVS2BbmPR9nx/Ie4Av2+YpsjHZdEqULZyP?= =?us-ascii?Q?HY5H2t5NjBVDaTWN3s+PGygD/DLmZ5sq7AOqNEoiKpnsdah5fBOULc7hcpd3?= =?us-ascii?Q?HhOysVQxteFsPJg58d5kpxwhzdh6e+ePdBUrjHz0LA6MI8meI2N7okrtt/i5?= =?us-ascii?Q?iQLe2N6nmbcHQB4c6rjIzFhm5wExTmmHzFa4DMI1QvM25QQIt8yH+EWSNaYB?= =?us-ascii?Q?raMgic57Dnuj971ijl6XKPu2XARP06KgZ3GA4UZfV1S6xwyuilm3V5M1io9u?= =?us-ascii?Q?YptVMDbLYoOElU3htyVK8WggOBAiuWIfJAK+9H+ansc1tSaGjaWqJwO6INIn?= =?us-ascii?Q?sUEx9AksZ+kRJdglzoDbiauC1bJ+0+nJ9W0iLvTc6D0ASLPrIMLZAaCA8/1t?= =?us-ascii?Q?F2haB3sdiZejsB5aFDR4kGL8eebC4mezFfDc7zyYMxsT9j5HukKI5xstbBpe?= =?us-ascii?Q?cuFNWxM/R8BP4qzm2qNAU8nONUbVcnzhcXkwPANB0I1jc1a66HcSGFnpO1o4?= =?us-ascii?Q?aD82/x9W1cY8ou0TYJeQfwdzSqFVZhSfNW0Wzgm8dFsoQDaqcBJNMboBUAHm?= =?us-ascii?Q?sj9wuZJl0ZCGCfE69Ys9+sn2BAYDMwwHVhnMcqvqLnDGgUx9nBJQy8IJsK/7?= =?us-ascii?Q?/0OuEVWnJK7XTNa1ssCa/lx4gLmRl1naguS22qisaDsYqGyC1FSzz2HRfeEz?= =?us-ascii?Q?G79szdUx8PXNP55jzaBOVvRh82yxzJ04ghCpKf2h?= Content-Type: multipart/alternative; boundary="_000_76FA87FCB54240C88F03350B3A1175B9amdcom_" MIME-Version: 1.0 X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: MW6PR12MB8999.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 17cbe956-be49-42e1-195b-08dc32423794 X-MS-Exchange-CrossTenant-originalarrivaltime: 20 Feb 2024 18:31:56.0932 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: PJXU18dKgEu0VOrc1J5IJJ1T7NrX2HFrs0zTFW8++x7mgFgL2nIHmS2hHk9TqDmG X-MS-Exchange-Transport-CrossTenantHeadersStamped: CYYPR12MB8653 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org --_000_76FA87FCB54240C88F03350B3A1175B9amdcom_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable On Feb 19, 2024, at 9:02 PM, Wathsala Wathawana Vithanage wrote: Caution: This message originated from an External Source. Use proper cautio= n when opening attachments, clicking links, or responding. -----Original Message----- From: Wathsala Wathawana Vithanage > Sent: Monday, February 19, 2024 4:17 PM To: Andrew Boyer >; dev@d= pdk.org Cc: Neel Patel >; nd >; Honnappa Nagarahalli >; nd > Subject: RE: [PATCH 2/3] net/ionic: remove duplicate barriers Hi Andrew, I think that this barrier may have been added to ensure any writes to q- hw_index and q->head_idx complete before ionic_q_flush computes val. Dependency chains can also prevent reordering if that's the case this barri= er is not required. However, I have the following concern. diff --git a/drivers/net/ionic/ionic_main.c b/drivers/net/ionic/ionic_main.c index 1f24f64a33..814bb3b8f4 100644 --- a/drivers/net/ionic/ionic_main.c +++ b/drivers/net/ionic/ionic_main.c @@ -223,7 +223,6 @@ ionic_adminq_post(struct ionic_lif *lif, struct ionic_admin_ctx *ctx) q->head_idx =3D Q_NEXT_TO_POST(q, 1); /* Ring doorbell */ - rte_wmb(); ionic_q_flush(q); err_out: Ionic_q_flush(q) uses q->hw_index and q->head_idx to compute the value of val which it writes to the address stored in q->db. I can see that q->head_= idx is updated before the removed rte_wmb(), therefore it's safe to assume that " = q- head_idx =3D Q_NEXT_TO_POST(q, 1)" and "val =3D IONIC_DBELL_QID(q- hw_index) | q->head_idx" will maintain the program order due to that dependency. But I don't know if there exists a dependency chain over q- hw_index. If not, then that may have been the motive behind this barrier. Since q->hw_index is also updated in the same CPU ionic_q_flush will always see the correct value, consequently val should be always correct. It's safe to remove this barrier. Thank you for the careful review. We agree with this analysis. -Andrew --_000_76FA87FCB54240C88F03350B3A1175B9amdcom_ Content-Type: text/html; charset="us-ascii" Content-ID: <476E6B39C1BAF445916EDDBCD739D045@namprd12.prod.outlook.com> Content-Transfer-Encoding: quoted-printable

On Feb 19, 2024, at 9:02 PM, Wathsala Wathawana Vithanage <wathsala= .vithanage@arm.com> wrote:

Ca= ution: This message originated from an External Source. Use proper caution when o= pening attachments, clicking links, or responding.


-----Original Message-----
From: Wathsala Wathawana Vithanage <wathsala.vithanage@arm.com>
Sent: Monday, February 19, 2024 4:17 PM
To: Andrew Boyer <andrew.boyer@a= md.com>; dev@dpdk.org
Cc: Neel Patel <neel.patel@amd.com= >; nd <nd@arm.com>; Honnappa=
Nagarahalli <Honnappa.Na= garahalli@arm.com>; nd <nd@arm.com<= /a>>
Subject: RE: [PATCH 2/3] net/ionic: remove duplicate barriers

Hi Andrew,

I think that this barrier may have been added to ensure any writes to q-
hw_index and q->head_idx complete before ionic= _q_flush computes val.
Dependency chains can also prevent reordering if that's the case this barri= er is
not required.
However, I have the following concern.

diff --git a/drivers/net/ionic/ionic_main.c
b/drivers/net/ionic/ionic_main.c index 1f24f64a33..814bb3b8f4 100644
--- a/drivers/net/ionic/ionic_main.c
+++ b/drivers/net/ionic/ionic_main.c
@@ -223,7 +223,6 @@ ionic_adminq_post(struct ionic_lif *lif, struct
ionic_admin_ctx *ctx)
   q->head_idx =3D Q_NEXT_TO_POST(q, 1);

   /* Ring doorbell */
-   rte_wmb();
   ionic_q_flush(q);

err_out:

Ionic_q_flush(q) uses q->hw_index and q->head_idx to compute the valu= e of
val which it writes to the address stored in q->db. I can see that q->= ;head_idx is
updated before the removed rte_wmb(), therefore it's safe to assume that &q= uot; q-
head_idx =3D Q_NEXT_TO_POST(q, 1)" and "= ;val =3D IONIC_DBELL_QID(q-
hw_index) | q->head_idx" will maintain the program order due to tha= t
dependency. But I don't know if there exists a dependency chain over q-
hw_index. If not, then that may have been the mot= ive behind this barrier.


Since q->hw_index is also updated in the same CPU ionic_q_flush will always see the correct value, consequently val should be always correct. It's safe to remove this barrier.


Thank you for the careful review. We agree with this analysis.

-Andrew

--_000_76FA87FCB54240C88F03350B3A1175B9amdcom_--