'noon,
On 23/10/2015 15:27, Wiles, Keith wrote:
>> + uint32_t __rte_cache_aligned state_flags[RTE_KEEPALIVE_MAXCORES];
> Normally I see the __rte_cache_aligned at the end of the line before
> the ?;? did you have a reason to have it here? If not then I would
> move it to the end to look the same as the others. I did a quick grop
> in the code and that is the normal location.
>
> My next question is why not align the whole, which would do the same
> thing. I did not check the compiler output, but I was thinking it
> would possible leave gaps in the structure for bytes we can not use
> normally, but maybe that is not a problem.
Each element of state_flags is assigned to a different LCore, so they
have to be individually cache-aligned. The gaps it leaves behind are
unavoidable.
> Next it appears the state_flags is only being set to 0-3, which means
> it does not need to be a uint43_t, but could be a uint8_t, correct?
Yes, but since it all needs to be cache aligned anyway, wouldn't
actually gain anything.
>> + keepcfg = malloc(sizeof(struct rte_keepalive));
>> + if (keepcfg != NULL) {
>> + for (idx_core = 0; idx_core < RTE_KEEPALIVE_MAXCORES;
>> idx_core++) {
>> + keepcfg->state_flags[idx_core] = 0;
>> + keepcfg->active_cores[idx_core] = 0;
>> + }
>
> Could you have done a calloc then you do not need the for loop to zero stuff?
Could do. It was written this way because the function originally took a
structure rather than allocate one.
..Remy
