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 62145A0597; Wed, 8 Apr 2020 19:30:08 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id BF21D1C115; Wed, 8 Apr 2020 19:30:07 +0200 (CEST) Received: from mail-lf1-f66.google.com (mail-lf1-f66.google.com [209.85.167.66]) by dpdk.org (Postfix) with ESMTP id D80351C06D for ; Wed, 8 Apr 2020 19:30:06 +0200 (CEST) Received: by mail-lf1-f66.google.com with SMTP id f8so5735596lfe.12 for ; Wed, 08 Apr 2020 10:30:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=semihalf-com.20150623.gappssmtp.com; s=20150623; h=subject:to:references:from:message-id:date:user-agent:mime-version :in-reply-to:content-language:content-transfer-encoding; bh=dhgNKUBZIWlcsCkZi85m6trCEyYxoNWiBY3G4SmACOA=; b=jPf0ETZ26mHISXb3QI13vFU6yMYha6bQM+0YOo/Dc1KigscTwn4w9yxay4Ka6NSv2v Bar5pYTxrjU2Ql4dlqzNY5ecIRM9MPX0tUd/VemBdMl62zfQfpl/Cp5s2zgopTOzbwZg hFe1FlxrNTPpM3HtP1EnP3wvCWTX3uYgKDoRkGX4tKN0UJizw4XmSWYJgW7Kv8k9bWAG 4JCHGofT3/uPI0jEPkdGK9FrzQclv8UJpw7QyyuVE7MCyq58XGpFR5xCckmMGzzcdrei c6C6Uu4ry1blswL5VB+NolzTonXDBwpqciZgHRPlKKLwRDkYXBxeMYbt69/ql6dpiDF/ GA9g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=dhgNKUBZIWlcsCkZi85m6trCEyYxoNWiBY3G4SmACOA=; b=JlQgdM+ZVixwdIsRSiyXlD/8eD5CZ5x+WUnjxr6kPxkl6W5sraY2cCg4x5bG9ayH+7 +R4EDDLzAqoGnrDL05T8AGk9vLKtuCV5kn654L72hQYtOeZr/0ygwkGTsdmqS74Uo2gG t1qSgSC4hFVH8X1PWEcf+7WbdRj6uUpuHWqH6BT31AU3CROUrR3U9vskV9kvPn4VYUle jJ709Tu5wLs8a1xIw0nI/Fam+zGZuKI9El4Y7S10KjGng4AdVeq5rCogGX4VZ4seem58 BiOBQ6UEuNTpVkAphgu8RG5N7kHx5ZHWGvvNI0/CU2i5j7SHCmJm8dfPJte5Z1krDv8a qNzg== X-Gm-Message-State: AGi0PuZs+h5B69I0vkPSTc6vXfV+LXYltqDvpcONlt322oiHwUikkPtD YjTGPzaz73zoqxyVDF0u+WLdMtIU3Dg= X-Google-Smtp-Source: APiQypL5IQkk83/fKtNblZ71Gxdaem/AWg4HJiD1ygAYMNtFFDsd1QPQHmE4xlFN0tBKfvMY7B6ccQ== X-Received: by 2002:ac2:4853:: with SMTP id 19mr5052939lfy.171.1586367005741; Wed, 08 Apr 2020 10:30:05 -0700 (PDT) Received: from [192.168.8.100] (user-5-173-33-152.play-internet.pl. [5.173.33.152]) by smtp.gmail.com with ESMTPSA id 137sm13609330lfa.48.2020.04.08.10.30.04 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 08 Apr 2020 10:30:05 -0700 (PDT) To: dev@dpdk.org References: <20200331192945.2466880-1-jerinj@marvell.com> <20200405085613.1336841-1-jerinj@marvell.com> <20200405085613.1336841-7-jerinj@marvell.com> From: Andrzej Ostruszka Message-ID: Date: Wed, 8 Apr 2020 19:30:04 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Firefox/68.0 Thunderbird/68.4.1 MIME-Version: 1.0 In-Reply-To: <20200405085613.1336841-7-jerinj@marvell.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v4 06/29] graph: populate fastpath memory for graph reel 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 4/5/20 10:55 AM, jerinj@marvell.com wrote: > From: Jerin Jacob [...] > diff --git a/lib/librte_graph/graph_populate.c b/lib/librte_graph/graph_populate.c > new file mode 100644 > index 000000000..093512efa > --- /dev/null > +++ b/lib/librte_graph/graph_populate.c > @@ -0,0 +1,234 @@ > +/* SPDX-License-Identifier: BSD-3-Clause > + * Copyright(C) 2020 Marvell International Ltd. > + */ > + > +#include > +#include > + > +#include > +#include > +#include > +#include > + > +#include "graph_private.h" > + > +static size_t > +graph_fp_mem_calc_size(struct graph *graph) > +{ > + struct graph_node *graph_node; > + rte_node_t val; > + size_t sz; > + > + /* Graph header */ > + sz = sizeof(struct rte_graph); > + /* Source nodes list */ > + sz += sizeof(rte_graph_off_t) * graph->src_node_count; > + /* Circular buffer for pending streams of size number of nodes */ > + val = rte_align32pow2(graph->node_count * sizeof(rte_graph_off_t)); > + sz = RTE_ALIGN(sz, val); > + graph->cir_start = sz; > + graph->cir_mask = rte_align32pow2(graph->node_count) - 1; > + sz += val; Aren't here source nodes counted twice? I'm trying now to wrap my head around how this all is structured and laid out in memory (thus the slowdown in review) so I am most probably missing something here. > + /* Fence */ > + sz += sizeof(RTE_GRAPH_FENCE); > + sz = RTE_ALIGN(sz, RTE_CACHE_LINE_SIZE); > + graph->nodes_start = sz; > + /* For 0..N node objects with fence */ > + STAILQ_FOREACH(graph_node, &graph->node_list, next) { > + sz = RTE_ALIGN(sz, RTE_CACHE_LINE_SIZE); > + sz += sizeof(struct rte_node); > + /* Pointer to next nodes(edges) */ > + sz += sizeof(struct rte_node *) * graph_node->node->nb_edges; > + } > + > + graph->mem_sz = sz; > + return sz; > +} > + > +static void > +graph_header_popluate(struct graph *_graph) > +{ > + struct rte_graph *graph = _graph->graph; > + > + graph->tail = 0; > + graph->head = (int32_t)-_graph->src_node_count; > + graph->cir_mask = _graph->cir_mask; > + graph->nb_nodes = _graph->node_count; > + graph->cir_start = RTE_PTR_ADD(graph, _graph->cir_start); > + graph->nodes_start = _graph->nodes_start; > + graph->socket = _graph->socket; > + graph->id = _graph->id; > + memcpy(graph->name, _graph->name, RTE_GRAPH_NAMESIZE); As I've mentioned above I'm learning the structure of the lib/memory so quick question here. My understanding is that rte_graph is a "view of the 'struct graph' sufficient for worker" so does it need both id & name? Both of them seems to be used in error or dump/debug paths. It probably doesn't matter (e.g. for performance) - just asking because 'id' seems to be used only in one place (where name could replace it probably). > + graph->fence = RTE_GRAPH_FENCE; > +} > + > +static void > +graph_nodes_populate(struct graph *_graph) > +{ > + rte_graph_off_t off = _graph->nodes_start; > + struct rte_graph *graph = _graph->graph; > + struct graph_node *graph_node; > + rte_edge_t count, nb_edges; > + const char *parent; > + rte_node_t pid; > + > + STAILQ_FOREACH(graph_node, &_graph->node_list, next) { > + struct rte_node *node = RTE_PTR_ADD(graph, off); > + memset(node, 0, sizeof(*node)); > + node->fence = RTE_GRAPH_FENCE; > + node->off = off; > + node->process = graph_node->node->process; > + memcpy(node->name, graph_node->node->name, RTE_GRAPH_NAMESIZE); > + pid = graph_node->node->parent_id; > + if (pid != RTE_NODE_ID_INVALID) { /* Cloned node */ > + parent = rte_node_id_to_name(pid); > + memcpy(node->parent, parent, RTE_GRAPH_NAMESIZE); > + } > + node->id = graph_node->node->id; > + node->parent_id = pid; > + nb_edges = graph_node->node->nb_edges; > + node->nb_edges = nb_edges; > + off += sizeof(struct rte_node); > + /* Copy the name in first pass to replace with rte_node* later*/ > + for (count = 0; count < nb_edges; count++) > + node->nodes[count] = (struct rte_node *)&graph_node > + ->adjacency_list[count] > + ->node->name[0]; I'm not sure I understand what is going here. Please see below ... > + > + off += sizeof(struct rte_node *) * nb_edges; > + off = RTE_ALIGN(off, RTE_CACHE_LINE_SIZE); > + node->next = off; > + __rte_node_stream_alloc(graph, node); > + } > +} [...] > +static int > +graph_node_nexts_populate(struct graph *_graph) > +{ > + rte_node_t count, val; > + rte_graph_off_t off; > + struct rte_node *node; > + const struct rte_graph *graph = _graph->graph; > + const char *name; > + > + rte_graph_foreach_node(count, off, graph, node) { > + for (val = 0; val < node->nb_edges; val++) { > + name = (const char *)node->nodes[val]; > + node->nodes[val] = graph_node_name_to_ptr(graph, name); ... Is it so that during node the first loop above some node might refer (by name) to other node that is not yet "registered" so instead of storing rte_node pointer you stored actually pointer to name which you now update to proper rte_node? > + if (node->nodes[val] == NULL) > + SET_ERR_JMP(EINVAL, fail, "%s not found", name); > + } > + } > + > + return 0; > +fail: > + return -rte_errno; > +} [...] With regards Andrzej Ostruszka