> From: Mattias Rönnblom [mailto:mattias.ronnb...@ericsson.com]
> Sent: Tuesday, 20 February 2024 09.49
>
> Replace static array of cache-aligned structs with an lcore variable,
> to slightly benefit code simplicity and performance.
>
> Signed-off-by: Mattias Rönnblom <mattias.ronnb...@ericsson.com>
> ---
> @@ -486,8 +489,7 @@ service_runner_func(void *arg)
> {
> RTE_SET_USED(arg);
> uint8_t i;
> - const int lcore = rte_lcore_id();
> - struct core_state *cs = &lcore_states[lcore];
> + struct core_state *cs = RTE_LCORE_VAR_PTR(lcore_states);
Typo: TAB -> SPACE.
>
> rte_atomic_store_explicit(&cs->thread_active, 1,
> rte_memory_order_seq_cst);
>
> @@ -533,13 +535,16 @@ service_runner_func(void *arg)
> int32_t
> rte_service_lcore_may_be_active(uint32_t lcore)
> {
> - if (lcore >= RTE_MAX_LCORE || !lcore_states[lcore].is_service_core)
> + struct core_state *cs =
> + RTE_LCORE_VAR_LCORE_PTR(lcore, lcore_states);
> +
> + if (lcore >= RTE_MAX_LCORE || !cs->is_service_core)
> return -EINVAL;
This comment is mostly related to patch 1 in the series...
You are setting cs = RTE_LCORE_VAR_LCORE_PTR(lcore, ...) before validating that
lcore < RTE_MAX_LCORE. I wondered if that potentially was an overrun bug.
It is obvious when looking at the RTE_LCORE_VAR_LCORE_PTR() macro
implementation, but perhaps its description could mention that it is safe to
use with an "invalid" lcore_id, but not dereferencing it.
