Re: [PATCH v4] lib: set/get max memzone segments

[Thomas Monjalon <thomas@monjalon.net>]

25/05/2023 00:25, Ophir Munk:
> --- a/config/rte_config.h
> +++ b/config/rte_config.h
> -#define RTE_MAX_MEMZONE 2560

Good to be able to remove this compilation-time configuration.


> --- a/lib/eal/common/eal_common_memzone.c
> +++ b/lib/eal/common/eal_common_memzone.c
> +#define DEFAULT_MAX_MEMZONE 2560

Maybe add "_COUNT" at the end to make clear it is not about the size of a memzone.
We should add a comment here to explain the meaning of this default:
used until the "set" function is called.


> -		"%s(): Number of requested memzone segments exceeds RTE_MAX_MEMZONE\n",
> -			__func__);
> +		"%s(): Number of requested memzone segments exceeds "
> +		"maximum %u\n", __func__, arr->len);

We should keep "maximum" on the first line to ease "grep" in the code.

> +int
> +rte_memzone_max_set(size_t max)
> +{
> +	struct rte_mem_config *mcfg;
> +
> +	if (eal_get_internal_configuration()->init_complete > 0)
> +		return -1;

An error log would be needed here I think.

> +
> +	mcfg = rte_eal_get_configuration()->mem_config;
> +	if (!mcfg)

Better to use "== NULL" for pointers.

> +		return -1;

Do we need an error log as well?

> +
> +	mcfg->max_memzone = max;
> +
> +	return 0;
> +}
> +
> +size_t
> +rte_memzone_max_get(void)
> +{
> +	struct rte_mem_config *mcfg;
> +
> +	mcfg = rte_eal_get_configuration()->mem_config;
> +	if (!mcfg || !mcfg->max_memzone)

Same comment as above: don't use boolean operator for pointer or value.
 
> +		return DEFAULT_MAX_MEMZONE;
> +
> +	return mcfg->max_memzone;
> +}
> diff --git a/lib/eal/common/eal_memcfg.h b/lib/eal/common/eal_memcfg.h
> index ea013a9..183bb25 100644
> --- a/lib/eal/common/eal_memcfg.h
> +++ b/lib/eal/common/eal_memcfg.h
> @@ -75,6 +75,8 @@ struct rte_mem_config {
>  	/**< TSC rate */
>  
>  	uint8_t dma_maskbits; /**< Keeps the more restricted dma mask. */
> +
> +	size_t max_memzone; /**< maximum allowed allocated memzones. */

Uppercase for first work, and we may remove "allowed"?
Suggestion: "Maximum number of allocated memzones."

[...]
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Set max memzone value

Add a dot at the end.
Instead of "value", we should mention "number" or "count".

> + *
> + * This function can only be called prior to rte_eal_init().
> + *
> + * @param max
> + *   Maximum number of memzones
> + * @return
> + *  0 on success, -1 otherwise
> + */
> +__rte_experimental
> +int rte_memzone_max_set(size_t max);
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Get the maximum number of memzones.
> + *
> + * @note: The maximum value will not change after calling rte_eal_init().
> + *
> + * @return
> + *   Maximum number of memzones
> + */
> +__rte_experimental
> +size_t rte_memzone_max_get(void);

Good, thank you.