On 4 July 2017 at 23:04, Honnappa Nagarahalli
<honnappa.nagaraha...@linaro.org> wrote:
> 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.
>
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
>>
