diff mbox series

[API-NEXT,1/4] linux-gen: sched: remove schedule interface depedency to qentry

Message ID 20170630141056.11272-2-petri.savolainen@linaro.org
State New
Headers show
Series Clean up scheduler interface | expand

Commit Message

Petri Savolainen June 30, 2017, 2:10 p.m. UTC
Do not use queue internal type in schedule interface.

Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org>

---
 platform/linux-generic/include/odp_schedule_if.h |  8 +++--
 platform/linux-generic/odp_queue.c               |  9 ++++--
 platform/linux-generic/odp_schedule.c            | 18 ++++-------
 platform/linux-generic/odp_schedule_iquery.c     | 41 +++++++++++++-----------
 platform/linux-generic/odp_schedule_sp.c         | 18 +++--------
 5 files changed, 45 insertions(+), 49 deletions(-)

-- 
2.13.0

Comments

Honnappa Nagarahalli July 5, 2017, 4:04 a.m. UTC | #1
On 30 June 2017 at 09:10, Petri Savolainen <petri.savolainen@linaro.org> wrote:
> Do not use queue internal type in schedule interface.

>

> Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org>

> ---

>  platform/linux-generic/include/odp_schedule_if.h |  8 +++--

>  platform/linux-generic/odp_queue.c               |  9 ++++--

>  platform/linux-generic/odp_schedule.c            | 18 ++++-------

>  platform/linux-generic/odp_schedule_iquery.c     | 41 +++++++++++++-----------

>  platform/linux-generic/odp_schedule_sp.c         | 18 +++--------

>  5 files changed, 45 insertions(+), 49 deletions(-)

>

> diff --git a/platform/linux-generic/include/odp_schedule_if.h b/platform/linux-generic/include/odp_schedule_if.h

> index 5877a1cd..5abbb732 100644

> --- a/platform/linux-generic/include/odp_schedule_if.h

> +++ b/platform/linux-generic/include/odp_schedule_if.h

> @@ -35,9 +35,10 @@ typedef int (*schedule_term_local_fn_t)(void);

>  typedef void (*schedule_order_lock_fn_t)(void);

>  typedef void (*schedule_order_unlock_fn_t)(void);

>  typedef unsigned (*schedule_max_ordered_locks_fn_t)(void);

> -typedef void (*schedule_save_context_fn_t)(queue_entry_t *queue);

> +typedef void (*schedule_save_context_fn_t)(uint32_t queue_index, void *ptr);

>

