On 30 June 2017 at 09:10, Petri Savolainen <petri.savolai...@linaro.org> wrote:
> Do not use queue internal type in schedule interface.
>
> Signed-off-by: Petri Savolainen <petri.savolai...@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
>
