From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id A58F1A0096 for ; Tue, 4 Jun 2019 12:28:35 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 4B4381BBEB; Tue, 4 Jun 2019 12:28:34 +0200 (CEST) Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by dpdk.org (Postfix) with ESMTP id 0F7291BBD5 for ; Tue, 4 Jun 2019 12:28:32 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga106.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 04 Jun 2019 03:28:32 -0700 X-ExtLoop1: 1 Received: from aburakov-mobl1.ger.corp.intel.com (HELO [10.237.220.94]) ([10.237.220.94]) by orsmga003.jf.intel.com with ESMTP; 04 Jun 2019 03:28:30 -0700 To: =?UTF-8?Q?Micha=c5=82_Krawczyk?= , dev@dpdk.org Cc: Marcin Wojtas , Guy Tzalik , Evgeny Schemeilin , stephen@networkplumber.org, thomas@monjalon.net, david.marchand@redhat.com References: <5f6e26e27ad524f85ee9a911aeebae69f1ec0c1a.1559147228.git.anatoly.burakov@intel.com> <2f73f49d-e13b-ac1f-9e32-80b9d39b1166@semihalf.com> <4faeb0be-fe10-d866-1027-0f3ef351cd3a@semihalf.com> From: "Burakov, Anatoly" Message-ID: Date: Tue, 4 Jun 2019 11:28:29 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.7.0 MIME-Version: 1.0 In-Reply-To: <4faeb0be-fe10-d866-1027-0f3ef351cd3a@semihalf.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH 24/25] net/ena: fix direct access to shared memory config 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 03-Jun-19 2:36 PM, Michał Krawczyk wrote: > On 03.06.2019 09:33, Michał Krawczyk wrote: >> On 29.05.2019 18:31, Anatoly Burakov wrote: >>> The ENA driver calculates a ring's NUMA node affinity by directly >>> accessing the memzone list. Fix it to do it through the public >>> API's instead. >>> >>> Signed-off-by: Anatoly Burakov >>> --- >>>   drivers/net/ena/ena_ethdev.c | 18 +++--------------- >>>   1 file changed, 3 insertions(+), 15 deletions(-) >>> >>> diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c >>> index b6651fc0f..e745e9e92 100644 >>> --- a/drivers/net/ena/ena_ethdev.c >>> +++ b/drivers/net/ena/ena_ethdev.c >>> @@ -274,20 +274,6 @@ static const struct eth_dev_ops ena_dev_ops = { >>>   #define NUMA_NO_NODE    SOCKET_ID_ANY >>> -static inline int ena_cpu_to_node(int cpu) >>> -{ >>> -    struct rte_config *config = rte_eal_get_configuration(); >>> -    struct rte_fbarray *arr = &config->mem_config->memzones; >>> -    const struct rte_memzone *mz; >>> - >>> -    if (unlikely(cpu >= RTE_MAX_MEMZONE)) >>> -        return NUMA_NO_NODE; >>> - >>> -    mz = rte_fbarray_get(arr, cpu); >>> - >>> -    return mz->socket_id; >>> -} >>> - >>>   static inline void ena_rx_mbuf_prepare(struct rte_mbuf *mbuf, >>>                          struct ena_com_rx_ctx *ena_rx_ctx) >>>   { >>> @@ -1099,6 +1085,7 @@ static int ena_create_io_queue(struct ena_ring >>> *ring) >>>   { >>>       struct ena_adapter *adapter; >>>       struct ena_com_dev *ena_dev; >>> +    struct rte_memseg_list *msl; >>>       struct ena_com_create_io_ctx ctx = >>>           /* policy set to _HOST just to satisfy icc compiler */ >>>           { ENA_ADMIN_PLACEMENT_POLICY_HOST, >>> @@ -1126,7 +1113,8 @@ static int ena_create_io_queue(struct ena_ring >>> *ring) >>>       } >>>       ctx.qid = ena_qid; >>>       ctx.msix_vector = -1; /* interrupts not used */ >>> -    ctx.numa_node = ena_cpu_to_node(ring->id); >>> +    msl = rte_mem_virt2memseg_list(ring); >>> +    ctx.numa_node = msl->socket_id; >>>       rc = ena_com_create_io_queue(ena_dev, &ctx); >>>       if (rc) { >>> >> >> Hi Anatoly, >> >> I'm not sure why the previous maintainers implemented this that way, I >> can only guess. I think that they were assuming, that each queue will >> be assigned to the lcore which is equal to ring id. They probably also >> misunderstood how the memzones are working and they thought that each >> lcore is having assigned only one memzone which is being mapped 1 to 1. >> >> They wanted to prevent cross NUMA data acces, when the CPU is >> operating in the different NUMA zone and the IO queues memory resides >> in the other. I think that above solution won't prevent that neither, >> as you are using ring address, which is being allocated together with >> struct ena_adapter (it is just an array), so it will probably reside >> in the single numa zone. >> >> I'm currently thinking on solution that could help us to determine on >> which numa zone the queue descriptors will be allocated and on which >> the lcore assigned to the queue will be working, but have no any ideas >> for now :) >> >> Anyway, your fix won't break anything, as the previous solution wasn't >> working as it was supposed to work, so before I will fix that, we can >> keep that patch to prevent direct usage of the memzone. >> >> Thanks, >> Michal > > After investigation I think that we should use socket_id provided by the > tx/rx queue setup functions. > Could you, please, abandon this patch? I will send the proper fix soon. > I can't really "abandon" it as it will break ENA compilation once the structure is hidden in the last patch. What i can do is wait for you to submit your patch, and either rebase my patchset on top of it, or (better) include it in the patchset itself. > Thanks, > Michal > -- Thanks, Anatoly