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 DEC0AA0487 for ; Thu, 4 Jul 2019 09:50:55 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id A267F1B942; Thu, 4 Jul 2019 09:50:55 +0200 (CEST) Received: from mail-vs1-f67.google.com (mail-vs1-f67.google.com [209.85.217.67]) by dpdk.org (Postfix) with ESMTP id 626461041 for ; Thu, 4 Jul 2019 09:50:54 +0200 (CEST) Received: by mail-vs1-f67.google.com with SMTP id k9so1430512vso.5 for ; Thu, 04 Jul 2019 00:50:54 -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=nTdfD2R7N7Omcr2E7qzU367ZK1jRenQCXyo5MoZq7yE=; b=YC2MKa8jxzHMN4okYXjPZSg5y5KD7M58hiMWr30Iecvzh0ZbbkOb16pQoO/UhbUku5 lTm/iyppPJC9NoCX2knytRUx/cZb9JoEbn4DeZFjbPzu9wVPusTT8614HdRPypN9dhCe p4djum44OyUN9+v91PMl81X+5iimwy7l4QeJFKFtA8lI7pWLKMfVU0ETBkq5kIHHE1T6 38LLGvl486/cM9zh4ToJBsbvp0V3TZuUQ8WU+g49KpyaRhA+jeQCmTMCI7Cc+RuQ5I9N YctP27E+Wr41abq49H9kHB7HVjSMWn7XH4ie7wNo9EL4J3E3doQbJBY3x4TMq7eRBLCo vp7A== X-Gm-Message-State: APjAAAXszyrGC7gfElS+AHTxFoA15axMgtz6PC8ID5mNVPCwsm73jxRz PYqXf8wSE5YXYRIU8HJ63jnzm6LNhA9KGmo2YnZ1zw== X-Google-Smtp-Source: APXvYqzKLGUX9tx5Epgaa80JH+BlZzvrhO9qNwZMP0hK4BMcp44dgxGkhzfPb1Z47V/MagoBCMLw10ddzHvjEoATCOU= X-Received: by 2002:a67:e9ca:: with SMTP id q10mr21427584vso.105.1562226653816; Thu, 04 Jul 2019 00:50:53 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: David Marchand Date: Thu, 4 Jul 2019 09:50:43 +0200 Message-ID: To: Anatoly Burakov Cc: dev , Bruce Richardson , Thomas Monjalon , Stephen Hemminger Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH v3 13/14] eal: unify internal config initialization 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 Thu, Jun 27, 2019 at 1:39 PM Anatoly Burakov wrote: > Currently, each EAL will update internal/shared config in their > own way at init, resulting in needless duplication of code and > OS-dependent behavior. Move the functions to a common file and > add missing FreeBSD steps. > > Signed-off-by: Anatoly Burakov > --- > lib/librte_eal/common/eal_common_mcfg.c | 18 ++++++++++++++++++ > lib/librte_eal/common/eal_memcfg.h | 8 ++++++++ > lib/librte_eal/freebsd/eal/eal.c | 2 ++ > lib/librte_eal/linux/eal/eal.c | 22 ++-------------------- > 4 files changed, 30 insertions(+), 20 deletions(-) > > diff --git a/lib/librte_eal/common/eal_common_mcfg.c > b/lib/librte_eal/common/eal_common_mcfg.c > index dc6665d6a..fe8d2b726 100644 > --- a/lib/librte_eal/common/eal_common_mcfg.c > +++ b/lib/librte_eal/common/eal_common_mcfg.c > @@ -31,6 +31,24 @@ eal_mcfg_wait_complete(void) > rte_pause(); > } > > +void > +eal_mcfg_update_internal(void) > +{ > + struct rte_mem_config *mcfg = > rte_eal_get_configuration()->mem_config; > + > + internal_config.legacy_mem = mcfg->legacy_mem; > + internal_config.single_file_segments = mcfg->single_file_segments; > +} > + > +void > +eal_mcfg_update_from_internal(void) > +{ > + struct rte_mem_config *mcfg = > rte_eal_get_configuration()->mem_config; > + > + mcfg->legacy_mem = internal_config.legacy_mem; > + mcfg->single_file_segments = internal_config.single_file_segments; > +} > + > void > rte_mcfg_mem_read_lock(void) > { > diff --git a/lib/librte_eal/common/eal_memcfg.h > b/lib/librte_eal/common/eal_memcfg.h > index a2434417e..d02ac1621 100644 > --- a/lib/librte_eal/common/eal_memcfg.h > +++ b/lib/librte_eal/common/eal_memcfg.h > @@ -68,6 +68,14 @@ struct rte_mem_config { > uint8_t dma_maskbits; > }; > > +/* update internal config from shared mem config */ > +void > +eal_mcfg_update_internal(void); > + > +/* update shared mem config from internal config */ > +void > +eal_mcfg_update_from_internal(void); > + > /* wait until primary process initialization is complete */ > void > eal_mcfg_wait_complete(void); > diff --git a/lib/librte_eal/freebsd/eal/eal.c > b/lib/librte_eal/freebsd/eal/eal.c > index 13e230fc8..6bfe203fd 100644 > --- a/lib/librte_eal/freebsd/eal/eal.c > +++ b/lib/librte_eal/freebsd/eal/eal.c > @@ -379,6 +379,7 @@ rte_config_init(void) > case RTE_PROC_PRIMARY: > if (rte_eal_config_create() < 0) > return -1; > + eal_mcfg_update_internal(); > break; > case RTE_PROC_SECONDARY: > if (rte_eal_config_attach() < 0) > @@ -386,6 +387,7 @@ rte_config_init(void) > eal_mcfg_wait_complete(); > if (rte_eal_config_reattach() < 0) > return -1; > + eal_mcfg_update_from_internal(); > break; > case RTE_PROC_AUTO: > case RTE_PROC_INVALID: > Hum, you swapped eal_mcfg_update_internal and eal_mcfg_update_from_internal. The names are a bit ambiguous, and I wonder if we really need those separate helpers. rte_eal_config_create and rte_eal_config_attach are already awfully close in linux and freebsd implementation. Can't we have them as common code and put those helpers bits direct in them ? > diff --git a/lib/librte_eal/linux/eal/eal.c > b/lib/librte_eal/linux/eal/eal.c > index 4fd18b15f..fa205fd29 100644 > --- a/lib/librte_eal/linux/eal/eal.c > +++ b/lib/librte_eal/linux/eal/eal.c > @@ -472,24 +472,6 @@ eal_proc_type_detect(void) > return ptype; > } > > -/* copies data from internal config to shared config */ > -static void > -eal_update_mem_config(void) > -{ > - struct rte_mem_config *mcfg = > rte_eal_get_configuration()->mem_config; > - mcfg->legacy_mem = internal_config.legacy_mem; > - mcfg->single_file_segments = internal_config.single_file_segments; > -} > - > -/* copies data from shared config to internal config */ > -static void > -eal_update_internal_config(void) > -{ > - struct rte_mem_config *mcfg = > rte_eal_get_configuration()->mem_config; > - internal_config.legacy_mem = mcfg->legacy_mem; > - internal_config.single_file_segments = mcfg->single_file_segments; > -} > - > /* Sets up rte_config structure with the pointer to shared memory > config.*/ > static int > rte_config_init(void) > @@ -500,7 +482,7 @@ rte_config_init(void) > case RTE_PROC_PRIMARY: > if (rte_eal_config_create() < 0) > return -1; > - eal_update_mem_config(); > + eal_mcfg_update_from_internal(); > break; > case RTE_PROC_SECONDARY: > if (rte_eal_config_attach() < 0) > @@ -508,7 +490,7 @@ rte_config_init(void) > eal_mcfg_wait_complete(); > if (rte_eal_config_reattach() < 0) > return -1; > - eal_update_internal_config(); > + eal_mcfg_update_internal(); > break; > case RTE_PROC_AUTO: > case RTE_PROC_INVALID: > -- > 2.17.1 > -- David Marchand