Re: [PATCH 4/4] net/netvsc: cache device parameters for hot plug events

[Stephen Hemminger <stephen@networkplumber.org>]

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?
Why does it need to be huge pages? versus normal malloc?
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[];
};


	cache = malloc(sizeof(*cache) + strlen(da->drv_str) + 1);
...
	strcpy(cache->drv_str, da->drv_str);