RE: [EXTERNAL] [PATCH v4 2/2] graph: add alignment to the member of rte_node

[Jerin Jacob <jerinj@marvell.com>]

> -----Original Message-----
> From: Huichao Cai <chcchc88@163.com>
> Sent: Thursday, November 14, 2024 2:15 PM
> To: Jerin Jacob <jerinj@marvell.com>; Kiran Kumar Kokkilagadda
> <kirankumark@marvell.com>; Nithin Kumar Dabilpuram
> <ndabilpuram@marvell.com>; yanzhirun_163@163.com
> Cc: dev@dpdk.org
> Subject: [EXTERNAL] [PATCH v4 2/2] graph: add alignment to the member of
> rte_node
> 
> The members dispatch and xstat_off of the structure rte_node can be min cache
> aligned to make room for future expansion and to make sure have better
> performance. Add corresponding comments. Due to the modification of the
> alignment of some members 
> The members dispatch and xstat_off of the structure rte_node can be min cache
> aligned to make room for future expansion and to make sure have better
> performance. Add corresponding comments.
> 

Please change subject to graph: improve node layout


> Due to the modification of the alignment of some members of the rte_node
> structure, update file release_24_11.rst.

The above section is not needed.


> 
> Signed-off-by: Huichao Cai <chcchc88@163.com>
> ---
>  doc/guides/rel_notes/release_24_11.rst | 3 +++
>  lib/graph/rte_graph_worker_common.h    | 7 ++++++-
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/guides/rel_notes/release_24_11.rst
> b/doc/guides/rel_notes/release_24_11.rst
> index 592116b979..6903b1d0f0 100644
> --- a/doc/guides/rel_notes/release_24_11.rst
> +++ b/doc/guides/rel_notes/release_24_11.rst
> @@ -425,6 +425,9 @@ ABI Changes
> 
>  * graph: added ``graph`` field to the ``dispatch`` structure in the ``rte_node``
> structure.
> 
> +* graph: The members ``dispatch`` and ``xstat_off`` of the structure
> +``rte_node`` have been
> +  marked as RTE_CACHE_LINE_MIN_SIZE bytes aligned.
> +
>  Known Issues
>  ------------
> 
> diff --git a/lib/graph/rte_graph_worker_common.h
> b/lib/graph/rte_graph_worker_common.h
> index 4c2432b47f..d36abec08b 100644
> --- a/lib/graph/rte_graph_worker_common.h
> +++ b/lib/graph/rte_graph_worker_common.h
> @@ -104,16 +104,21 @@ struct __rte_cache_aligned rte_node {
>  	/** Original process function when pcap is enabled. */
>  	rte_node_process_t original_process;
> 
> +	/** Fast path area cache line 1. */


Fast schedule area for mcore dispatch model

>  	union {
>  		/* Fast schedule area for mcore dispatch model */

Above comment you can remove it

> -		struct {
> +		alignas(RTE_CACHE_LINE_MIN_SIZE) struct {
>  			unsigned int lcore_id;  /**< Node running lcore. */
>  			uint64_t total_sched_objs; /**< Number of objects
> scheduled. */
>  			uint64_t total_sched_fail; /**< Number of scheduled
> failure. */
>  			struct rte_graph *graph;  /**< Graph corresponding to
> lcore_id. */
>  		} dispatch;
>  	};
> +
> +	/** Fast path area cache line 2. */
> +	alignas(RTE_CACHE_LINE_MIN_SIZE)
>  	rte_graph_off_t xstat_off; /**< Offset to xstat counters. */
> +
>  	/* Fast path area  */


Fast path area cache line 1

>  	__extension__ struct __rte_cache_aligned {  #define RTE_NODE_CTX_SZ
> 16

With above: Acked-by: Jerin Jacob <jerinj@marvell.com>

Looks loke we cannot merge new feature in rc3. I would suggest skip 1/2 and send only this patch so that 1/2 can merged in next release.

Please add @david.marchand@redhat.com in Cc.