RE: [PATCH] eventdev: fix alignment padding

["Morten Brørup" <mb@smartsharesystems.com>]

> From: Mattias Rönnblom [mailto:mattias.ronnblom@ericsson.com]
> Sent: Tuesday, 18 April 2023 17.17
> 
> On 2023-04-18 16:07, Morten Brørup wrote:
> >> From: Mattias Rönnblom [mailto:mattias.ronnblom@ericsson.com]
> >> Sent: Tuesday, 18 April 2023 14.31
> >>
> >> On 2023-04-18 12:45, Sivaprasad Tummala wrote:
> >>> fixed the padding required to align to cacheline size.
> >>>
> >>
> >> What's the point in having this structure cache-line aligned? False
> >> sharing is a non-issue, since this is more or less a read only struct.
> >>
> >> This is not so much a comment on your patch, but the __rte_cache_aligned
> >> attribute.
> >
> > When the structure is cache aligned, an individual entry in the array does
> not unnecessarily cross a cache line border. With 16 pointers and aligned, it
> uses exactly two cache lines. If unaligned, it may span three cache lines.
> >
> An *element* in the reserved uint64_t array won't span across two cache
> lines, regardless if __rte_cache_aligned is specified or not. You would
> need a packed struct for that to occur, plus the reserved array field
> being preceded by some appropriately-sized fields.

What I wrote above made no sense, and I agree with everything in your response.

However, I meant to write "an individual entry in the rte_event_fp_ops[RTE_EVENT_MAX_DEVS] array", so please re-read my comment with that in mind.

There are similar arrays, e.g. for Ethdev, where the same alignment goal applies.

> 
> The only effect __rte_cache_aligned has on this particular struct is
> that if you instantiate the struct on the stack, or as a static
> variable, it will be cache-line aligned.

Or as elements in an array, such as rte_event_fp_ops[RTE_EVENT_MAX_DEVS].

> That effect you can get by
> specifying the attribute when you define the variable, and you will save
> some space (by having smaller elements). In this case it doesn't matter
> if the array is compact or not, since an application is likely to only
> use one of the members in the array.
> 
> It also doesn't matter of the struct is two or three cache lines, as
> long as only the first two are used.

I think the drivers are likely to use only one function pointer at a time, but they are likely to use the data pointer at the same time. So, when the struct is used in an array, cache aligning the struct prevents the data pointer from ending up as the last pointer in a preceding cache line.

> 
> >>
> >>> Fixes: 54f17843a887 ("eventdev: add port maintenance API")
> >>> Cc: mattias.ronnblom@ericsson.com
> >>>
> >>> Signed-off-by: Sivaprasad Tummala <sivaprasad.tummala@amd.com>
> >>> ---
> >>>    lib/eventdev/rte_eventdev_core.h | 2 +-
> >>>    1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/lib/eventdev/rte_eventdev_core.h
> >> b/lib/eventdev/rte_eventdev_core.h
> >>> index c328bdbc82..c27a52ccc0 100644
> >>> --- a/lib/eventdev/rte_eventdev_core.h
> >>> +++ b/lib/eventdev/rte_eventdev_core.h
> >>> @@ -65,7 +65,7 @@ struct rte_event_fp_ops {
> >>>    	/**< PMD Tx adapter enqueue same destination function. */
> >>>    	event_crypto_adapter_enqueue_t ca_enqueue;
> >>>    	/**< PMD Crypto adapter enqueue function. */
> >>> -	uintptr_t reserved[6];
> >>> +	uintptr_t reserved[5];
> >>>    } __rte_cache_aligned;
> >>>
> >>>    extern struct rte_event_fp_ops rte_event_fp_ops[RTE_EVENT_MAX_DEVS];
> >