[PATCH v3 1/5] event/opdl: fix non-constant compile time assertion

Andrew Rybchenko andrew.rybchenko at oktetlabs.ru
Wed Jan 17 08:58:12 CET 2024


On 1/16/24 21:41, Stephen Hemminger wrote:
> RTE_BUILD_BUG_ON() was being used with a non-constant value.
> The inline function rte_is_power_of_2() is not constant since
> inline expansion happens later in the compile process.
> Replace it with macro which will be constant.
> 
> Fixes: 4236ce9bf5bf ("event/opdl: add OPDL ring infrastructure library")
> Cc: liang.j.ma at intel.com
> Signed-off-by: Stephen Hemminger <stephen at networkplumber.org>
> Acked-by: Bruce Richardson <bruce.richardson at intel.com>
> Acked-by: Tyler Retzlaff <roretzla at linux.microsoft.com>
> ---
>   drivers/event/opdl/opdl_ring.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/event/opdl/opdl_ring.c b/drivers/event/opdl/opdl_ring.c
> index 69392b56bbec..24e0bbe3222d 100644
> --- a/drivers/event/opdl/opdl_ring.c
> +++ b/drivers/event/opdl/opdl_ring.c
> @@ -31,6 +31,9 @@
>   #define OPDL_OPA_MASK    (0xFF)
>   #define OPDL_OPA_OFFSET  (0x38)
>   
> +/* Equivalent to rte_is_power_of_2() but as macro. */
> +#define IS_POWER_OF_2(x) (((x) & ((x) - 1)) == 0)

IMHO adding it in specific driver is a wrong direction. I'm afraid it
will result in duplication of such macros in code base without clear
reason why.

May be it is better to add it with a proper name to EAL?

> +
>   /* Types of dependency between stages */
>   enum dep_type {
>   	DEP_NONE = 0,  /* no dependency */
> @@ -910,7 +913,7 @@ opdl_ring_create(const char *name, uint32_t num_slots, uint32_t slot_size,
>   			RTE_CACHE_LINE_MASK) != 0);
>   	RTE_BUILD_BUG_ON((offsetof(struct opdl_ring, slots) &
>   			RTE_CACHE_LINE_MASK) != 0);
> -	RTE_BUILD_BUG_ON(!rte_is_power_of_2(OPDL_DISCLAIMS_PER_LCORE));
> +	RTE_BUILD_BUG_ON(!IS_POWER_OF_2(OPDL_DISCLAIMS_PER_LCORE));
>   
>   	/* Parameter checking */
>   	if (name == NULL) {



More information about the dev mailing list