On Mon, Nov 11, 2024 at 6:39 AM Jerin Jacob <jer...@marvell.com> wrote:
>
>
>
> > -----Original Message-----
> > From: David Marchand <david.march...@redhat.com>
> > Sent: Friday, November 8, 2024 7:08 PM
> > To: Jerin Jacob <jer...@marvell.com>
> > Cc: Huichao Cai <chcch...@163.com>; Kiran Kumar Kokkilagadda
> > <kirankum...@marvell.com>; Nithin Kumar Dabilpuram
> > <ndabilpu...@marvell.com>; yanzhirun_...@163.com; dev@dpdk.org;
> > Thomas Monjalon <tho...@monjalon.net>; Robin Jarry <rja...@redhat.com>
> > Subject: Re: Re:RE: [EXTERNAL] [PATCH] graph: optimize graph search when
> > scheduling nodes
> >
> > Hello Jerin, On Fri, Nov 8, 2024 at 1: 22 PM Jerin Jacob <jerinj@ marvell.
> > com>
> > wrote: > > > Is n't breaking the ABI? > > > > So can't we modify the ABI,
> > or is
> > there any special operation required to modify > >
> > Hello Jerin,
>
> Hello David,
>
> >
> > On Fri, Nov 8, 2024 at 1:22 PM Jerin Jacob <jer...@marvell.com> wrote:
> > > > > Is n't breaking the ABI?
> > > >
> > > > So can't we modify the ABI, or is there any special operation
> > > > required to modify the ABI?
> > >
> > > Only LTS release (xx.11) can change the ABI after sending deprecation
> > > notice.
> > > Looking at the pahole output, one option will be making dispatch and
> > > new semi fastpath Additions like xstat_off can be min cache aligned
> > > to make room for future expansion and to make sure have better
> > performance.
> >
> > Adding holes may be a short term solution, but in my opinion, the slow path
> > part should be entirely hidden and we only expose the fp part.
>
> The new cache line alignment items are proposed are fastpath items only.
I had only noticed the second comment:
+ alignas(RTE_CACHE_LINE_MIN_SIZE)
rte_graph_off_t xstat_off; /**< Offset to xstat counters. */
/* Fast path area */
^^^^^^^^^^^^
And I assumed the part in the struct before was slow path.
(it may be worth enhancing these comments, with a single limit of
slow/fast path areas)
>
> > Reminder, those holes must be in a "known state" as we release v24.11 so
> > that
> > the presence of future additions can be safely detected.
If the rte_node objects are allocated by the graph library and zero'd,
then we are good.
It seems to be the case in graph_nodes_populate(), and the rte_node
objects are embedded in the rte_graph object.
Is there another location in the graph library where a rte_node object
is allocated?
If not, and an application can not create a rte_node object, your
proposal looks good to me.
--
David Marchand
