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 23170A0540; Sun, 19 Jul 2020 13:13:15 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id E397B1C0B6; Sun, 19 Jul 2020 13:13:14 +0200 (CEST) Received: from out5-smtp.messagingengine.com (out5-smtp.messagingengine.com [66.111.4.29]) by dpdk.org (Postfix) with ESMTP id C1F301C02C; Sun, 19 Jul 2020 13:13:13 +0200 (CEST) Received: from compute7.internal (compute7.nyi.internal [10.202.2.47]) by mailout.nyi.internal (Postfix) with ESMTP id 02FE55C010A; Sun, 19 Jul 2020 07:13:13 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute7.internal (MEProxy); Sun, 19 Jul 2020 07:13:13 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=fm1; bh= 1i0vTR+u5XrgZ6j30TOEobw4aeY5Bhia9B095+KFMQ0=; b=qe0ZRMmGfIUdupKa V+ORN/y/v6k61m9eheD5hciO+ycdobwaZKyRIWTTm6SuY0lUoOKhmnZEC1KX053/ jVRDffqxefb5odsYfXVviQ4RO1+PaOhjlzT15MNS3sOHHAEWNr0BZmOcZWd1IeDj C639xxIUcHJom1J9yWfiZ5IIZurzCQ+uUzmSFfIZYClmty1VxszvJyNHitF4K/dE 0U+QLJwIHmT7hnVh8+R6BELRsTUlyFYAH4qFwV8A7Gvnj0wIkRX2fLkXIwZK38jg JiyBQMe3KRVsiRvTr1bpGECn5yOp4kGTOS504XqPIhh3Ywo2q/blofIvLwq8k250 Q7OvkQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm3; bh=1i0vTR+u5XrgZ6j30TOEobw4aeY5Bhia9B095+KFM Q0=; b=LpsrXa/k/gLKDXWX62u5dKPjCMFReusirwffycLh5Iw5xoz/Ow3sKdMgQ uSqdUAMquLdFHBqp5tX8P19X+9ARrXkqkAAoa9dXWYQCPqNT+KerAo8hqhnnm21/ P0077pDLCMUb7rz1vMfe0mcvedqvLHZHZGT62ye6u7oUgZ1PK9kLDLQoGYngIPZX WcQmIMW56Nq0yoLhLnHZkmWPn1sZ5c7biqqccazLGHTZxdA64iymQTMd8ttzdjz6 cll4WO0iNBbNFugkClafeICpsazxHgzS6dbsJd6uRnu4lKGL/tLBHG6wL83hGSzO qL8Jpmp009s+ZytkEWxZ0QQ87I8Zw== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduiedrgedugdefjecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefhvffufffkjghfggfgtgesthfuredttddtvdenucfhrhhomhepvfhhohhmrghs ucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenucggtf frrghtthgvrhhnpedugefgvdefudfftdefgeelgffhueekgfffhfeujedtteeutdejueei iedvffegheenucfkphepleefrdeirddugeelrdduudegnecuvehluhhsthgvrhfuihiivg eptdenucfrrghrrghmpehmrghilhhfrhhomhepthhhohhmrghssehmohhnjhgrlhhonhdr nhgvth X-ME-Proxy: Received: from xps.localnet (114.149.6.93.rev.sfr.net [93.6.149.114]) by mail.messagingengine.com (Postfix) with ESMTPA id C173E3280063; Sun, 19 Jul 2020 07:13:10 -0400 (EDT) From: Thomas Monjalon To: Matan Azrad Cc: "dev@dpdk.org" , Raslan Darawsheh , Shiri Kuzin , "stable@dpdk.org" , Shahaf Shuler , Slava Ovsiienko , Ray Kinsella , Neil Horman Date: Sun, 19 Jul 2020 13:13:09 +0200 Message-ID: <4865593.5jmsT3qBYc@thomas> In-Reply-To: References: <20200719100735.2473014-1-thomas@monjalon.net> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" 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" 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? I depends on linker options. > > It is fixed by exporting a function, which is cleaner than a variable. > > Can you explain why? > We have classic example - rte_eth_devices. There is more control and more abstraction in functions, it can provide futre-proof abstraction. We should not export variables at all, it is a basic rule of writing API. Having a bad example in ethdev doesn't mean we should follow 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. Constructor must remain minimal. If constructor can be avoided, it must be. This is a golden rule. > > Note: the priority of the constructor was probably irrelevant. No comment about the constructor priority which was set as LOG for no good reason, proving that this code was not well reviewed? > > 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