From mboxrd@z Thu Jan 1 00:00:00 1970 From: Adrien Mazarguil Subject: Re: [PATCH v2 2/7] net/mlx5: remove redundant objects in probe code Date: Wed, 27 Jun 2018 15:30:51 +0200 Message-ID: <20180627133051.GR4025@6wind.com> References: <20180525161814.13873-1-adrien.mazarguil@6wind.com> <20180614083047.10812-1-adrien.mazarguil@6wind.com> <20180614083047.10812-3-adrien.mazarguil@6wind.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: "dev@dpdk.org" To: Shahaf Shuler Return-path: Received: from mail-wm0-f65.google.com (mail-wm0-f65.google.com [74.125.82.65]) by dpdk.org (Postfix) with ESMTP id D71A41B8C9 for ; Wed, 27 Jun 2018 15:31:08 +0200 (CEST) Received: by mail-wm0-f65.google.com with SMTP id z137-v6so5550449wmc.0 for ; Wed, 27 Jun 2018 06:31:08 -0700 (PDT) Content-Disposition: inline In-Reply-To: List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hey Shahaf, I couldn't reply earlier, sorry for that. See below. On Sun, Jun 17, 2018 at 10:14:01AM +0000, Shahaf Shuler wrote: > Hi Adrien, > > Small nit, > > Thursday, June 14, 2018 11:35 AM, Adrien Mazarguil: > > Subject: [PATCH v2 2/7] net/mlx5: remove redundant objects in probe code > > > > This patch gets rid of redundant calls to open the device and query its > > attributes in order to simplify the code. > > > > Signed-off-by: Adrien Mazarguil > > -- > > v2 changes: > > > > - Minor indent fix on existing code. > > --- > > drivers/net/mlx5/mlx5.c | 64 +++++++++++++++++++++----------------------- > > 1 file changed, 30 insertions(+), 34 deletions(-) > > > > diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c index > > @@ -907,7 +904,8 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv > > __rte_unused, > > continue; > > } > > DRV_LOG(DEBUG, "using port %u", port); > > - ctx = mlx5_glue->open_device(ibv_dev); > > + if (!ctx) > > Is it really possible for ctx to be NULL on this stage? > Maybe assert is preferable? See below, ctx is only inherited (non-NULL) during the first iteration. It is reset and reopened for each instance since they need their own dedicated Verbs context. In any case, this patch focuses on removing redundant calls in preparation for subsequent patches in the series. This code disappears entirely later. > > + /* > > + * Each eth_dev instance is assigned its own Verbs context, > > + * since this one is consumed, let the next iteration open > > + * another. > > + */ > > + ctx = NULL; > > continue; No problem if I leave it that way? -- Adrien Mazarguil 6WIND