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 17E0FA0096 for ; Tue, 4 Jun 2019 14:38:44 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 66E2F1BB09; Tue, 4 Jun 2019 14:38:42 +0200 (CEST) Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id 425871B9CD for ; Tue, 4 Jun 2019 14:38:40 +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 fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 04 Jun 2019 05:38:39 -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 05:38:37 -0700 To: =?UTF-8?Q?Micha=c5=82_Krawczyk?= Cc: dev@dpdk.org, Marcin Wojtas , Guy Tzalik , Evgeny Schemeilin , stephen@networkplumber.org, Thomas Monjalon , 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: <08be0d61-3d6e-961a-92d9-42be6ff3a95c@intel.com> Date: Tue, 4 Jun 2019 13:38:36 +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: 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 04-Jun-19 11:45 AM, Michał Krawczyk wrote: > wt., 4 cze 2019 o 12:28 Burakov, Anatoly napisał(a): >> >> 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. > > Ok, I've just uploaded the patch (second version has fixed commit > log), you can find it below > https://patches.dpdk.org/patch/54352/ > > I'm fine with including the patch into this patchset. > Thanks for your quick implementation! I'll include it in the inevitable v2. >> >>> Thanks, >>> Michal >>> >> >> >> -- >> Thanks, >> Anatoly > -- Thanks, Anatoly