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 9E68645FE6; Wed, 29 Jan 2025 01:31:35 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 2A65D4026F; Wed, 29 Jan 2025 01:31:35 +0100 (CET) Received: from mail-pj1-f49.google.com (mail-pj1-f49.google.com [209.85.216.49]) by mails.dpdk.org (Postfix) with ESMTP id E761A40268 for ; Wed, 29 Jan 2025 01:31:33 +0100 (CET) Received: by mail-pj1-f49.google.com with SMTP id 98e67ed59e1d1-2ee9a780de4so8477317a91.3 for ; Tue, 28 Jan 2025 16:31:33 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20230601.gappssmtp.com; s=20230601; t=1738110693; x=1738715493; darn=dpdk.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=1rCyiCy2z+oiN62cPQpqcs26V4xiWlTUwrykqKmeW9s=; b=rK1H3Iafd9SGWNXnkvNXa2Fgh1AkqHRe4kfz5APNi5HRrL8JGehYy/sXopyUmJh6rX 26IZl23+UIPTVFJZrFsBJSiHqKD1PlidThDtc/4Y2mxt0KRlCxjKXlkLDg/Yh3JZ7qPi tNAKwhRywc6qp0oCkooytqMYttHQ5Z+n659zwWoCPt9iKPQgsWXlf7z72vj4oTUdJKsT Vc0WzOJ+pyQMWpDfWEgwoY+JsJUf4kfxt8O5l+ee2ISfkAm2G8aDURBDJi1a7a3LR4Sm OGWOsAspq1gRLT0Sgb56WRtcbaS7t5eKie+lihokNvhglG40L/hDy0gQOub3lJ7skAic zcpw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738110693; x=1738715493; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=1rCyiCy2z+oiN62cPQpqcs26V4xiWlTUwrykqKmeW9s=; b=Y11zoMTMVD4an+MQTfRIg/9bnPHw6X0myO6bOeVCUCOA67xVoT8omQKoG1GKf7WX5U flKSBKeJvdRxAVp4Jw+PAIAKiqgzGNvXbRFk9u9+vAzihxpbooA2xsQO9EgOsINy3rK4 gIYloONDU9BiSxHcQIML+1zk/YixXxmKZh2R1QaEfeiIJor7igFlxLUBh4S/HZuWi4+F tglD/8LWKZe1nlHbjPGK6W/K9veL45QdUSI78O/Siez7qcB6xQEJloDYnfAStjWO0MMq vOr+DHrTiiElol7+w0qYUCZ2weFuSuoloklUA0YOMQYVDAK/ySVyXg6jR5MjXhbPk3/s /URA== X-Forwarded-Encrypted: i=1; AJvYcCXN+D5ZUts3+DixTA7lih2g35koIG1OWVICG1S3CO7tkeJlGL6IvqKl64Z8XOCJ8pnTaSw=@dpdk.org X-Gm-Message-State: AOJu0Yzjl1Xy3MvQm5fS5017vbouchZFAYRRos1LVFWs8uymiNeg+e7W uJmK9/cufu70JsyhU+FMaN2E8cr8uu26CfrCAW9EVssn1FLKlI6+iF1GGsYrDAg= X-Gm-Gg: ASbGncvBokxaiy4DzQxKr5jCG3gKs/a3jnBKWsTOFalgOps7dkcrRlJzwSkE3zXKlRu 7USDQx+22prB2Th4TUtkUdi8djsGprl4CRKFZuF3VgEuOuOHYIkvAEkeIuvSvK6lWql0wKVxnc7 YdT+sfXYQpe30zcELcsxL0IpQjLZd06Ei/Ifr3yZbYHZh1aD88lnObTxGzksnhcUkgvVEiUJIa2 SbG5UAJcuKfg8iXtEmetoerutyUayCHAaoqk1epm/o1vp6YjnerCXvQ5Hs+43BWRGChT7iB1l/T hTnULUWnkvf84Dnf1+3nByDNEHCzTXAuKgNWkSmQ2t+QBWsb3GTdOk8G90hWgDvG2GAb X-Google-Smtp-Source: AGHT+IFnvStFHNgeDkxfY+oKXFhKfOCOwVoyhnpnx/o4r1lLn8/NKIE48jCoYUx5itzuRXGzrq/beQ== X-Received: by 2002:a17:90b:1f8c:b0:2ee:94d1:7a89 with SMTP id 98e67ed59e1d1-2f83ab8c371mr1499907a91.1.1738110692592; Tue, 28 Jan 2025 16:31:32 -0800 (PST) Received: from hermes.local (204-195-96-226.wavecable.com. [204.195.96.226]) by smtp.gmail.com with ESMTPSA id 98e67ed59e1d1-2f83bd0963esm186136a91.22.2025.01.28.16.31.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 28 Jan 2025 16:31:32 -0800 (PST) Date: Tue, 28 Jan 2025 16:31:30 -0800 From: Stephen Hemminger To: Long Li Cc: "longli@linuxonhyperv.com" , Ferruh Yigit , Andrew Rybchenko , Wei Hu , "dev@dpdk.org" Subject: Re: [EXTERNAL] Re: [PATCH 4/4] net/netvsc: cache device parameters for hot plug events Message-ID: <20250128163130.4e718cc2@hermes.local> In-Reply-To: References: <1738028106-25239-1-git-send-email-longli@linuxonhyperv.com> <1738028106-25239-4-git-send-email-longli@linuxonhyperv.com> <20250128130056.399a1dd5@hermes.local> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Wed, 29 Jan 2025 00:10:12 +0000 Long Li wrote: > > Subject: [EXTERNAL] Re: [PATCH 4/4] net/netvsc: cache device parameters for > > hot plug events > > > > On Mon, 27 Jan 2025 17:35:06 -0800 > > longli@linuxonhyperv.com wrote: > > > > > @@ -1409,9 +1424,6 @@ eth_hn_dev_uninit(struct rte_eth_dev *eth_dev) > > > ret_stop = hn_dev_stop(eth_dev); > > > hn_dev_close(eth_dev); > > > > > > - free(hv->vf_devargs); > > > - hv->vf_devargs = NULL; > > > - > > > hn_detach(hv); > > > hn_chim_uninit(eth_dev); > > > rte_vmbus_chan_close(hv->channels[0]); > > > @@ -1423,6 +1435,61 @@ eth_hn_dev_uninit(struct rte_eth_dev *eth_dev) > > > return ret_stop; > > > } > > > > > > +static int populate_cache_list(void) > > > +{ > > > + int ret; > > > + struct rte_devargs *da; > > > + > > > + rte_spinlock_lock(&netvsc_lock); > > > + da_cache_usage++; > > > + if (da_cache_usage > 1) { > > > + ret = 0; > > > + goto out; > > > + } > > > + > > > + LIST_INIT(&da_cache_list); > > > + RTE_EAL_DEVARGS_FOREACH("pci", da) { > > > + struct da_cache *cache; > > > + > > > + cache = rte_zmalloc("NETVSC-HOTADD", sizeof(*cache), > > rte_mem_page_size()); > > > + if (!cache) { > > > + ret = -ENOMEM; > > > + goto out; > > > + } > > > + > > > + strncpy(cache->name, da->name, sizeof(da->name)); > > > + cache->drv_str = strdup(da->drv_str); > > > + if (!cache->drv_str) { > > > + rte_free(cache); > > > + ret = -ENOMEM; > > > + goto out; > > > + } > > > + > > > + LIST_INSERT_HEAD(&da_cache_list, cache, list); > > > + } > > > > Why do you need to cache entry to be page aligned, that seems unnecessary > > wasteful? > > You are right it doesn't need to. I'm removing the alignment. > > > Why does it need to be huge pages? versus normal malloc? > > I thought it would make it easier to debug memory leaks through EAL. But normal malloc is fine because this data is not shared between primary/secondary. > > > The string is coming from malloc (strdup) so it can't be used by secondary process. > > > > Since you are allocating a devargs cache entry, you could just as well use flexible > > array where entry was like: > > struct da_cache { > > LIST_ENTRY(da_cache) list; > > char name[RTE_DEV_NAME_MAX_LEN]; > > char drv_str[]; > > }; > > This is difficult as I don't know how big it is for drv_str[]. That is why the malloc adds extra space for drv_str, see below. Flexible array allow one array at end of structure > > > > > > cache = malloc(sizeof(*cache) + strlen(da->drv_str) + 1); ... > > strcpy(cache->drv_str, da->drv_str); > > Another approach is to modify EAL to never delete driver arguments when a device is removed. i.e., It doesn't call rte_devargs_remove() on device removal, instead keep those devargs for the lifetime of the process. Do you think this is a better approach? This will save work if other drivers want to cache devargs list for device hot plug events. Not sure, right now I don't think it is possible to pre-add devargs for future hot plug