From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 03EAD45DE4 for ; Fri, 6 Dec 2024 12:29:30 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id C3A8240267; Fri, 6 Dec 2024 12:29:30 +0100 (CET) Received: from mail.lysator.liu.se (mail.lysator.liu.se [130.236.254.3]) by mails.dpdk.org (Postfix) with ESMTP id 0BE7F40267; Fri, 6 Dec 2024 12:29:30 +0100 (CET) Received: from mail.lysator.liu.se (localhost [127.0.0.1]) by mail.lysator.liu.se (Postfix) with ESMTP id 998A6162BB; Fri, 6 Dec 2024 12:29:29 +0100 (CET) Received: by mail.lysator.liu.se (Postfix, from userid 1004) id 6B2581626C; Fri, 6 Dec 2024 12:29:29 +0100 (CET) X-Spam-Checker-Version: SpamAssassin 4.0.0 (2022-12-13) on hermod.lysator.liu.se X-Spam-Level: X-Spam-Status: No, score=-1.2 required=5.0 tests=ALL_TRUSTED,AWL, T_SCC_BODY_TEXT_LINE autolearn=disabled version=4.0.0 X-Spam-Score: -1.2 Received: from [192.168.1.85] (h-62-63-215-114.A163.priv.bahnhof.se [62.63.215.114]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by mail.lysator.liu.se (Postfix) with ESMTPSA id 40BCA162B7; Fri, 6 Dec 2024 12:29:25 +0100 (CET) Message-ID: <16f6d334-d999-4fe7-b177-6daf881b8f1e@lysator.liu.se> Date: Fri, 6 Dec 2024 12:29:24 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 2/3] power: defer lcore variable allocation To: David Marchand , dev@dpdk.org Cc: thomas@monjalon.net, frode.nordahl@canonical.com, mattias.ronnblom@ericsson.com, stable@dpdk.org, Anatoly Burakov , David Hunt , Sivaprasad Tummala , Stephen Hemminger , Chengwen Feng , Konstantin Ananyev , =?UTF-8?Q?Morten_Br=C3=B8rup?= References: <20241205175754.1673888-1-david.marchand@redhat.com> <20241205175754.1673888-3-david.marchand@redhat.com> Content-Language: en-US From: =?UTF-8?Q?Mattias_R=C3=B6nnblom?= In-Reply-To: <20241205175754.1673888-3-david.marchand@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Virus-Scanned: ClamAV using ClamSMTP X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org On 2024-12-05 18:57, David Marchand wrote: > The lcore variable in this code unit is only used through > rte_power_ethdev_pmgmt_queue_*() public symbols. > > Defer the unconditional lcore variable allocation in those symbols. > > Fixes: 130643319579 ("power: keep per-lcore state in lcore variable") > Cc: stable@dpdk.org > > Signed-off-by: David Marchand > --- > lib/power/rte_power_pmd_mgmt.c | 27 +++++++++++++++++++-------- > 1 file changed, 19 insertions(+), 8 deletions(-) > > diff --git a/lib/power/rte_power_pmd_mgmt.c b/lib/power/rte_power_pmd_mgmt.c > index a2fff3b765..29e2d438a3 100644 > --- a/lib/power/rte_power_pmd_mgmt.c > +++ b/lib/power/rte_power_pmd_mgmt.c > @@ -72,6 +72,19 @@ struct __rte_cache_aligned pmd_core_cfg { > }; > static RTE_LCORE_VAR_HANDLE(struct pmd_core_cfg, lcore_cfgs); > > +static void > +alloc_lcore_cfgs(void) > +{ > + struct pmd_core_cfg *lcore_cfg; > + unsigned int lcore_id; > + > + RTE_LCORE_VAR_ALLOC(lcore_cfgs); > + > + /* initialize all tailqs */ > + RTE_LCORE_VAR_FOREACH(lcore_id, lcore_cfg, lcore_cfgs) > + TAILQ_INIT(&lcore_cfg->head); > +} > + > static inline bool > queue_equal(const union queue *l, const union queue *r) > { > @@ -517,6 +530,9 @@ rte_power_ethdev_pmgmt_queue_enable(unsigned int lcore_id, uint16_t port_id, > goto end; > } > > + if (lcore_cfgs == NULL) > + alloc_lcore_cfgs(); > + I would wrap all RTE_LCORE_VAR_LCORE() and RTE_LCORE_VAR(). static struct pmd_core_cfg * get_cfg_lcore(unsigned int lcore_id) { assure_lcore_cfgs_alloced(); return RTE_LCORE_VAR_LCORE(lcore_cfgs, lcore_id); } static struct pmd_core_cfg * get_cfg(void) { get_cfg_lcore(rte_lcore_id()); } Add static void assure_lcore_cfgs_alloced(unsigned int lcore_id) { if (lcore_cfgs != NULL) lcore_cfgs_alloc(); } ..or maybe better merge assure_lcore_cfgs_alloced() and lcore_cfgs_alloc(). Makes it a little harder to make mistakes. A somewhat unrelated question: why is pmd_core_cfg cache-line aligned? I don't think it should be. > lcore_cfg = RTE_LCORE_VAR_LCORE(lcore_id, lcore_cfgs); > > /* check if other queues are stopped as well */ > @@ -617,6 +633,9 @@ rte_power_ethdev_pmgmt_queue_disable(unsigned int lcore_id, > return ret < 0 ? -EINVAL : -EBUSY; > } > > + if (lcore_cfgs == NULL) > + alloc_lcore_cfgs(); > + > /* no need to check queue id as wrong queue id would not be enabled */ > lcore_cfg = RTE_LCORE_VAR_LCORE(lcore_id, lcore_cfgs); > > @@ -768,16 +787,8 @@ rte_power_pmd_mgmt_get_scaling_freq_max(unsigned int lcore) > } > > RTE_INIT(rte_power_ethdev_pmgmt_init) { > - unsigned int lcore_id; > - struct pmd_core_cfg *lcore_cfg; > int i; > > - RTE_LCORE_VAR_ALLOC(lcore_cfgs); > - > - /* initialize all tailqs */ > - RTE_LCORE_VAR_FOREACH(lcore_id, lcore_cfg, lcore_cfgs) > - TAILQ_INIT(&lcore_cfg->head); > - > /* initialize config defaults */ > emptypoll_max = 512; > pause_duration = 1;