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 DF172A0527; Fri, 24 Jul 2020 16:54:05 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id D2B2B1C032; Fri, 24 Jul 2020 16:54:03 +0200 (CEST) Received: from us-smtp-delivery-1.mimecast.com (us-smtp-1.mimecast.com [207.211.31.81]) by dpdk.org (Postfix) with ESMTP id 150F41C025 for ; Fri, 24 Jul 2020 16:54:01 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1595602441; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=yhaC+XX0m68DeIkFB/O40J+FU9NOECWN2VPUVU+H35Q=; b=HWDWYOyH78alA3ybJKG23vtpHallmUG5DJeeDwzvJpHzQHNpZe7GZgaYHo91r26ROrEUL/ nivXStTSS9s8O4IoGxMc8bDOfR/58FuK4tpIyBqfnyfJa7J8aF+Mv7i4pHsKq372FyOgZs gU0ZIHF+f2L+a5u5Hclt8kKkRv0l1KM= Received: from mail-ua1-f69.google.com (mail-ua1-f69.google.com [209.85.222.69]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-321-jWRs2O0TPvKHtU8WUQhgcQ-1; Fri, 24 Jul 2020 10:53:59 -0400 X-MC-Unique: jWRs2O0TPvKHtU8WUQhgcQ-1 Received: by mail-ua1-f69.google.com with SMTP id h94so2749893uad.14 for ; Fri, 24 Jul 2020 07:53:59 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=yhaC+XX0m68DeIkFB/O40J+FU9NOECWN2VPUVU+H35Q=; b=iI4K8fnzY8OOQtXgTkkfaGPQcEmHdv77u0QSlwpX0X5+NDTWu8QsF3iNRf0jvlrx5h T2i1uXNFNxQh40Z7/op+bT/tpDrJJ00JQv1W43fIHyj2Tl201PucXQTpYM4terhLSe2N rR+KW64SkzcHTol0NjcnrKGxMLKarUzoilk/OO7DQvrFyyR7UfMUpcA+a1qw5mlBYT2I ENKjVxx5BGYl/buHBIgcFQd/oTeLob4Z9+1l2OkAALoRGRmosn9qPvfH/n5GjEkDGcWp JYcwj1dRByBrI9f9AUgSVv9t7YmDW8ibrWO8m56kBym2hx0qhFBFpBLWa8wgSuIoLFnX Axqw== X-Gm-Message-State: AOAM530IYXgwkBL2NbIM0kyyQgsRmoj2jVsk1t1Mf523M0FjceSm4Usp CyyG3j3sIF3Ii5Qrq4XooF89IOWMjjnUSTU0XvXOWWRHWqLM3WBZk1b9T+ZumzG2kPzsHVuv/EM AfBbFoRwDU6kd3ZmKa3Y= X-Received: by 2002:ab0:5a72:: with SMTP id m47mr8169853uad.86.1595602439136; Fri, 24 Jul 2020 07:53:59 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwkH5zny7NVJkrA+ROmWsfVzVYv5Mq1xeDDtUxnbideL+2Pz8ywgCE3wr3VdVkSp8dCTb/sWTSWNZuFBcOygAs= X-Received: by 2002:ab0:5a72:: with SMTP id m47mr8169824uad.86.1595602438790; Fri, 24 Jul 2020 07:53:58 -0700 (PDT) MIME-Version: 1.0 References: <20200719100735.2473014-1-thomas@monjalon.net> In-Reply-To: <20200719100735.2473014-1-thomas@monjalon.net> From: David Marchand Date: Fri, 24 Jul 2020 16:53:47 +0200 Message-ID: To: Thomas Monjalon Cc: dev , Raslan , shirik@mellanox.com, dpdk stable , Matan Azrad , Shahaf Shuler , Viacheslav Ovsiienko , Ray Kinsella , Neil Horman X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" 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" On Sun, Jul 19, 2020 at 12:09 PM Thomas Monjalon wrote: > > 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. > It is fixed by exporting a function, which is cleaner than a variable. > > By checking the CPU only at the first call of the function, > doing the check in a constructor becomes useless. > Note: the priority of the constructor was probably irrelevant. > > 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 > --- > drivers/common/mlx5/linux/mlx5_common_verbs.c | 2 +- > drivers/common/mlx5/mlx5_common.c | 53 ++++++++----------- > drivers/common/mlx5/mlx5_common.h | 4 +- > .../common/mlx5/rte_common_mlx5_version.map | 2 + > drivers/net/mlx5/mlx5_flow_dv.c | 2 +- > 5 files changed, 28 insertions(+), 35 deletions(-) > > diff --git a/drivers/common/mlx5/linux/mlx5_common_verbs.c b/drivers/common/mlx5/linux/mlx5_common_verbs.c > index a2fc7a36bd..31ac20fe09 100644 > --- a/drivers/common/mlx5/linux/mlx5_common_verbs.c > +++ b/drivers/common/mlx5/linux/mlx5_common_verbs.c > @@ -55,7 +55,7 @@ mlx5_common_verbs_reg_mr(void *pd, void *addr, size_t length, > memset(pmd_mr, 0, sizeof(*pmd_mr)); > ibv_mr = mlx5_glue->reg_mr(pd, addr, length, > IBV_ACCESS_LOCAL_WRITE | > - (haswell_broadwell_cpu ? 0 : > + (mlx5_cpu_is_haswell_broadwell() ? 0 : > IBV_ACCESS_RELAXED_ORDERING)); > if (!ibv_mr) > return -1; > diff --git a/drivers/common/mlx5/mlx5_common.c b/drivers/common/mlx5/mlx5_common.c > index 693e2c68c8..7232d5131d 100644 > --- a/drivers/common/mlx5/mlx5_common.c > +++ b/drivers/common/mlx5/mlx5_common.c > @@ -20,8 +20,6 @@ int mlx5_common_logtype; > const struct mlx5_glue *mlx5_glue; > #endif > > -uint8_t haswell_broadwell_cpu; > - > static int > mlx5_class_check_handler(__rte_unused const char *key, const char *value, > void *opaque) > @@ -59,19 +57,8 @@ mlx5_class_get(struct rte_devargs *devargs) > } > > > -/* In case this is an x86_64 intel processor to check if > - * we should use relaxed ordering. > - */ > #ifdef RTE_ARCH_X86_64 > -/** > - * This function returns processor identification and feature information > - * into the registers. > - * > - * @param eax, ebx, ecx, edx > - * Pointers to the registers that will hold cpu information. > - * @param level > - * The main category of information returned. > - */ > +/* Processor identification and feature information filled in registers. */ Nit, no need for inline. > static inline void mlx5_cpu_id(unsigned int level, > unsigned int *eax, unsigned int *ebx, > unsigned int *ecx, unsigned int *edx) > @@ -97,17 +84,7 @@ RTE_INIT_PRIO(mlx5_glue_init, CLASS) > mlx5_glue_constructor(); > } > > -/** > - * This function is responsible of initializing the variable > - * haswell_broadwell_cpu by checking if the cpu is intel > - * and reading the data returned from mlx5_cpu_id(). > - * since haswell and broadwell cpus don't have improved performance > - * when using relaxed ordering we want to check the cpu type before > - * before deciding whether to enable RO or not. > - * if the cpu is haswell or broadwell the variable will be set to 1 > - * otherwise it will be 0. > - */ > -RTE_INIT_PRIO(mlx5_is_haswell_broadwell_cpu, LOG) > +static bool mlx5_x86_is_haswell_broadwell(void) > { > #ifdef RTE_ARCH_X86_64 > unsigned int broadwell_models[4] = {0x3d, 0x47, 0x4F, 0x56}; > @@ -125,8 +102,7 @@ RTE_INIT_PRIO(mlx5_is_haswell_broadwell_cpu, LOG) > vendor = ebx; > max_level = eax; > if (max_level < 1) { > - haswell_broadwell_cpu = 0; > - return; > + return false; > } > mlx5_cpu_id(1, &eax, &ebx, &ecx, &edx); > model = (eax >> 4) & 0x0f; > @@ -140,18 +116,31 @@ RTE_INIT_PRIO(mlx5_is_haswell_broadwell_cpu, LOG) > if (brand_id == 0 && family == 0x6) { > for (i = 0; i < RTE_DIM(broadwell_models); i++) > if (model == broadwell_models[i]) { > - haswell_broadwell_cpu = 1; > - return; > + return true; > } > for (i = 0; i < RTE_DIM(haswell_models); i++) > if (model == haswell_models[i]) { > - haswell_broadwell_cpu = 1; > - return; > + return true; > } > } > } > #endif > - haswell_broadwell_cpu = 0; > + return false; > +} > + > +/* > + * Check if the CPU is Intel Haswell or Broadwell, > + * because PCI relaxed ordering has no performance benefit with these CPUs. > + */ > +bool mlx5_cpu_is_haswell_broadwell(void) > +{ > + static bool haswell_broadwell_cpu; > + static bool once = false; > + > + if (once) > + return haswell_broadwell_cpu; > + once = true; > + return haswell_broadwell_cpu = mlx5_x86_is_haswell_broadwell(); > } > > /** > diff --git a/drivers/common/mlx5/mlx5_common.h b/drivers/common/mlx5/mlx5_common.h > index 2851507058..d453e0b3d8 100644 > --- a/drivers/common/mlx5/mlx5_common.h > +++ b/drivers/common/mlx5/mlx5_common.h > @@ -243,6 +243,9 @@ struct mlx5_klm { > > LIST_HEAD(mlx5_dbr_page_list, mlx5_devx_dbr_page); > > +__rte_internal > +bool mlx5_cpu_is_haswell_broadwell(void); > + > __rte_internal > enum mlx5_class mlx5_class_get(struct rte_devargs *devargs); > __rte_internal > @@ -255,6 +258,5 @@ int64_t mlx5_get_dbr(void *ctx, struct mlx5_dbr_page_list *head, > __rte_internal > int32_t mlx5_release_dbr(struct mlx5_dbr_page_list *head, uint32_t umem_id, > uint64_t offset); > -extern uint8_t haswell_broadwell_cpu; > > #endif /* RTE_PMD_MLX5_COMMON_H_ */ > diff --git a/drivers/common/mlx5/rte_common_mlx5_version.map b/drivers/common/mlx5/rte_common_mlx5_version.map > index ae57ebdba5..501b9fff3b 100644 > --- a/drivers/common/mlx5/rte_common_mlx5_version.map > +++ b/drivers/common/mlx5/rte_common_mlx5_version.map > @@ -6,6 +6,8 @@ INTERNAL { > mlx5_common_verbs_reg_mr; > mlx5_common_verbs_dereg_mr; > > + mlx5_cpu_is_haswell_broadwell; > + > mlx5_create_mr_ext; > > mlx5_dev_to_pci_addr; > diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c > index 8b5b6838fa..f1109ae095 100644 > --- a/drivers/net/mlx5/mlx5_flow_dv.c > +++ b/drivers/net/mlx5/mlx5_flow_dv.c > @@ -4201,7 +4201,7 @@ flow_dv_create_counter_stat_mem_mng(struct rte_eth_dev *dev, int raws_n) > mkey_attr.klm_num = 0; > if (priv->config.hca_attr.relaxed_ordering_write && > priv->config.hca_attr.relaxed_ordering_read && > - !haswell_broadwell_cpu) > + !mlx5_cpu_is_haswell_broadwell()) > mkey_attr.relaxed_ordering = 1; > mem_mng->dm = mlx5_devx_cmd_mkey_create(sh->ctx, &mkey_attr); > if (!mem_mng->dm) { > -- > 2.27.0 > I ended up on this constructor hack while looking at the common code. This patch makes sense. Reviewed-by: David Marchand -- David Marchand