From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 0A589A0540; Sun, 19 Jul 2020 13:41:31 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 7A9DB1C07E; Sun, 19 Jul 2020 13:41:30 +0200 (CEST) Received: from EUR04-HE1-obe.outbound.protection.outlook.com (mail-eopbgr70072.outbound.protection.outlook.com [40.107.7.72]) by dpdk.org (Postfix) with ESMTP id 5018B1BEB4; Sun, 19 Jul 2020 13:41:29 +0200 (CEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=kBhaFbvGBdvXflNBWXeVRKc6aB97OL28Lj0fBsvq3OzPkkNXquKlWSA5zyK/6qw3uhULHeNyVCVz2KWXg1pGWlhHS4rwEVvuYllSaBwhgiIZoHS+e98OIOd8vPGxov6mnwFqt0OdeIph7UD4ax08+rjjA1+SBPj0Zm9tAwrYLgsNplZylxabXbn9MBlJ6WSRF4qAEV+ArkAFw/3JtJGBFo7DhpOwn9c46YSddRqKnu0xMOIOlpCywI8hudnhihL09JDBicAO2S6gXK7FbPOxjFXtO1PvEr62EC8o80qRz4qbn872y2BuUW8xU2Ho8NbwiKF5kdOZZrVyvUYG3/pbKQ== 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-SenderADCheck; bh=v2+4SFvmK213o6sZDuixgW1k9KdZBHLEiOEmk3Lm3PA=; b=gwsPw808sDleKq1WiWC7D8tjh3IYxXFjRk2eCcKNS6zmTBKUkpKNwzszNRrO3QZ+CX2+1bueeEIUTm5QMele9xE8nLkLn7HdrdjJ7BvlBl88e+Crfm4XJJnpkPfpDNr/szIL+1QMnN7u7WG3izj1oOkqVkOv7e38F2YV1IVHBEfSm21tkqKVSO3qLg2talUXXeldCH0mZrcJ+9bENClZYi0VGHPiLUqQBrBNGaUPiQrovgYClrMu6qKeN57gSCjK/aasLVluqbD7JZKVdVYs96kPvH20HK+qU/jhG1AUcsLhIqAIIkNba1o/pUwBUCp+tV1OgWfjMAF/2rml8YsNow== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=mellanox.com; dmarc=pass action=none header.from=mellanox.com; dkim=pass header.d=mellanox.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Mellanox.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=v2+4SFvmK213o6sZDuixgW1k9KdZBHLEiOEmk3Lm3PA=; b=SEnsUjfHljXJ7MNpsfhctbSbVuFO/WvEFELUrZDAwhpUT+nScFqGpSk79MpJD9MWGVks+OaFMBUkkNnlR0SOkL0YFNvuXH/4C84myCxBcwYkIBkNjZGIbSRuZVsz/EfRDwtm6AVQvBqRRx72zF6qfCHijbL3YAytCa2t/ROS/54= Received: from AM0PR0502MB4019.eurprd05.prod.outlook.com (2603:10a6:208:f::11) by AM0PR05MB4851.eurprd05.prod.outlook.com (2603:10a6:208:c1::30) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3195.17; Sun, 19 Jul 2020 11:41:27 +0000 Received: from AM0PR0502MB4019.eurprd05.prod.outlook.com ([fe80::c010:7069:920a:3663]) by AM0PR0502MB4019.eurprd05.prod.outlook.com ([fe80::c010:7069:920a:3663%6]) with mapi id 15.20.3195.025; Sun, 19 Jul 2020 11:41:27 +0000 From: Matan Azrad To: Thomas Monjalon CC: "dev@dpdk.org" , Raslan Darawsheh , Shiri Kuzin , "stable@dpdk.org" , Shahaf Shuler , Slava Ovsiienko , Ray Kinsella , Neil Horman Thread-Topic: [PATCH] common/mlx5: fix CPU detection for PCI relaxed ordering Thread-Index: AQHWXbS7Xt/9LATOx0a0pYP/MvbRPakOuSNggAAGnoCAAAJrAA== Date: Sun, 19 Jul 2020 11:41:27 +0000 Message-ID: References: <20200719100735.2473014-1-thomas@monjalon.net> <4865593.5jmsT3qBYc@thomas> In-Reply-To: <4865593.5jmsT3qBYc@thomas> Accept-Language: en-US, he-IL Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: monjalon.net; dkim=none (message not signed) header.d=none;monjalon.net; dmarc=none action=none header.from=mellanox.com; x-originating-ip: [87.70.223.40] x-ms-publictraffictype: Email x-ms-office365-filtering-ht: Tenant x-ms-office365-filtering-correlation-id: 89bec579-4d44-420f-b9ac-08d82bd8ac6d x-ms-traffictypediagnostic: AM0PR05MB4851: x-ld-processed: a652971c-7d2e-4d9b-a6a4-d149256f461b,ExtFwd,ExtAddr x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:9508; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: a4TDrrTdG84vVr72TjIgmdKcfODc/jbk4U4y46EBOkR16Fw+P21oFl9F+7sYASljh2pX5sOg+AaA4Uh2z6rQaPEiaOK/Gg++YiofZ1KfoiZ4CSqO8689Bj5sHoELXIQxzTkaG4TlqBgt4mk28UjAWSB1u9umwNUJW7f6ccmVGktLi20SokiNpN5lcRULcQzkj/HT5flCAZ9KBrv5BWn1CmGmInxPJpVDnfKsnjBko0Yjj2ZO+evy3Uh93M3FBgv0VIdoCnBNi3jGw+tvNMD4vhM1iChcHVUR2+5/9ngrGmNDN39KKH2d2iORjzG1tNqZmv0vWFzjtix2aGg6EfKliw== x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:AM0PR0502MB4019.eurprd05.prod.outlook.com; PTR:; CAT:NONE; SFTY:; SFS:(4636009)(39860400002)(366004)(376002)(396003)(136003)(346002)(5660300002)(7696005)(33656002)(478600001)(8676002)(55016002)(8936002)(26005)(6506007)(76116006)(186003)(54906003)(66446008)(9686003)(71200400001)(4326008)(86362001)(6916009)(2906002)(64756008)(66556008)(66476007)(66946007)(52536014)(316002)(83380400001); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata: IiZY0c8kDFQSt0lUjKUakAX9meepnH5dUfkpwxZkRskFwI7EgLvkUHgX1KZmbf32cFtlogHuY1krORspglnw05qQUW2LGmMgD8jqT3JT3YDPEtINslilQWwvrmlOGUO6HItfcHNTBbtwsg0e8BXyN0/kPJhpiKjC4iUyVkg/nHNtRPLZT8Vu1buPlWurPkyxMe0EveSvZvtKoSHXb+k4jEdHVdkfmYX/GrL3htSO0vlEvy2uASb/Yat2hbDq6Y/lcpgrJlUfNLWDiiB5PdXwxta5eLdSILDZmM2WHfUpvyiDv+qTZviJ2diPzZeuLKQNczFca2tQe8UOZP15iy7cmsUgeKIghxEsj2nX/GE0CYRRrwiSdToUW9A2jG6LQhCzo/RRGsJxQVHvsU11BIWriakIn6N4UDno6wMiJfan1IuMlOGSF3BzxJMiLfP9DkJh7Zw7DKUIGpsvbH4MQfTGIhuReBicNsoiDKQJVsbuEeo= Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: AM0PR0502MB4019.eurprd05.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 89bec579-4d44-420f-b9ac-08d82bd8ac6d X-MS-Exchange-CrossTenant-originalarrivaltime: 19 Jul 2020 11:41:27.6378 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: YPcPTLXyGztLTnaaj7peCJmeynv8w35ztcJSbShnWxZW6/Q6eCP+ef5z4KvBDgJZUCQPAniICEA+gD8LGQCxvA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM0PR05MB4851 Subject: Re: [dpdk-dev] [PATCH] common/mlx5: fix CPU detection for PCI relaxed ordering X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" From: Thomas Monjalon: > 19/07/2020 12:56, Matan Azrad: > > > > From: Thomas Monjalon > > > The detection of the CPU was done in a constructor and shared in a > > > global variable. > > > > > > This variable may not be visible in the net PMD because it was not > > > exported as part of the .map file. > > > > Can you explain exactly when it is not visible? >=20 > I depends on linker options. >=20 > > > It is fixed by exporting a function, which is cleaner than a variable= . > > > > Can you explain why? > > We have classic example - rte_eth_devices. >=20 > There is more control and more abstraction in functions, it can provide f= utre- > proof abstraction. Also variable have more abstraction - struct. In future, if it will be needed, we can change it. > We should not export variables at all, > it is a basic rule of writing API. It is variable which is depended only in the running CPU - almost like comp= ile time condition, so it is not regular case. I think it makes sense also to use a singleton variable as internal API. > Having a bad example in ethdev doesn't mean we should follow it. If ethdev rte_eth_devices is bad API, Are you going to change it? > > > By checking the CPU only at the first call of the function, doing > > > the check in a constructor becomes useless. > > > > Yes, but why not to do it in constructor? this variable is initialized = only once > and doesn't depend in any parameter. >=20 > Constructor must remain minimal. > If constructor can be avoided, it must be. > This is a golden rule. The cpu detection is a fast code. Using constructor here makes sense: 1. we need only one initialization for all the program. 2. no need to take care of multithreading on the single initialization (are= your code thread safe?). 3. no parameters are required. >=20 > > > Note: the priority of the constructor was probably irrelevant. >=20 > No comment about the constructor priority which was set as LOG for no goo= d > reason, proving that this code was not well reviewed? I guess you mean that comment is missing - you right. We want to be sure that the variable is ready before any usage of it in the= drivers (even in driver contractors). >=20 > > > At the same time, the comments are reworded or dropped if useless. > > > > > > Fixes: 4c204fe5e5d2 ("common/mlx5: disable relaxed ordering in > > > unsuitable > > > CPUs") > > > Cc: shirik@mellanox.com > > > Cc: stable@dpdk.org > > > > > > Signed-off-by: Thomas Monjalon >=20 >=20