[v9,13/17] graph: enable graph multicore dispatch scheduler model
Checks
Commit Message
This patch enables to chose new scheduler model. Must define
RTE_GRAPH_MODEL_SELECT before including rte_graph_worker.h
to enable specific model choosing.
Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
Signed-off-by: Cunming Liang <cunming.liang@intel.com>
Signed-off-by: Zhirun Yan <zhirun.yan@intel.com>
---
lib/graph/rte_graph_worker.h | 12 ++++++++++++
1 file changed, 12 insertions(+)
Comments
On Wed, Jun 7, 2023 at 9:30 AM Zhirun Yan <zhirun.yan@intel.com> wrote:
>
> This patch enables to chose new scheduler model. Must define
> RTE_GRAPH_MODEL_SELECT before including rte_graph_worker.h
> to enable specific model choosing.
>
> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> Signed-off-by: Cunming Liang <cunming.liang@intel.com>
> Signed-off-by: Zhirun Yan <zhirun.yan@intel.com>
> ---
> lib/graph/rte_graph_worker.h | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> 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?
Use bracket around RTE_GRAPH_MODEL_SELECT == RTE_GRAPH_MODEL_RTC.
> 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 ==
> + 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.
> +
> + if (model == RTE_GRAPH_MODEL_RTC)
> + rte_graph_walk_rtc(graph);
> + else if (model == RTE_GRAPH_MODEL_MCORE_DISPATCH)
> + rte_graph_walk_mcore_dispatch(graph);
I think, switch case better to support new model in future. Please
check the performance before changing.
i.e
switch ( rte_graph_worker_model_no_check_get())
{
case RTE_GRAPH_MODEL_MCORE_DISPATCH:
rte_graph_walk_mcore_dispatch(graph)
break;
default:
rte_graph_walk_rtc(graph);
}
> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Wednesday, June 7, 2023 4:15 PM
> To: Yan, Zhirun <zhirun.yan@intel.com>
> Cc: dev@dpdk.org; jerinj@marvell.com; kirankumark@marvell.com;
> ndabilpuram@marvell.com; stephen@networkplumber.org;
> pbhagavatula@marvell.com; Liang, Cunming <cunming.liang@intel.com>; Wang,
> Haiyue <haiyue.wang@intel.com>; mattias.ronnblom
> <mattias.ronnblom@ericsson.com>
> Subject: Re: [PATCH v9 13/17] graph: enable graph multicore dispatch scheduler
> model
>
> On Wed, Jun 7, 2023 at 9:30 AM Zhirun Yan <zhirun.yan@intel.com> wrote:
> >
> > This patch enables to chose new scheduler model. Must define
> > RTE_GRAPH_MODEL_SELECT before including rte_graph_worker.h to enable
> > specific model choosing.
> >
> > Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> > Signed-off-by: Cunming Liang <cunming.liang@intel.com>
> > Signed-off-by: Zhirun Yan <zhirun.yan@intel.com>
> > ---
> > lib/graph/rte_graph_worker.h | 12 ++++++++++++
> > 1 file changed, 12 insertions(+)
> >
>
> > 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?
> 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.
> > +
> > + if (model == RTE_GRAPH_MODEL_RTC)
> > + rte_graph_walk_rtc(graph);
> > + else if (model == RTE_GRAPH_MODEL_MCORE_DISPATCH)
> > + rte_graph_walk_mcore_dispatch(graph);
>
> I think, switch case better to support new model in future. Please check the
> performance before changing.
>
Yes, I agree.
And I checked the performance with switch case and rte_graph_worker_model_no_check_get()
I get very similar performance. The improvements is <0.1%. I guess the performance impact will
be less if the workerload goes more complicated, cause the node process in walk will spent more
time.
And I will change in next version.
> i.e
>
> switch ( rte_graph_worker_model_no_check_get())
> {
> case RTE_GRAPH_MODEL_MCORE_DISPATCH:
> rte_graph_walk_mcore_dispatch(graph)
> break;
> default:
> rte_graph_walk_rtc(graph);
> }
On Wed, Jun 7, 2023 at 5:55 PM Yan, Zhirun <zhirun.yan@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.
>
> > 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.
>
> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Wednesday, June 7, 2023 9:26 PM
> To: Yan, Zhirun <zhirun.yan@intel.com>
> Cc: dev@dpdk.org; jerinj@marvell.com; kirankumark@marvell.com;
> ndabilpuram@marvell.com; stephen@networkplumber.org;
> pbhagavatula@marvell.com; Liang, Cunming <cunming.liang@intel.com>; Wang,
> Haiyue <haiyue.wang@intel.com>; mattias.ronnblom
> <mattias.ronnblom@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@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:
rte_graph_walk()
{
#if defined(RTE_GRAPH_MODEL_SELECT) && 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_walk_mcore_dispatch(graph);
#else
const int model = rte_graph_worker_model_no_check_get();
switch (model) {
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.
> >
On Thu, Jun 8, 2023 at 8:39 AM Yan, Zhirun <zhirun.yan@intel.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Jerin Jacob <jerinjacobk@gmail.com>
> > Sent: Wednesday, June 7, 2023 9:26 PM
> > To: Yan, Zhirun <zhirun.yan@intel.com>
> > Cc: dev@dpdk.org; jerinj@marvell.com; kirankumark@marvell.com;
> > ndabilpuram@marvell.com; stephen@networkplumber.org;
> > pbhagavatula@marvell.com; Liang, Cunming <cunming.liang@intel.com>; Wang,
> > Haiyue <haiyue.wang@intel.com>; mattias.ronnblom
> > <mattias.ronnblom@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@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.
>
> 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
> 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.
> > >
> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Thursday, June 8, 2023 1:34 PM
> To: Yan, Zhirun <zhirun.yan@intel.com>
> Cc: dev@dpdk.org; jerinj@marvell.com; kirankumark@marvell.com;
> ndabilpuram@marvell.com; stephen@networkplumber.org;
> pbhagavatula@marvell.com; Liang, Cunming <cunming.liang@intel.com>; Wang,
> Haiyue <haiyue.wang@intel.com>; mattias.ronnblom
> <mattias.ronnblom@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@intel.com> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > Sent: Wednesday, June 7, 2023 9:26 PM
> > > To: Yan, Zhirun <zhirun.yan@intel.com>
> > > Cc: dev@dpdk.org; jerinj@marvell.com; kirankumark@marvell.com;
> > > ndabilpuram@marvell.com; stephen@networkplumber.org;
> > > pbhagavatula@marvell.com; Liang, Cunming <cunming.liang@intel.com>;
> > > Wang, Haiyue <haiyue.wang@intel.com>; mattias.ronnblom
> > > <mattias.ronnblom@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@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.
> > > >
@@ -11,6 +11,7 @@ extern "C" {
#endif
#include "rte_graph_model_rtc.h"
+#include "rte_graph_model_mcore_dispatch.h"
/**
* Perform graph walk on the circular buffer and invoke the process function
@@ -25,7 +26,18 @@ __rte_experimental
static inline void
rte_graph_walk(struct rte_graph *graph)
{
+#if !defined(RTE_GRAPH_MODEL_SELECT) || RTE_GRAPH_MODEL_SELECT == RTE_GRAPH_MODEL_RTC
rte_graph_walk_rtc(graph);
+#elif defined(RTE_GRAPH_MODEL_SELECT) && RTE_GRAPH_MODEL_SELECT == RTE_GRAPH_MODEL_MCORE_DISPATCH
+ rte_graph_walk_mcore_dispatch(graph);
+#else
+ int model = rte_graph_worker_model_get(graph);
+
+ if (model == RTE_GRAPH_MODEL_RTC)
+ rte_graph_walk_rtc(graph);
+ else if (model == RTE_GRAPH_MODEL_MCORE_DISPATCH)
+ rte_graph_walk_mcore_dispatch(graph);
+#endif
}
#ifdef __cplusplus