[PATCH v9 13/17] graph: enable graph multicore dispatch scheduler model

[Yan, Zhirun]

> -----Original Message-----
> From: Jerin Jacob <jerinjacobk at gmail.com>
> Sent: Thursday, June 8, 2023 1:34 PM
> To: Yan, Zhirun <zhirun.yan at intel.com>
> Cc: dev at dpdk.org; jerinj at marvell.com; kirankumark at marvell.com;
> ndabilpuram at marvell.com; stephen at networkplumber.org;
> pbhagavatula at marvell.com; Liang, Cunming <cunming.liang at intel.com>; Wang,
> Haiyue <haiyue.wang at intel.com>; mattias.ronnblom
> <mattias.ronnblom at ericsson.com>
> Subject: Re: [PATCH v9 13/17] graph: enable graph multicore dispatch scheduler
> model
> 
> On Thu, Jun 8, 2023 at 8:39 AM Yan, Zhirun <zhirun.yan at intel.com> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Jerin Jacob <jerinjacobk at gmail.com>
> > > Sent: Wednesday, June 7, 2023 9:26 PM
> > > To: Yan, Zhirun <zhirun.yan at intel.com>
> > > Cc: dev at dpdk.org; jerinj at marvell.com; kirankumark at marvell.com;
> > > ndabilpuram at marvell.com; stephen at networkplumber.org;
> > > pbhagavatula at marvell.com; Liang, Cunming <cunming.liang at intel.com>;
> > > Wang, Haiyue <haiyue.wang at intel.com>; mattias.ronnblom
> > > <mattias.ronnblom at ericsson.com>
> > > Subject: Re: [PATCH v9 13/17] graph: enable graph multicore dispatch
> > > scheduler model
> > >
> > > On Wed, Jun 7, 2023 at 5:55 PM Yan, Zhirun <zhirun.yan at intel.com> wrote:
> > >
> > > > >
> > > > > >  rte_graph_walk(struct rte_graph *graph)  {
> > > > > > +#if !defined(RTE_GRAPH_MODEL_SELECT) ||
> > > RTE_GRAPH_MODEL_SELECT
> > > > > ==
> > > > > > +RTE_GRAPH_MODEL_RTC
> > > > >
> > > > > Is nt defined instead of !defined?
> > > > >
> > > >
> > > > !defined(XX) means not defined XX.
> > > > What is nt defined means?
> > >
> > > #undef RTE_GRAPH_MODEL_SELECT or not #define
> RTE_GRAPH_MODEL_SELECT
> > > anywhere in .c file.
> > >
> >
> > In the implementation, RTE_GRAPH_MODEL_SELECT is only defined once in
> > app I think #if !define(XX) is a judgement, #undef XX is an action.
> > Here should be #if !define(XX)
> > For this impl, I treat not define as default and go into rtc_walk().
> >
> >
> > So If we treat not defined RTE_GRAPH_MODEL_SELECT as runtime pick.
> > The #else case should cover: 1. Not defined and 2. Defined other type.
> > It should be as follow:
> 
> 
> Ack. We are aligned, You can send the next version. Keep my existing Acked-by
> on the patches which is already reviewed..
> I should be able to give Acked-by on the pending one to complete the review.
> 
Thanks, I will send new patch set.
> 
> >
> > rte_graph_walk()
> > {
> > #if defined(RTE_GRAPH_MODEL_SELECT) && RTE_GRAPH_MODEL_SELECT ==
> > RTE_GRAPH_MODEL_RTC
> 
> ( RTE_GRAPH_MODEL_SELECT == RTE_GRAPH_MODEL_RTC)
> 
> >         rte_graph_walk_rtc();
> >
> > #elif defined(RTE_GRAPH_MODEL_SELECT) && RTE_GRAPH_MODEL_SELECT
> ==
> > RTE_GRAPH_MODEL_MCORE_DISPATCH
> 
> (RTE_GRAPH_MODEL_SELECT == RTE_GRAPH_MODEL_MCORE_DISPATCH)
> 
> >         rte_graph_walk_mcore_dispatch(graph);
> >
> > #else
> >         const int model =;
> >
> >         switch (model) {
> 
> switch ( rte_graph_worker_model_no_check_get()) { as model not used
> anywhere else belwo , model is changing to uint8_t
> 
Got it.
> >         case RTE_GRAPH_MODEL_MCORE_DISPATCH:
> >                 rte_graph_walk_mcore_dispatch();
> >                 break;
> >         default:
> >                 rte_graph_walk_rtc();
> >         }
> > #endif
> > }
> >
> > > >
> > > > > Use bracket around RTE_GRAPH_MODEL_SELECT ==
> > > RTE_GRAPH_MODEL_RTC.
> > > > >
> > > > Ok.
> > > >
> > > > >
> > > > > >         rte_graph_walk_rtc(graph);
> > > > > > +#elif defined(RTE_GRAPH_MODEL_SELECT) &&
> > > RTE_GRAPH_MODEL_SELECT
> > > > > ==
> > > > > > +RTE_GRAPH_MODEL_MCORE_DISPATCH
> > > > >
> > > > > Use bracket around RTE_GRAPH_MODEL_SELECT ==
> > > > Ok.
> > > > >
> > > > > > +       rte_graph_walk_mcore_dispatch(graph);
> > > > > > +#else
> > > > > > +       int model = rte_graph_worker_model_get(graph);
> > > > >
> > > > > Introduce rte_graph_worker_model_no_check_get() as commented
> earlier.
> > > >
> > > > Got it.
> > > >