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 930E1A04A2; Tue, 5 Nov 2019 18:25:07 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 281421BEAD; Tue, 5 Nov 2019 18:25:07 +0100 (CET) Received: from mail-pl1-f193.google.com (mail-pl1-f193.google.com [209.85.214.193]) by dpdk.org (Postfix) with ESMTP id 22B7C5B32 for ; Tue, 5 Nov 2019 18:25:06 +0100 (CET) Received: by mail-pl1-f193.google.com with SMTP id j12so7837182plt.9 for ; Tue, 05 Nov 2019 09:25:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=Yf5GsEQJXZKIbY5VO1e5KtZJbni63FkQWs8LKBYYMho=; b=OmMIHJWit2baLCkpFeuFSC1BE0OuhhahtLFziuP0mLtpij7S+oSBMB5Qrd/lbz/pzL D064fWg+fPHMg93DhhLDxN5NCtNUOTlb2mrEkX53ZfMKhV7jAu+2XJbV8q9U5eCAVDEY knbdehv9XZgbIQ+2GImZar2aDhLElrXcx7Rf6JL48XYXNZLv7U2ow6UZ3QLGMQwyan38 tVFRM/tKN2IKu5C3XDO2blT9q3xcRmKbvpwi4/9PPfoAAuXkM9w3v/xbpraoKC5XFo6V 5MsB3l5rcMFQrtUCYrZllTLk2ajz7OPPdmK1bIQRYFYk5qiM/nAmAtJAuSSO2vfS0/bR S0vg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=Yf5GsEQJXZKIbY5VO1e5KtZJbni63FkQWs8LKBYYMho=; b=hcmLkNukQMFHgpClYqQug3wGyGNVg3o54RMTWqgpaT1Ho5BIsvHaFyPTgOy6D+PEn/ UWN9WDj6ff+qEv9xTvwKx9LipsJEdwsOuQjlSlLAhHrHz6NnHtId6+nBSm/K63WDen+z fVOki+6ayn1TFFqpC6pzqq8K1KrwGsvF+/fdZaqdYU6pwaR2FUoRGBIA9nYC5vyibJj3 XtBoKm16g8QMawD8O4/9wCmtptEBfMBTogz+5F3qWuSKvkuEhdGYFvYABNDAf+WGtdpb SR7ldRbSUtmzzPekeVjxhEGDpkwOS7bSJ4Q3MrcjGAhYT/ovEa7p5MBU93tLIWG8NG35 Wmiw== X-Gm-Message-State: APjAAAUf9IA9GTwOAccHL9cpqM3uinsfk5fN0b/4QK03BGKk1bwTHf+r p9J1QTdx+2Tglem6XaHbCSoP4Q== X-Google-Smtp-Source: APXvYqxZ/5nQRIbji9enthqHRUsaT87J+XZXXfgUwd3cNAucTjp9/KxDCHjWuIiQ9V+QOqi2qmRVOQ== X-Received: by 2002:a17:902:fe06:: with SMTP id g6mr31391818plj.159.1572974705227; Tue, 05 Nov 2019 09:25:05 -0800 (PST) Received: from hermes.lan (204-195-22-127.wavecable.com. [204.195.22.127]) by smtp.gmail.com with ESMTPSA id a145sm15525209pfa.7.2019.11.05.09.25.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 05 Nov 2019 09:25:04 -0800 (PST) Date: Tue, 5 Nov 2019 09:25:01 -0800 From: Stephen Hemminger To: Bing Zhao Cc: orika@mellanox.com, viacheslavo@mellanox.com, rasland@mellanox.com, dev@dpdk.org Message-ID: <20191105092501.04521668@hermes.lan> In-Reply-To: <1572967680-93338-1-git-send-email-bingz@mellanox.com> References: <1572967680-93338-1-git-send-email-bingz@mellanox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH] net/mlx5: introduce mlx5 hash list 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 Tue, 5 Nov 2019 17:28:00 +0200 Bing Zhao wrote: > +struct mlx5_hlist * > +mlx5_hlist_create(char *name, uint64_t size, mlx5_hlist_destroy_callback_fn cb) Name is unused in rte_malloc, so why bother with it here? > +{ > + struct mlx5_hlist *h; > + uint64_t act_size; > + uint64_t alloc_size; > + > + if (!size) > + return NULL; > + /* If the low 32B is power of 2, then the 64B integer is too. */ > + if (!rte_is_power_of_2((uint32_t)size)) { If you are doing cast, therefore you are truncating, therefore size can't really be a uint64_t and should be size_t or uint32_t. > + act_size = rte_align64pow2(size); > + DRV_LOG(WARNING, "Size 0x%" PRIX64 " is not power of 2, will " > + "be aligned to 0x%" PRIX64 ".\n", size, act_size); > + } else { > + act_size = size; > + } Power of 2 hash list size wastes significant amount of memory. > + alloc_size = sizeof(struct mlx5_hlist) + > + sizeof(struct mlx5_hlist_head) * act_size; > + /* Using zmalloc, then no need to initialize the heads. */ > + h = rte_zmalloc(name, alloc_size, RTE_CACHE_LINE_SIZE); Why not use rte_calloc? Why use rte_malloc at all? Does this need to be accessed from hardware or shared between primary/secondary. Also rte_malloc has less tooling support (coverity, leak checking, etc) than standard malloc. > + if (!h) { > + DRV_LOG(ERR, "No memory for hash list %s creation\n", > + name ? name : "None"); > + return NULL; > + } > + if (name) > + snprintf(h->name, MLX5_HLIST_NAMESIZE, "%s", name); > + if (!cb) > + DRV_LOG(WARNING, "No callback function is specified, will use " > + "rte_free when destroying hlist %p %s\n", > + (void *)h, h->name); > + else > + h->cb = cb; > + h->table_sz = act_size; > + h->mask = act_size - 1; > + DRV_LOG(DEBUG, "Hash list with %s size 0x%" PRIX64 ", callback " > + "0x%" PRIX64 "is created.\n", > + h->name, act_size, (uint64_t)(uintptr_t)cb); > + return h; > +}