>  typedef struct schedule_fn_t {

> +       int                         status_sync;


this structure should contain functions that are provided by scheduler
to other components of ODP. 'status_sync' seems to be an internal
mechanism between the default scheduler and default queue. Hence it
should not be here.

>         schedule_pktio_start_fn_t   pktio_start;

>         schedule_thr_add_fn_t       thr_add;

>         schedule_thr_rem_fn_t       thr_rem;

> @@ -45,7 +46,6 @@ typedef struct schedule_fn_t {

>         schedule_init_queue_fn_t    init_queue;

>         schedule_destroy_queue_fn_t destroy_queue;

>         schedule_sched_queue_fn_t   sched_queue;

> -       schedule_unsched_queue_fn_t unsched_queue;

these queue related functions are not used by other components within
ODP. These are specific to default queue and default schedulers. These
should not be part of this file.

>         schedule_ord_enq_multi_fn_t ord_enq_multi;

>         schedule_init_global_fn_t   init_global;

>         schedule_term_global_fn_t   term_global;

> @@ -54,7 +54,11 @@ typedef struct schedule_fn_t {

>         schedule_order_lock_fn_t    order_lock;

>         schedule_order_unlock_fn_t  order_unlock;

>         schedule_max_ordered_locks_fn_t max_ordered_locks;

> +

> +       /* Called only when status_sync is set */

> +       schedule_unsched_queue_fn_t unsched_queue;

>         schedule_save_context_fn_t  save_context;

> +

>  } schedule_fn_t;

>

>  /* Interface towards the scheduler */

> diff --git a/platform/linux-generic/odp_queue.c b/platform/linux-generic/odp_queue.c

> index 19945584..2db95fc6 100644

> --- a/platform/linux-generic/odp_queue.c

> +++ b/platform/linux-generic/odp_queue.c

> @@ -474,6 +474,7 @@ static inline int deq_multi(queue_t q_int, odp_buffer_hdr_t *buf_hdr[],

>         int i, j;

>         queue_entry_t *queue;

>         int updated = 0;

> +       int status_sync = sched_fn->status_sync;

>

>         queue = qentry_from_int(q_int);

>         LOCK(&queue->s.lock);

> @@ -490,7 +491,9 @@ static inline int deq_multi(queue_t q_int, odp_buffer_hdr_t *buf_hdr[],

>                 /* Already empty queue */

>                 if (queue->s.status == QUEUE_STATUS_SCHED) {

>                         queue->s.status = QUEUE_STATUS_NOTSCHED;

> -                       sched_fn->unsched_queue(queue->s.index);

> +

> +                       if (status_sync)

> +                               sched_fn->unsched_queue(queue->s.index);

>                 }

>

>                 UNLOCK(&queue->s.lock);

> @@ -533,8 +536,8 @@ static inline int deq_multi(queue_t q_int, odp_buffer_hdr_t *buf_hdr[],

>         if (hdr == NULL)

>                 queue->s.tail = NULL;

>

> -       if (queue->s.type == ODP_QUEUE_TYPE_SCHED)

> -               sched_fn->save_context(queue);

> +       if (status_sync && queue->s.type == ODP_QUEUE_TYPE_SCHED)

> +               sched_fn->save_context(queue->s.index, queue);

>

>         UNLOCK(&queue->s.lock);

>

> diff --git a/platform/linux-generic/odp_schedule.c b/platform/linux-generic/odp_schedule.c

> index 814746c7..69de7ac0 100644

> --- a/platform/linux-generic/odp_schedule.c

> +++ b/platform/linux-generic/odp_schedule.c

> @@ -22,9 +22,11 @@

>  #include <odp/api/sync.h>

>  #include <odp/api/packet_io.h>

>  #include <odp_ring_internal.h>

> -#include <odp_queue_internal.h>

>  #include <odp_timer_internal.h>

>

> +/* Should remove this dependency */

> +#include <odp_queue_internal.h>

> +

>  /* Number of priority levels  */

>  #define NUM_PRIO 8

>

> @@ -1340,22 +1342,14 @@ static int schedule_sched_queue(uint32_t queue_index)

>         return 0;

>  }

>

> -static int schedule_unsched_queue(uint32_t queue_index ODP_UNUSED)

> -{

> -       return 0;

> -}

> -

>  static int schedule_num_grps(void)

>  {

>         return NUM_SCHED_GRPS;

>  }

>

> -static void schedule_save_context(queue_entry_t *queue ODP_UNUSED)

> -{

> -}

> -

>  /* Fill in scheduler interface */

>  const schedule_fn_t schedule_default_fn = {

> +       .status_sync = 0,

>         .pktio_start = schedule_pktio_start,

>         .thr_add = schedule_thr_add,

>         .thr_rem = schedule_thr_rem,

> @@ -1363,7 +1357,6 @@ const schedule_fn_t schedule_default_fn = {

>         .init_queue = schedule_init_queue,

>         .destroy_queue = schedule_destroy_queue,

>         .sched_queue = schedule_sched_queue,

> -       .unsched_queue = schedule_unsched_queue,

>         .ord_enq_multi = schedule_ord_enq_multi,

>         .init_global = schedule_init_global,

>         .term_global = schedule_term_global,

> @@ -1372,7 +1365,8 @@ const schedule_fn_t schedule_default_fn = {

>         .order_lock = order_lock,

>         .order_unlock = order_unlock,

>         .max_ordered_locks = schedule_max_ordered_locks,

> -       .save_context = schedule_save_context

> +       .unsched_queue = NULL,

> +       .save_context = NULL

>  };

>

>  /* Fill in scheduler API calls */

> diff --git a/platform/linux-generic/odp_schedule_iquery.c b/platform/linux-generic/odp_schedule_iquery.c

> index b33ba7cd..75f56e63 100644

> --- a/platform/linux-generic/odp_schedule_iquery.c

> +++ b/platform/linux-generic/odp_schedule_iquery.c

> @@ -12,7 +12,6 @@

>  #include <odp_internal.h>

>  #include <odp_debug_internal.h>

>  #include <odp_ring_internal.h>

> -#include <odp_queue_internal.h>

>  #include <odp_buffer_internal.h>

>  #include <odp_bitmap_internal.h>

>  #include <odp/api/thread.h>

> @@ -25,6 +24,9 @@

>  #include <odp_config_internal.h>

>  #include <odp_timer_internal.h>

>

> +/* Should remove this dependency */

> +#include <odp_queue_internal.h>

> +

>  /* Number of priority levels */

>  #define NUM_SCHED_PRIO 8

>

> @@ -1267,11 +1269,23 @@ static unsigned schedule_max_ordered_locks(void)

>         return MAX_ORDERED_LOCKS_PER_QUEUE;

>  }

>

> -static void schedule_save_context(queue_entry_t *queue)

> +static inline bool is_atomic_queue(unsigned int queue_index)

> +{

> +       return (sched->queues[queue_index].sync == ODP_SCHED_SYNC_ATOMIC);

> +}

> +

> +static inline bool is_ordered_queue(unsigned int queue_index)

> +{

> +       return (sched->queues[queue_index].sync == ODP_SCHED_SYNC_ORDERED);

> +}

> +

> +static void schedule_save_context(uint32_t queue_index, void *ptr)

>  {

> -       if (queue->s.param.sched.sync == ODP_SCHED_SYNC_ATOMIC) {

> -               thread_local.atomic = &sched->availables[queue->s.index];

> -       } else if (queue->s.param.sched.sync == ODP_SCHED_SYNC_ORDERED) {

> +       queue_entry_t *queue = ptr;

> +

> +       if (is_atomic_queue(queue_index)) {

> +               thread_local.atomic = &sched->availables[queue_index];

> +       } else if (is_ordered_queue(queue_index)) {

>                 uint64_t ctx;

>                 odp_atomic_u64_t *next_ctx;

>

> @@ -1285,6 +1299,7 @@ static void schedule_save_context(queue_entry_t *queue)

>

>  /* Fill in scheduler interface */

>  const schedule_fn_t schedule_iquery_fn = {

> +       .status_sync   = 1,

>         .pktio_start   = schedule_pktio_start,

>         .thr_add       = group_add_thread,

>         .thr_rem       = group_remove_thread,

> @@ -1292,7 +1307,6 @@ const schedule_fn_t schedule_iquery_fn = {

>         .init_queue    = init_sched_queue,

>         .destroy_queue = destroy_sched_queue,

>         .sched_queue   = schedule_sched_queue,

> -       .unsched_queue = schedule_unsched_queue,

>         .ord_enq_multi = schedule_ord_enq_multi,

>         .init_global   = schedule_init_global,

>         .term_global   = schedule_term_global,

> @@ -1301,7 +1315,8 @@ const schedule_fn_t schedule_iquery_fn = {

>         .order_lock    = order_lock,

>         .order_unlock  = order_unlock,

>         .max_ordered_locks = schedule_max_ordered_locks,

> -       .save_context  = schedule_save_context,

> +       .unsched_queue = schedule_unsched_queue,

> +       .save_context  = schedule_save_context

>  };

>

>  /* Fill in scheduler API calls */

> @@ -1428,18 +1443,6 @@ static void thread_clear_interests(sched_thread_local_t *thread,

>         }

>  }

>

> -static inline bool is_atomic_queue(unsigned int queue_index)

> -{

> -       return (sched->queues[queue_index].sync

> -                       == ODP_SCHED_SYNC_ATOMIC);

> -}

> -

> -static inline bool is_ordered_queue(unsigned int queue_index)

> -{

> -       return (sched->queues[queue_index].sync

> -                       == ODP_SCHED_SYNC_ORDERED);

> -}

> -

>  static inline bool compete_atomic_queue(unsigned int queue_index)

>  {

>         bool expected = sched->availables[queue_index];

> diff --git a/platform/linux-generic/odp_schedule_sp.c b/platform/linux-generic/odp_schedule_sp.c

> index 14b7aa88..9829acc5 100644

> --- a/platform/linux-generic/odp_schedule_sp.c

> +++ b/platform/linux-generic/odp_schedule_sp.c

> @@ -410,11 +410,6 @@ static int sched_queue(uint32_t qi)

>         return 0;

>  }

>

> -static int unsched_queue(uint32_t qi ODP_UNUSED)

> -{

> -       return 0;

> -}

> -

>  static int ord_enq_multi(queue_t q_int, void *buf_hdr[], int num,

>                          int *ret)

>  {

> @@ -830,12 +825,9 @@ static void order_unlock(void)

>  {

>  }

>

> -static void save_context(queue_entry_t *queue ODP_UNUSED)

> -{

> -}

> -

>  /* Fill in scheduler interface */

>  const schedule_fn_t schedule_sp_fn = {

> +       .status_sync   = 0,

>         .pktio_start   = pktio_start,

>         .thr_add       = thr_add,

>         .thr_rem       = thr_rem,

> @@ -843,16 +835,16 @@ const schedule_fn_t schedule_sp_fn = {

>         .init_queue    = init_queue,

>         .destroy_queue = destroy_queue,

>         .sched_queue   = sched_queue,

> -       .unsched_queue = unsched_queue,

>         .ord_enq_multi = ord_enq_multi,

>         .init_global   = init_global,

>         .term_global   = term_global,

>         .init_local    = init_local,

>         .term_local    = term_local,

> -       .order_lock =    order_lock,

> -       .order_unlock =  order_unlock,

> +       .order_lock    = order_lock,

> +       .order_unlock  = order_unlock,

>         .max_ordered_locks = max_ordered_locks,

> -       .save_context  = save_context

> +       .unsched_queue = NULL,

> +       .save_context  = NULL

>  };

>

>  /* Fill in scheduler API calls */

> --

> 2.13.0

>
Savolainen, Petri (Nokia - FI/Espoo) July 5, 2017, 6:31 a.m. UTC | #2
> -----Original Message-----

> From: Honnappa Nagarahalli [mailto:honnappa.nagarahalli@linaro.org]

> Sent: Wednesday, July 05, 2017 7:04 AM

> To: Petri Savolainen <petri.savolainen@linaro.org>

> Cc: lng-odp-forward <lng-odp@lists.linaro.org>

> Subject: Re: [lng-odp] [API-NEXT PATCH 1/4] linux-gen: sched: remove

> schedule interface depedency to qentry

> 

> On 30 June 2017 at 09:10, Petri Savolainen <petri.savolainen@linaro.org>

> wrote:

> > Do not use queue internal type in schedule interface.

> >

> > Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org>

> > ---

> >  platform/linux-generic/include/odp_schedule_if.h |  8 +++--

> >  platform/linux-generic/odp_queue.c               |  9 ++++--

> >  platform/linux-generic/odp_schedule.c            | 18 ++++-------

> >  platform/linux-generic/odp_schedule_iquery.c     | 41 +++++++++++++----

> -------

> >  platform/linux-generic/odp_schedule_sp.c         | 18 +++--------

> >  5 files changed, 45 insertions(+), 49 deletions(-)

> >

> > diff --git a/platform/linux-generic/include/odp_schedule_if.h

> b/platform/linux-generic/include/odp_schedule_if.h

> > index 5877a1cd..5abbb732 100644

> > --- a/platform/linux-generic/include/odp_schedule_if.h

> > +++ b/platform/linux-generic/include/odp_schedule_if.h

> > @@ -35,9 +35,10 @@ typedef int (*schedule_term_local_fn_t)(void);

> >  typedef void (*schedule_order_lock_fn_t)(void);

> >  typedef void (*schedule_order_unlock_fn_t)(void);

> >  typedef unsigned (*schedule_max_ordered_locks_fn_t)(void);

> > -typedef void (*schedule_save_context_fn_t)(queue_entry_t *queue);

> > +typedef void (*schedule_save_context_fn_t)(uint32_t queue_index, void

> *ptr);

> >

> >  typedef struct schedule_fn_t {

> > +       int                         status_sync;

> 

> this structure should contain functions that are provided by scheduler

> to other components of ODP. 'status_sync' seems to be an internal

> mechanism between the default scheduler and default queue. Hence it

> should not be here.


This flags if unsched_queue() and save_context() needs to be called. Those calls are only needed by iquery scheduler. With this flag, queue needs to check only single variable if those calls are needed or not. Today, these calls are made always, which hurt performance.

> 

> >         schedule_pktio_start_fn_t   pktio_start;

> >         schedule_thr_add_fn_t       thr_add;

> >         schedule_thr_rem_fn_t       thr_rem;

> > @@ -45,7 +46,6 @@ typedef struct schedule_fn_t {

> >         schedule_init_queue_fn_t    init_queue;

> >         schedule_destroy_queue_fn_t destroy_queue;

> >         schedule_sched_queue_fn_t   sched_queue;

> > -       schedule_unsched_queue_fn_t unsched_queue;

> these queue related functions are not used by other components within

> ODP. These are specific to default queue and default schedulers. These

> should not be part of this file.


Didn't add or remove those functions in this patch set. This discussion can be done in context of another patch set.

-Petri

> 

> >         schedule_ord_enq_multi_fn_t ord_enq_multi;

> >         schedule_init_global_fn_t   init_global;

> >         schedule_term_global_fn_t   term_global;

> > @@ -54,7 +54,11 @@ typedef struct schedule_fn_t {

> >         schedule_order_lock_fn_t    order_lock;

> >         schedule_order_unlock_fn_t  order_unlock;

> >         schedule_max_ordered_locks_fn_t max_ordered_locks;

> > +

> > +       /* Called only when status_sync is set */

> > +       schedule_unsched_queue_fn_t unsched_queue;

> >         schedule_save_context_fn_t  save_context;

> > +

> >  } schedule_fn_t;
Honnappa Nagarahalli July 6, 2017, 8:07 p.m. UTC | #3
On 5 July 2017 at 01:31, Savolainen, Petri (Nokia - FI/Espoo)
<petri.savolainen@nokia.com> wrote:
>

>

>> -----Original Message-----

>> From: Honnappa Nagarahalli [mailto:honnappa.nagarahalli@linaro.org]

>> Sent: Wednesday, July 05, 2017 7:04 AM

>> To: Petri Savolainen <petri.savolainen@linaro.org>

>> Cc: lng-odp-forward <lng-odp@lists.linaro.org>

>> Subject: Re: [lng-odp] [API-NEXT PATCH 1/4] linux-gen: sched: remove

>> schedule interface depedency to qentry

>>

>> On 30 June 2017 at 09:10, Petri Savolainen <petri.savolainen@linaro.org>

>> wrote:

>> > Do not use queue internal type in schedule interface.

>> >

>> > Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org>

>> > ---

>> >  platform/linux-generic/include/odp_schedule_if.h |  8 +++--

>> >  platform/linux-generic/odp_queue.c               |  9 ++++--

>> >  platform/linux-generic/odp_schedule.c            | 18 ++++-------

>> >  platform/linux-generic/odp_schedule_iquery.c     | 41 +++++++++++++----

>> -------

>> >  platform/linux-generic/odp_schedule_sp.c         | 18 +++--------

>> >  5 files changed, 45 insertions(+), 49 deletions(-)

>> >

>> > diff --git a/platform/linux-generic/include/odp_schedule_if.h

>> b/platform/linux-generic/include/odp_schedule_if.h

>> > index 5877a1cd..5abbb732 100644

>> > --- a/platform/linux-generic/include/odp_schedule_if.h

>> > +++ b/platform/linux-generic/include/odp_schedule_if.h

>> > @@ -35,9 +35,10 @@ typedef int (*schedule_term_local_fn_t)(void);

>> >  typedef void (*schedule_order_lock_fn_t)(void);

>> >  typedef void (*schedule_order_unlock_fn_t)(void);

>> >  typedef unsigned (*schedule_max_ordered_locks_fn_t)(void);

>> > -typedef void (*schedule_save_context_fn_t)(queue_entry_t *queue);

>> > +typedef void (*schedule_save_context_fn_t)(uint32_t queue_index, void

>> *ptr);

>> >

>> >  typedef struct schedule_fn_t {

>> > +       int                         status_sync;

>>

>> this structure should contain functions that are provided by scheduler

>> to other components of ODP. 'status_sync' seems to be an internal

>> mechanism between the default scheduler and default queue. Hence it

>> should not be here.

>

> This flags if unsched_queue() and save_context() needs to be called. Those calls are only needed by iquery scheduler. With this flag, queue needs to check only single variable if those calls are needed or not. Today, these calls are made always, which hurt performance.

>

>>

>> >         schedule_pktio_start_fn_t   pktio_start;

>> >         schedule_thr_add_fn_t       thr_add;

>> >         schedule_thr_rem_fn_t       thr_rem;

>> > @@ -45,7 +46,6 @@ typedef struct schedule_fn_t {

>> >         schedule_init_queue_fn_t    init_queue;

>> >         schedule_destroy_queue_fn_t destroy_queue;

>> >         schedule_sched_queue_fn_t   sched_queue;

>> > -       schedule_unsched_queue_fn_t unsched_queue;

>> these queue related functions are not used by other components within

>> ODP. These are specific to default queue and default schedulers. These

>> should not be part of this file.

>

> Didn't add or remove those functions in this patch set. This discussion can be done in context of another patch set.

>

Are you just cleaning up the scheduler and queue interface in this
patch? If not this change can be taken up in this patch.

> -Petri

>

>>

>> >         schedule_ord_enq_multi_fn_t ord_enq_multi;

>> >         schedule_init_global_fn_t   init_global;

>> >         schedule_term_global_fn_t   term_global;

>> > @@ -54,7 +54,11 @@ typedef struct schedule_fn_t {

>> >         schedule_order_lock_fn_t    order_lock;

>> >         schedule_order_unlock_fn_t  order_unlock;

>> >         schedule_max_ordered_locks_fn_t max_ordered_locks;

>> > +

>> > +       /* Called only when status_sync is set */

>> > +       schedule_unsched_queue_fn_t unsched_queue;

>> >         schedule_save_context_fn_t  save_context;

>> > +

>> >  } schedule_fn_t;
Honnappa Nagarahalli July 6, 2017, 8:09 p.m. UTC | #4
On 4 July 2017 at 23:04, Honnappa Nagarahalli
<honnappa.nagarahalli@linaro.org> wrote:
> On 30 June 2017 at 09:10, Petri Savolainen <petri.savolainen@linaro.org> wrote:

>> Do not use queue internal type in schedule interface.

>>

>> Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org>

>> ---

>>  platform/linux-generic/include/odp_schedule_if.h |  8 +++--

>>  platform/linux-generic/odp_queue.c               |  9 ++++--

>>  platform/linux-generic/odp_schedule.c            | 18 ++++-------

>>  platform/linux-generic/odp_schedule_iquery.c     | 41 +++++++++++++-----------

>>  platform/linux-generic/odp_schedule_sp.c         | 18 +++--------

>>  5 files changed, 45 insertions(+), 49 deletions(-)

>>

>> diff --git a/platform/linux-generic/include/odp_schedule_if.h b/platform/linux-generic/include/odp_schedule_if.h

>> index 5877a1cd..5abbb732 100644

>> --- a/platform/linux-generic/include/odp_schedule_if.h

>> +++ b/platform/linux-generic/include/odp_schedule_if.h

>> @@ -35,9 +35,10 @@ typedef int (*schedule_term_local_fn_t)(void);

>>  typedef void (*schedule_order_lock_fn_t)(void);

>>  typedef void (*schedule_order_unlock_fn_t)(void);

>>  typedef unsigned (*schedule_max_ordered_locks_fn_t)(void);

>> -typedef void (*schedule_save_context_fn_t)(queue_entry_t *queue);

>> +typedef void (*schedule_save_context_fn_t)(uint32_t queue_index, void *ptr);

>>

>>  typedef struct schedule_fn_t {

>> +       int                         status_sync;

>

> this structure should contain functions that are provided by scheduler

> to other components of ODP. 'status_sync' seems to be an internal

> mechanism between the default scheduler and default queue. Hence it

> should not be here.

>

Any update on this comment?

>>         schedule_pktio_start_fn_t   pktio_start;

>>         schedule_thr_add_fn_t       thr_add;

>>         schedule_thr_rem_fn_t       thr_rem;

>> @@ -45,7 +46,6 @@ typedef struct schedule_fn_t {

>>         schedule_init_queue_fn_t    init_queue;

>>         schedule_destroy_queue_fn_t destroy_queue;

>>         schedule_sched_queue_fn_t   sched_queue;

>> -       schedule_unsched_queue_fn_t unsched_queue;

> these queue related functions are not used by other components within

> ODP. These are specific to default queue and default schedulers. These

> should not be part of this file.

>

>>         schedule_ord_enq_multi_fn_t ord_enq_multi;

>>         schedule_init_global_fn_t   init_global;

>>         schedule_term_global_fn_t   term_global;

>> @@ -54,7 +54,11 @@ typedef struct schedule_fn_t {

>>         schedule_order_lock_fn_t    order_lock;

>>         schedule_order_unlock_fn_t  order_unlock;

>>         schedule_max_ordered_locks_fn_t max_ordered_locks;

>> +

>> +       /* Called only when status_sync is set */

>> +       schedule_unsched_queue_fn_t unsched_queue;

>>         schedule_save_context_fn_t  save_context;

>> +

>>  } schedule_fn_t;

>>

>>  /* Interface towards the scheduler */

>> diff --git a/platform/linux-generic/odp_queue.c b/platform/linux-generic/odp_queue.c

>> index 19945584..2db95fc6 100644

>> --- a/platform/linux-generic/odp_queue.c

>> +++ b/platform/linux-generic/odp_queue.c

>> @@ -474,6 +474,7 @@ static inline int deq_multi(queue_t q_int, odp_buffer_hdr_t *buf_hdr[],

>>         int i, j;

>>         queue_entry_t *queue;

>>         int updated = 0;

>> +       int status_sync = sched_fn->status_sync;

>>

>>         queue = qentry_from_int(q_int);

>>         LOCK(&queue->s.lock);

>> @@ -490,7 +491,9 @@ static inline int deq_multi(queue_t q_int, odp_buffer_hdr_t *buf_hdr[],

>>                 /* Already empty queue */

>>                 if (queue->s.status == QUEUE_STATUS_SCHED) {

>>                         queue->s.status = QUEUE_STATUS_NOTSCHED;

>> -                       sched_fn->unsched_queue(queue->s.index);

>> +

>> +                       if (status_sync)

>> +                               sched_fn->unsched_queue(queue->s.index);

>>                 }

>>

>>                 UNLOCK(&queue->s.lock);

>> @@ -533,8 +536,8 @@ static inline int deq_multi(queue_t q_int, odp_buffer_hdr_t *buf_hdr[],

>>         if (hdr == NULL)

>>                 queue->s.tail = NULL;

>>

>> -       if (queue->s.type == ODP_QUEUE_TYPE_SCHED)

>> -               sched_fn->save_context(queue);

>> +       if (status_sync && queue->s.type == ODP_QUEUE_TYPE_SCHED)

>> +               sched_fn->save_context(queue->s.index, queue);

>>

>>         UNLOCK(&queue->s.lock);

>>

>> diff --git a/platform/linux-generic/odp_schedule.c b/platform/linux-generic/odp_schedule.c

>> index 814746c7..69de7ac0 100644

>> --- a/platform/linux-generic/odp_schedule.c

>> +++ b/platform/linux-generic/odp_schedule.c

>> @@ -22,9 +22,11 @@

>>  #include <odp/api/sync.h>

>>  #include <odp/api/packet_io.h>

>>  #include <odp_ring_internal.h>

>> -#include <odp_queue_internal.h>

>>  #include <odp_timer_internal.h>

>>

>> +/* Should remove this dependency */

>> +#include <odp_queue_internal.h>

>> +

>>  /* Number of priority levels  */

>>  #define NUM_PRIO 8

>>

>> @@ -1340,22 +1342,14 @@ static int schedule_sched_queue(uint32_t queue_index)

>>         return 0;

>>  }

>>

>> -static int schedule_unsched_queue(uint32_t queue_index ODP_UNUSED)

>> -{

>> -       return 0;

>> -}

>> -

>>  static int schedule_num_grps(void)

>>  {

>>         return NUM_SCHED_GRPS;

>>  }

>>

>> -static void schedule_save_context(queue_entry_t *queue ODP_UNUSED)

>> -{

>> -}

>> -

>>  /* Fill in scheduler interface */

>>  const schedule_fn_t schedule_default_fn = {

>> +       .status_sync = 0,

>>         .pktio_start = schedule_pktio_start,

>>         .thr_add = schedule_thr_add,

>>         .thr_rem = schedule_thr_rem,

>> @@ -1363,7 +1357,6 @@ const schedule_fn_t schedule_default_fn = {

>>         .init_queue = schedule_init_queue,

>>         .destroy_queue = schedule_destroy_queue,

>>         .sched_queue = schedule_sched_queue,

>> -       .unsched_queue = schedule_unsched_queue,

>>         .ord_enq_multi = schedule_ord_enq_multi,

>>         .init_global = schedule_init_global,

>>         .term_global = schedule_term_global,

>> @@ -1372,7 +1365,8 @@ const schedule_fn_t schedule_default_fn = {

>>         .order_lock = order_lock,

>>         .order_unlock = order_unlock,

>>         .max_ordered_locks = schedule_max_ordered_locks,

>> -       .save_context = schedule_save_context

>> +       .unsched_queue = NULL,

>> +       .save_context = NULL

>>  };

>>

>>  /* Fill in scheduler API calls */

>> diff --git a/platform/linux-generic/odp_schedule_iquery.c b/platform/linux-generic/odp_schedule_iquery.c

>> index b33ba7cd..75f56e63 100644

>> --- a/platform/linux-generic/odp_schedule_iquery.c

>> +++ b/platform/linux-generic/odp_schedule_iquery.c

>> @@ -12,7 +12,6 @@

>>  #include <odp_internal.h>

>>  #include <odp_debug_internal.h>

>>  #include <odp_ring_internal.h>

>> -#include <odp_queue_internal.h>

>>  #include <odp_buffer_internal.h>

>>  #include <odp_bitmap_internal.h>

>>  #include <odp/api/thread.h>

>> @@ -25,6 +24,9 @@

>>  #include <odp_config_internal.h>

>>  #include <odp_timer_internal.h>

>>

>> +/* Should remove this dependency */

>> +#include <odp_queue_internal.h>

>> +

>>  /* Number of priority levels */

>>  #define NUM_SCHED_PRIO 8

>>

>> @@ -1267,11 +1269,23 @@ static unsigned schedule_max_ordered_locks(void)

>>         return MAX_ORDERED_LOCKS_PER_QUEUE;

>>  }

>>

>> -static void schedule_save_context(queue_entry_t *queue)

>> +static inline bool is_atomic_queue(unsigned int queue_index)

>> +{

>> +       return (sched->queues[queue_index].sync == ODP_SCHED_SYNC_ATOMIC);

>> +}

>> +

>> +static inline bool is_ordered_queue(unsigned int queue_index)

>> +{

>> +       return (sched->queues[queue_index].sync == ODP_SCHED_SYNC_ORDERED);

>> +}

>> +

>> +static void schedule_save_context(uint32_t queue_index, void *ptr)

>>  {

>> -       if (queue->s.param.sched.sync == ODP_SCHED_SYNC_ATOMIC) {

>> -               thread_local.atomic = &sched->availables[queue->s.index];

>> -       } else if (queue->s.param.sched.sync == ODP_SCHED_SYNC_ORDERED) {

>> +       queue_entry_t *queue = ptr;

>> +

>> +       if (is_atomic_queue(queue_index)) {

>> +               thread_local.atomic = &sched->availables[queue_index];

>> +       } else if (is_ordered_queue(queue_index)) {

>>                 uint64_t ctx;

>>                 odp_atomic_u64_t *next_ctx;

>>

>> @@ -1285,6 +1299,7 @@ static void schedule_save_context(queue_entry_t *queue)

>>

>>  /* Fill in scheduler interface */

>>  const schedule_fn_t schedule_iquery_fn = {

>> +       .status_sync   = 1,

>>         .pktio_start   = schedule_pktio_start,

>>         .thr_add       = group_add_thread,

>>         .thr_rem       = group_remove_thread,

>> @@ -1292,7 +1307,6 @@ const schedule_fn_t schedule_iquery_fn = {

>>         .init_queue    = init_sched_queue,

>>         .destroy_queue = destroy_sched_queue,

>>         .sched_queue   = schedule_sched_queue,

>> -       .unsched_queue = schedule_unsched_queue,

>>         .ord_enq_multi = schedule_ord_enq_multi,

>>         .init_global   = schedule_init_global,

>>         .term_global   = schedule_term_global,

>> @@ -1301,7 +1315,8 @@ const schedule_fn_t schedule_iquery_fn = {

>>         .order_lock    = order_lock,

>>         .order_unlock  = order_unlock,

>>         .max_ordered_locks = schedule_max_ordered_locks,

>> -       .save_context  = schedule_save_context,

>> +       .unsched_queue = schedule_unsched_queue,

>> +       .save_context  = schedule_save_context

>>  };

>>

>>  /* Fill in scheduler API calls */

>> @@ -1428,18 +1443,6 @@ static void thread_clear_interests(sched_thread_local_t *thread,

>>         }

>>  }

>>

>> -static inline bool is_atomic_queue(unsigned int queue_index)

>> -{

>> -       return (sched->queues[queue_index].sync

>> -                       == ODP_SCHED_SYNC_ATOMIC);

>> -}

>> -

>> -static inline bool is_ordered_queue(unsigned int queue_index)

>> -{

>> -       return (sched->queues[queue_index].sync

>> -                       == ODP_SCHED_SYNC_ORDERED);

>> -}

>> -

>>  static inline bool compete_atomic_queue(unsigned int queue_index)

>>  {

>>         bool expected = sched->availables[queue_index];

>> diff --git a/platform/linux-generic/odp_schedule_sp.c b/platform/linux-generic/odp_schedule_sp.c

>> index 14b7aa88..9829acc5 100644

>> --- a/platform/linux-generic/odp_schedule_sp.c

>> +++ b/platform/linux-generic/odp_schedule_sp.c

>> @@ -410,11 +410,6 @@ static int sched_queue(uint32_t qi)

>>         return 0;

>>  }

>>

>> -static int unsched_queue(uint32_t qi ODP_UNUSED)

>> -{

>> -       return 0;

>> -}

>> -

>>  static int ord_enq_multi(queue_t q_int, void *buf_hdr[], int num,

>>                          int *ret)

>>  {

>> @@ -830,12 +825,9 @@ static void order_unlock(void)

>>  {

>>  }

>>

>> -static void save_context(queue_entry_t *queue ODP_UNUSED)

>> -{

>> -}

>> -

>>  /* Fill in scheduler interface */

>>  const schedule_fn_t schedule_sp_fn = {

>> +       .status_sync   = 0,

>>         .pktio_start   = pktio_start,

>>         .thr_add       = thr_add,

>>         .thr_rem       = thr_rem,

>> @@ -843,16 +835,16 @@ const schedule_fn_t schedule_sp_fn = {

>>         .init_queue    = init_queue,

>>         .destroy_queue = destroy_queue,

>>         .sched_queue   = sched_queue,

>> -       .unsched_queue = unsched_queue,

>>         .ord_enq_multi = ord_enq_multi,

>>         .init_global   = init_global,

>>         .term_global   = term_global,

>>         .init_local    = init_local,

>>         .term_local    = term_local,

>> -       .order_lock =    order_lock,

>> -       .order_unlock =  order_unlock,

>> +       .order_lock    = order_lock,

>> +       .order_unlock  = order_unlock,

>>         .max_ordered_locks = max_ordered_locks,

>> -       .save_context  = save_context

>> +       .unsched_queue = NULL,

>> +       .save_context  = NULL

>>  };

>>

>>  /* Fill in scheduler API calls */

>> --

>> 2.13.0

>>
Savolainen, Petri (Nokia - FI/Espoo) July 7, 2017, 6:46 a.m. UTC | #5
> -----Original Message-----

> From: Honnappa Nagarahalli [mailto:honnappa.nagarahalli@linaro.org]

> Sent: Thursday, July 06, 2017 11:07 PM

> To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia.com>

> Cc: lng-odp-forward <lng-odp@lists.linaro.org>

> Subject: Re: [lng-odp] [API-NEXT PATCH 1/4] linux-gen: sched: remove

> schedule interface depedency to qentry

> 

> On 5 July 2017 at 01:31, Savolainen, Petri (Nokia - FI/Espoo)

> <petri.savolainen@nokia.com> wrote:

> >

> >

> >> -----Original Message-----

> >> From: Honnappa Nagarahalli [mailto:honnappa.nagarahalli@linaro.org]

> >> Sent: Wednesday, July 05, 2017 7:04 AM

> >> To: Petri Savolainen <petri.savolainen@linaro.org>

> >> Cc: lng-odp-forward <lng-odp@lists.linaro.org>

> >> Subject: Re: [lng-odp] [API-NEXT PATCH 1/4] linux-gen: sched: remove

> >> schedule interface depedency to qentry

> >>

> >> On 30 June 2017 at 09:10, Petri Savolainen

> <petri.savolainen@linaro.org>

> >> wrote:

> >> > Do not use queue internal type in schedule interface.

> >> >

> >> > Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org>

> >> > ---

> >> >  platform/linux-generic/include/odp_schedule_if.h |  8 +++--

> >> >  platform/linux-generic/odp_queue.c               |  9 ++++--

> >> >  platform/linux-generic/odp_schedule.c            | 18 ++++-------

> >> >  platform/linux-generic/odp_schedule_iquery.c     | 41 +++++++++++++-

> ---

> >> -------

> >> >  platform/linux-generic/odp_schedule_sp.c         | 18 +++--------

> >> >  5 files changed, 45 insertions(+), 49 deletions(-)

> >> >

> >> > diff --git a/platform/linux-generic/include/odp_schedule_if.h

> >> b/platform/linux-generic/include/odp_schedule_if.h

> >> > index 5877a1cd..5abbb732 100644

> >> > --- a/platform/linux-generic/include/odp_schedule_if.h

> >> > +++ b/platform/linux-generic/include/odp_schedule_if.h

> >> > @@ -35,9 +35,10 @@ typedef int (*schedule_term_local_fn_t)(void);

> >> >  typedef void (*schedule_order_lock_fn_t)(void);

> >> >  typedef void (*schedule_order_unlock_fn_t)(void);

> >> >  typedef unsigned (*schedule_max_ordered_locks_fn_t)(void);

> >> > -typedef void (*schedule_save_context_fn_t)(queue_entry_t *queue);

> >> > +typedef void (*schedule_save_context_fn_t)(uint32_t queue_index,

> void

> >> *ptr);

> >> >

> >> >  typedef struct schedule_fn_t {

> >> > +       int                         status_sync;

> >>

> >> this structure should contain functions that are provided by scheduler

> >> to other components of ODP. 'status_sync' seems to be an internal

> >> mechanism between the default scheduler and default queue. Hence it

> >> should not be here.

> >

> > This flags if unsched_queue() and save_context() needs to be called.

> Those calls are only needed by iquery scheduler. With this flag, queue

> needs to check only single variable if those calls are needed or not.

> Today, these calls are made always, which hurt performance.

> >

> >>

> >> >         schedule_pktio_start_fn_t   pktio_start;

> >> >         schedule_thr_add_fn_t       thr_add;

> >> >         schedule_thr_rem_fn_t       thr_rem;

> >> > @@ -45,7 +46,6 @@ typedef struct schedule_fn_t {

> >> >         schedule_init_queue_fn_t    init_queue;

> >> >         schedule_destroy_queue_fn_t destroy_queue;

> >> >         schedule_sched_queue_fn_t   sched_queue;

> >> > -       schedule_unsched_queue_fn_t unsched_queue;

> >> these queue related functions are not used by other components within

> >> ODP. These are specific to default queue and default schedulers. These

> >> should not be part of this file.

> >

> > Didn't add or remove those functions in this patch set. This discussion

> can be done in context of another patch set.

> >

> Are you just cleaning up the scheduler and queue interface in this

> patch? If not this change can be taken up in this patch.


This patch set cleans odp_schedule_if.h interface, which currently defines input and output interfaces for the scheduler. This set does not redesign the interface but cleans and optimizes the usage. The main point is to remove type dependencies, functions and function calls that are not needed.

These functions above are needed only by iquery. This change removes empty function defines and extra calls when iquery is not used. One option to get rid-off these functions is to remove iquery if it's not developed anymore.

-Petri
Savolainen, Petri (Nokia - FI/Espoo) July 7, 2017, 6:46 a.m. UTC | #6
> >>  typedef struct schedule_fn_t {

> >> +       int                         status_sync;

> >

> > this structure should contain functions that are provided by scheduler

> > to other components of ODP. 'status_sync' seems to be an internal

> > mechanism between the default scheduler and default queue. Hence it

> > should not be here.

> >

> Any update on this comment?


I did answer it already.
Maxim Uvarov July 7, 2017, 7:13 a.m. UTC | #7
Hello Honnappa, it looks like there is no blocker issue to merge these
patches. Improvements can be done later and clean up can go in now. Do you
agree?

Maxim.

On 7 July 2017 at 09:46, Savolainen, Petri (Nokia - FI/Espoo) <
petri.savolainen@nokia.com> wrote:

> > >>  typedef struct schedule_fn_t {

> > >> +       int                         status_sync;

> > >

> > > this structure should contain functions that are provided by scheduler

> > > to other components of ODP. 'status_sync' seems to be an internal

> > > mechanism between the default scheduler and default queue. Hence it

> > > should not be here.

> > >

> > Any update on this comment?

>

> I did answer it already.

>
Honnappa Nagarahalli July 7, 2017, 6:27 p.m. UTC | #8
On 7 July 2017 at 01:46, Savolainen, Petri (Nokia - FI/Espoo)
<petri.savolainen@nokia.com> wrote:
>> >>  typedef struct schedule_fn_t {

>> >> +       int                         status_sync;

>> >

>> > this structure should contain functions that are provided by scheduler

>> > to other components of ODP. 'status_sync' seems to be an internal

>> > mechanism between the default scheduler and default queue. Hence it

>> > should not be here.

>> >

>> Any update on this comment?

>

> I did answer it already.


Ok, found your answer. Should this variable be moved to queue internal
structure which is set only for iQuery scheduler?

This structure should contain only the functions exposed by the
scheduler to other components of ODP. It should not contain anything
related to the interface between queue and scheduler (they are being
considered as a single module).
Savolainen, Petri (Nokia - FI/Espoo) July 10, 2017, 8:05 a.m. UTC | #9
> -----Original Message-----

> From: Honnappa Nagarahalli [mailto:honnappa.nagarahalli@linaro.org]

> Sent: Friday, July 07, 2017 9:28 PM

> To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia.com>

> Cc: lng-odp-forward <lng-odp@lists.linaro.org>

> Subject: Re: [lng-odp] [API-NEXT PATCH 1/4] linux-gen: sched: remove

> schedule interface depedency to qentry

> 

> On 7 July 2017 at 01:46, Savolainen, Petri (Nokia - FI/Espoo)

> <petri.savolainen@nokia.com> wrote:

> >> >>  typedef struct schedule_fn_t {

> >> >> +       int                         status_sync;

> >> >

> >> > this structure should contain functions that are provided by

> scheduler

> >> > to other components of ODP. 'status_sync' seems to be an internal

> >> > mechanism between the default scheduler and default queue. Hence it

> >> > should not be here.

> >> >

> >> Any update on this comment?

> >

> > I did answer it already.

> 

> Ok, found your answer. Should this variable be moved to queue internal

> structure which is set only for iQuery scheduler?

> 

> This structure should contain only the functions exposed by the

> scheduler to other components of ODP. It should not contain anything

> related to the interface between queue and scheduler (they are being

> considered as a single module).


These functions are called from queue, so it's queue -> iquery interface (scheduler interface, not queue interface). These functions can be removed (if iquery is not anymore maintained) or moved as a next step, but that's out of scope of this patch set. This set simply removes unused functions and minimizes number of functions calls, but does not move functions from one (interface) file to another. This set removes dependency to those iquery specific functions already from all other code except queue and iquery, so removing / moving those is easier after this is merged.

-Petri
Honnappa Nagarahalli July 10, 2017, 1:14 p.m. UTC | #10
On 10 July 2017 at 03:05, Savolainen, Petri (Nokia - FI/Espoo)
<petri.savolainen@nokia.com> wrote:
>

>

>> -----Original Message-----

>> From: Honnappa Nagarahalli [mailto:honnappa.nagarahalli@linaro.org]

>> Sent: Friday, July 07, 2017 9:28 PM

>> To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia.com>

>> Cc: lng-odp-forward <lng-odp@lists.linaro.org>

>> Subject: Re: [lng-odp] [API-NEXT PATCH 1/4] linux-gen: sched: remove

>> schedule interface depedency to qentry

>>

>> On 7 July 2017 at 01:46, Savolainen, Petri (Nokia - FI/Espoo)

>> <petri.savolainen@nokia.com> wrote:

>> >> >>  typedef struct schedule_fn_t {

>> >> >> +       int                         status_sync;

>> >> >

>> >> > this structure should contain functions that are provided by

>> scheduler

>> >> > to other components of ODP. 'status_sync' seems to be an internal

>> >> > mechanism between the default scheduler and default queue. Hence it

>> >> > should not be here.

>> >> >

>> >> Any update on this comment?

>> >

>> > I did answer it already.

>>

>> Ok, found your answer. Should this variable be moved to queue internal

>> structure which is set only for iQuery scheduler?

>>

>> This structure should contain only the functions exposed by the

>> scheduler to other components of ODP. It should not contain anything

>> related to the interface between queue and scheduler (they are being

>> considered as a single module).

>

> These functions are called from queue, so it's queue -> iquery interface (scheduler interface, not queue interface). These functions can be removed (if iquery is not anymore maintained) or moved as a next step, but that's out of scope of this patch set. This set simply removes unused functions and minimizes number of functions calls, but does not move functions from one (interface) file to another. This set removes dependency to those iquery specific functions already from all other code except queue and iquery, so removing / moving those is easier after this is merged.

>


Ok. As along as we understand that odp_schedule_if.h should NOT
contain scheduler-queue interface functions/variables (as we consider
them as a single module for now), we can do it as a different patch.

> -Petri

>

>
diff mbox series

Patch

diff --git a/platform/linux-generic/include/odp_schedule_if.h b/platform/linux-generic/include/odp_schedule_if.h
index 5877a1cd..5abbb732 100644
--- a/platform/linux-generic/include/odp_schedule_if.h
+++ b/platform/linux-generic/include/odp_schedule_if.h
@@ -35,9 +35,10 @@  typedef int (*schedule_term_local_fn_t)(void);
 typedef void (*schedule_order_lock_fn_t)(void);
 typedef void (*schedule_order_unlock_fn_t)(void);
 typedef unsigned (*schedule_max_ordered_locks_fn_t)(void);
-typedef void (*schedule_save_context_fn_t)(queue_entry_t *queue);
+typedef void (*schedule_save_context_fn_t)(uint32_t queue_index, void *ptr);
 
 typedef struct schedule_fn_t {
+	int                         status_sync;
 	schedule_pktio_start_fn_t   pktio_start;
 	schedule_thr_add_fn_t       thr_add;
 	schedule_thr_rem_fn_t       thr_rem;
@@ -45,7 +46,6 @@  typedef struct schedule_fn_t {
 	schedule_init_queue_fn_t    init_queue;
 	schedule_destroy_queue_fn_t destroy_queue;
 	schedule_sched_queue_fn_t   sched_queue;
-	schedule_unsched_queue_fn_t unsched_queue;
 	schedule_ord_enq_multi_fn_t ord_enq_multi;
 	schedule_init_global_fn_t   init_global;
 	schedule_term_global_fn_t   term_global;
@@ -54,7 +54,11 @@  typedef struct schedule_fn_t {
 	schedule_order_lock_fn_t    order_lock;
 	schedule_order_unlock_fn_t  order_unlock;
 	schedule_max_ordered_locks_fn_t max_ordered_locks;
+
+	/* Called only when status_sync is set */
+	schedule_unsched_queue_fn_t unsched_queue;
 	schedule_save_context_fn_t  save_context;
+
 } schedule_fn_t;
 
 /* Interface towards the scheduler */
diff --git a/platform/linux-generic/odp_queue.c b/platform/linux-generic/odp_queue.c
index 19945584..2db95fc6 100644
--- a/platform/linux-generic/odp_queue.c
+++ b/platform/linux-generic/odp_queue.c
@@ -474,6 +474,7 @@  static inline int deq_multi(queue_t q_int, odp_buffer_hdr_t *buf_hdr[],
 	int i, j;
 	queue_entry_t *queue;
 	int updated = 0;
+	int status_sync = sched_fn->status_sync;
 
 	queue = qentry_from_int(q_int);
 	LOCK(&queue->s.lock);
@@ -490,7 +491,9 @@  static inline int deq_multi(queue_t q_int, odp_buffer_hdr_t *buf_hdr[],
 		/* Already empty queue */
 		if (queue->s.status == QUEUE_STATUS_SCHED) {
 			queue->s.status = QUEUE_STATUS_NOTSCHED;
-			sched_fn->unsched_queue(queue->s.index);
+
+			if (status_sync)
+				sched_fn->unsched_queue(queue->s.index);
 		}
 
 		UNLOCK(&queue->s.lock);
@@ -533,8 +536,8 @@  static inline int deq_multi(queue_t q_int, odp_buffer_hdr_t *buf_hdr[],
 	if (hdr == NULL)
 		queue->s.tail = NULL;
 
-	if (queue->s.type == ODP_QUEUE_TYPE_SCHED)
-		sched_fn->save_context(queue);
+	if (status_sync && queue->s.type == ODP_QUEUE_TYPE_SCHED)
+		sched_fn->save_context(queue->s.index, queue);
 
 	UNLOCK(&queue->s.lock);
 
diff --git a/platform/linux-generic/odp_schedule.c b/platform/linux-generic/odp_schedule.c
index 814746c7..69de7ac0 100644
--- a/platform/linux-generic/odp_schedule.c
+++ b/platform/linux-generic/odp_schedule.c
@@ -22,9 +22,11 @@ 
 #include <odp/api/sync.h>
 #include <odp/api/packet_io.h>
 #include <odp_ring_internal.h>
-#include <odp_queue_internal.h>
 #include <odp_timer_internal.h>
 
+/* Should remove this dependency */
+#include <odp_queue_internal.h>
+
 /* Number of priority levels  */
 #define NUM_PRIO 8
 
@@ -1340,22 +1342,14 @@  static int schedule_sched_queue(uint32_t queue_index)
 	return 0;
 }
 
-static int schedule_unsched_queue(uint32_t queue_index ODP_UNUSED)
-{
-	return 0;
-}
-
 static int schedule_num_grps(void)
 {
 	return NUM_SCHED_GRPS;
 }
 
-static void schedule_save_context(queue_entry_t *queue ODP_UNUSED)
-{
-}
-
 /* Fill in scheduler interface */
 const schedule_fn_t schedule_default_fn = {
+	.status_sync = 0,
 	.pktio_start = schedule_pktio_start,
 	.thr_add = schedule_thr_add,
 	.thr_rem = schedule_thr_rem,
@@ -1363,7 +1357,6 @@  const schedule_fn_t schedule_default_fn = {
 	.init_queue = schedule_init_queue,
 	.destroy_queue = schedule_destroy_queue,
 	.sched_queue = schedule_sched_queue,
-	.unsched_queue = schedule_unsched_queue,
 	.ord_enq_multi = schedule_ord_enq_multi,
 	.init_global = schedule_init_global,
 	.term_global = schedule_term_global,
@@ -1372,7 +1365,8 @@  const schedule_fn_t schedule_default_fn = {
 	.order_lock = order_lock,
 	.order_unlock = order_unlock,
 	.max_ordered_locks = schedule_max_ordered_locks,
-	.save_context = schedule_save_context
+	.unsched_queue = NULL,
+	.save_context = NULL
 };
 
 /* Fill in scheduler API calls */
diff --git a/platform/linux-generic/odp_schedule_iquery.c b/platform/linux-generic/odp_schedule_iquery.c
index b33ba7cd..75f56e63 100644
--- a/platform/linux-generic/odp_schedule_iquery.c
+++ b/platform/linux-generic/odp_schedule_iquery.c
@@ -12,7 +12,6 @@ 
 #include <odp_internal.h>
 #include <odp_debug_internal.h>
 #include <odp_ring_internal.h>
-#include <odp_queue_internal.h>
 #include <odp_buffer_internal.h>
 #include <odp_bitmap_internal.h>
 #include <odp/api/thread.h>
@@ -25,6 +24,9 @@ 
 #include <odp_config_internal.h>
 #include <odp_timer_internal.h>
 
+/* Should remove this dependency */
+#include <odp_queue_internal.h>
+
 /* Number of priority levels */
 #define NUM_SCHED_PRIO 8
 
@@ -1267,11 +1269,23 @@  static unsigned schedule_max_ordered_locks(void)
 	return MAX_ORDERED_LOCKS_PER_QUEUE;
 }
 
-static void schedule_save_context(queue_entry_t *queue)
+static inline bool is_atomic_queue(unsigned int queue_index)
+{
+	return (sched->queues[queue_index].sync == ODP_SCHED_SYNC_ATOMIC);
+}
+
+static inline bool is_ordered_queue(unsigned int queue_index)
+{
+	return (sched->queues[queue_index].sync == ODP_SCHED_SYNC_ORDERED);
+}
+
+static void schedule_save_context(uint32_t queue_index, void *ptr)
 {
-	if (queue->s.param.sched.sync == ODP_SCHED_SYNC_ATOMIC) {
-		thread_local.atomic = &sched->availables[queue->s.index];
-	} else if (queue->s.param.sched.sync == ODP_SCHED_SYNC_ORDERED) {
+	queue_entry_t *queue = ptr;
+
+	if (is_atomic_queue(queue_index)) {
+		thread_local.atomic = &sched->availables[queue_index];
+	} else if (is_ordered_queue(queue_index)) {
 		uint64_t ctx;
 		odp_atomic_u64_t *next_ctx;
 
@@ -1285,6 +1299,7 @@  static void schedule_save_context(queue_entry_t *queue)
 
 /* Fill in scheduler interface */
 const schedule_fn_t schedule_iquery_fn = {
+	.status_sync   = 1,
 	.pktio_start   = schedule_pktio_start,
 	.thr_add       = group_add_thread,
 	.thr_rem       = group_remove_thread,
@@ -1292,7 +1307,6 @@  const schedule_fn_t schedule_iquery_fn = {
 	.init_queue    = init_sched_queue,
 	.destroy_queue = destroy_sched_queue,
 	.sched_queue   = schedule_sched_queue,
-	.unsched_queue = schedule_unsched_queue,
 	.ord_enq_multi = schedule_ord_enq_multi,
 	.init_global   = schedule_init_global,
 	.term_global   = schedule_term_global,
@@ -1301,7 +1315,8 @@  const schedule_fn_t schedule_iquery_fn = {
 	.order_lock    = order_lock,
 	.order_unlock  = order_unlock,
 	.max_ordered_locks = schedule_max_ordered_locks,
-	.save_context  = schedule_save_context,
+	.unsched_queue = schedule_unsched_queue,
+	.save_context  = schedule_save_context
 };
 
 /* Fill in scheduler API calls */
@@ -1428,18 +1443,6 @@  static void thread_clear_interests(sched_thread_local_t *thread,
 	}
 }
 
-static inline bool is_atomic_queue(unsigned int queue_index)
-{
-	return (sched->queues[queue_index].sync
-			== ODP_SCHED_SYNC_ATOMIC);
-}
-
-static inline bool is_ordered_queue(unsigned int queue_index)
-{
-	return (sched->queues[queue_index].sync
-			== ODP_SCHED_SYNC_ORDERED);
-}
-
 static inline bool compete_atomic_queue(unsigned int queue_index)
 {
 	bool expected = sched->availables[queue_index];
diff --git a/platform/linux-generic/odp_schedule_sp.c b/platform/linux-generic/odp_schedule_sp.c
index 14b7aa88..9829acc5 100644
--- a/platform/linux-generic/odp_schedule_sp.c
+++ b/platform/linux-generic/odp_schedule_sp.c
@@ -410,11 +410,6 @@  static int sched_queue(uint32_t qi)
 	return 0;
 }
 
-static int unsched_queue(uint32_t qi ODP_UNUSED)
-{
-	return 0;
-}
-
 static int ord_enq_multi(queue_t q_int, void *buf_hdr[], int num,
 			 int *ret)
 {
@@ -830,12 +825,9 @@  static void order_unlock(void)
 {
 }
 
-static void save_context(queue_entry_t *queue ODP_UNUSED)
-{
-}
-
 /* Fill in scheduler interface */
 const schedule_fn_t schedule_sp_fn = {
+	.status_sync   = 0,
 	.pktio_start   = pktio_start,
 	.thr_add       = thr_add,
 	.thr_rem       = thr_rem,
@@ -843,16 +835,16 @@  const schedule_fn_t schedule_sp_fn = {
 	.init_queue    = init_queue,
 	.destroy_queue = destroy_queue,
 	.sched_queue   = sched_queue,
-	.unsched_queue = unsched_queue,
 	.ord_enq_multi = ord_enq_multi,
 	.init_global   = init_global,
 	.term_global   = term_global,
 	.init_local    = init_local,
 	.term_local    = term_local,
-	.order_lock =    order_lock,
-	.order_unlock =  order_unlock,
+	.order_lock    = order_lock,
+	.order_unlock  = order_unlock,
 	.max_ordered_locks = max_ordered_locks,
-	.save_context  = save_context
+	.unsched_queue = NULL,
+	.save_context  = NULL
 };
 
 /* Fill in scheduler API calls */