On 12 June 2017 at 06:11, Petri Savolainen <petri.savolai...@linaro.org> wrote:
> Clean up function and parameter naming after modular interface
> patch. Queue_t type is referred as "queue internal": queue_int or
> q_int. Term "handle" is reserved for API level handles (e.g.
> odp_queue_t, odp_pktio_t, etc) through out linux-gen implementation.
>
"queue_t" type should be referred to as "handle_int". "handle_int" is
clearly different from "handle".
If we look at the definition of "queue_t":
typedef struct { char dummy; } _queue_t;
typedef _queue_t *queue_t;
it is nothing but a definition of a handle. Why should it be called
with some other name and create confusion? Just like how odp_queue_t
is an abstract type, queue_t is also an abstract type. Just like how
odp_queue_t is a handle, queue_t is also a handle, albeit a handle
towards internal components.
"queue internal" is same as internal implementation definition of the
queue data structure.
> Signed-off-by: Petri Savolainen <petri.savolai...@linaro.org>
> ---
> platform/linux-generic/include/odp_queue_if.h | 28 +--
> .../linux-generic/include/odp_queue_internal.h | 4 +-
> platform/linux-generic/include/odp_schedule_if.h | 2 +-
> platform/linux-generic/odp_queue.c | 218
> +++++++++++----------
> platform/linux-generic/odp_schedule.c | 4 +-
> platform/linux-generic/odp_schedule_iquery.c | 4 +-
> platform/linux-generic/odp_schedule_sp.c | 4 +-
> 7 files changed, 133 insertions(+), 131 deletions(-)
>
> diff --git a/platform/linux-generic/include/odp_queue_if.h
> b/platform/linux-generic/include/odp_queue_if.h
> index 4359435f..168d0e9e 100644
> --- a/platform/linux-generic/include/odp_queue_if.h
> +++ b/platform/linux-generic/include/odp_queue_if.h
> @@ -53,24 +53,24 @@ typedef int (*queue_term_global_fn_t)(void);
> typedef int (*queue_init_local_fn_t)(void);
> typedef int (*queue_term_local_fn_t)(void);
> typedef queue_t (*queue_from_ext_fn_t)(odp_queue_t handle);
> -typedef odp_queue_t (*queue_to_ext_fn_t)(queue_t handle);
> -typedef int (*queue_enq_fn_t)(queue_t handle, odp_buffer_hdr_t *);
> -typedef int (*queue_enq_multi_fn_t)(queue_t handle, odp_buffer_hdr_t **,
> int);
> -typedef odp_buffer_hdr_t *(*queue_deq_fn_t)(queue_t handle);
> -typedef int (*queue_deq_multi_fn_t)(queue_t handle, odp_buffer_hdr_t **,
> int);
> -typedef odp_pktout_queue_t (*queue_get_pktout_fn_t)(queue_t handle);
> -typedef void (*queue_set_pktout_fn_t)(queue_t handle, odp_pktio_t pktio,
> +typedef odp_queue_t (*queue_to_ext_fn_t)(queue_t q_int);
> +typedef int (*queue_enq_fn_t)(queue_t q_int, odp_buffer_hdr_t *);
> +typedef int (*queue_enq_multi_fn_t)(queue_t q_int, odp_buffer_hdr_t **, int);
> +typedef odp_buffer_hdr_t *(*queue_deq_fn_t)(queue_t q_int);
> +typedef int (*queue_deq_multi_fn_t)(queue_t q_int, odp_buffer_hdr_t **, int);
> +typedef odp_pktout_queue_t (*queue_get_pktout_fn_t)(queue_t q_int);
> +typedef void (*queue_set_pktout_fn_t)(queue_t q_int, odp_pktio_t pktio,
> int index);
> -typedef odp_pktin_queue_t (*queue_get_pktin_fn_t)(queue_t handle);
> -typedef void (*queue_set_pktin_fn_t)(queue_t handle, odp_pktio_t pktio,
> +typedef odp_pktin_queue_t (*queue_get_pktin_fn_t)(queue_t q_int);
> +typedef void (*queue_set_pktin_fn_t)(queue_t q_int, odp_pktio_t pktio,
> int index);
> -typedef void (*queue_set_enq_fn_t)(queue_t handle, queue_enq_fn_t func);
> -typedef void (*queue_set_enq_multi_fn_t)(queue_t handle,
> +typedef void (*queue_set_enq_fn_t)(queue_t q_int, queue_enq_fn_t func);
> +typedef void (*queue_set_enq_multi_fn_t)(queue_t q_int,
> queue_enq_multi_fn_t func);
> -typedef void (*queue_set_deq_fn_t)(queue_t handle, queue_deq_fn_t func);
> -typedef void (*queue_set_deq_multi_fn_t)(queue_t handle,
> +typedef void (*queue_set_deq_fn_t)(queue_t q_int, queue_deq_fn_t func);
> +typedef void (*queue_set_deq_multi_fn_t)(queue_t q_int,
> queue_deq_multi_fn_t func);
> -typedef void (*queue_set_type_fn_t)(queue_t handle, odp_queue_type_t type);
> +typedef void (*queue_set_type_fn_t)(queue_t q_int, odp_queue_type_t type);
>
> /* Queue functions towards other internal components */
> typedef struct {
> diff --git a/platform/linux-generic/include/odp_queue_internal.h
> b/platform/linux-generic/include/odp_queue_internal.h
> index 0c31ce8a..d79abd23 100644
> --- a/platform/linux-generic/include/odp_queue_internal.h
> +++ b/platform/linux-generic/include/odp_queue_internal.h
> @@ -78,9 +78,9 @@ static inline uint32_t queue_to_id(odp_queue_t handle)
> return _odp_typeval(handle) - 1;
> }
>
> -static inline queue_entry_t *qentry_from_int(queue_t handle)
> +static inline queue_entry_t *qentry_from_int(queue_t q_int)
q_int should be "handle_int"
> {
> - return (queue_entry_t *)(void *)(handle);
> + return (queue_entry_t *)(void *)(q_int);
> }
>
> static inline queue_t qentry_to_int(queue_entry_t *qentry)
> diff --git a/platform/linux-generic/include/odp_schedule_if.h
> b/platform/linux-generic/include/odp_schedule_if.h
> index 9adacef7..5d10cd37 100644
> --- a/platform/linux-generic/include/odp_schedule_if.h
> +++ b/platform/linux-generic/include/odp_schedule_if.h
> @@ -26,7 +26,7 @@ typedef int (*schedule_init_queue_fn_t)(uint32_t
> queue_index,
> typedef void (*schedule_destroy_queue_fn_t)(uint32_t queue_index);
> typedef int (*schedule_sched_queue_fn_t)(uint32_t queue_index);
> typedef int (*schedule_unsched_queue_fn_t)(uint32_t queue_index);
> -typedef int (*schedule_ord_enq_multi_fn_t)(queue_t handle,
> +typedef int (*schedule_ord_enq_multi_fn_t)(queue_t q_int,
q_int should be "handle_int"
> void *buf_hdr[], int num, int
> *ret);
> typedef int (*schedule_init_global_fn_t)(void);
> typedef int (*schedule_term_global_fn_t)(void);
> diff --git a/platform/linux-generic/odp_queue.c
> b/platform/linux-generic/odp_queue.c
> index 3e18f578..19945584 100644
> --- a/platform/linux-generic/odp_queue.c
> +++ b/platform/linux-generic/odp_queue.c
> @@ -35,20 +35,22 @@
> #include <string.h>
> #include <inttypes.h>
>
> +static int queue_init(queue_entry_t *queue, const char *name,
> + const odp_queue_param_t *param);
> +
This is unnecessary for this patch. Don't waste reviewer's time with
unwanted changes. Unwanted changes have been discussed in the context
of other patches and clearly agreed that they should not be done. In
fact, such changes have been reversed.
> typedef struct queue_table_t {
> queue_entry_t queue[ODP_CONFIG_QUEUES];
> } queue_table_t;
>
> static queue_table_t *queue_tbl;
>
> -static queue_t queue_from_ext(odp_queue_t handle);
> -static int _queue_enq(queue_t handle, odp_buffer_hdr_t *buf_hdr);
> -static odp_buffer_hdr_t *_queue_deq(queue_t handle);
> +static inline queue_entry_t *handle_to_qentry(odp_queue_t handle)
Why is there a need to add this function? We already have
'queue_from_ext' and 'qentry_from_int' which are a must to implement.
The functionality provided by 'handle_to_qentry' can be achieved from
these two functions. 'handle_to_qentry' is adding another layer of
wrapper. This adds to code complexity.
> +{
> + uint32_t queue_id;
>
> -static int _queue_enq_multi(queue_t handle, odp_buffer_hdr_t *buf_hdr[],
> - int num);
> -static int _queue_deq_multi(queue_t handle, odp_buffer_hdr_t *buf_hdr[],
> - int num);
> + queue_id = queue_to_id(handle);
> + return get_qentry(queue_id);
> +}
>
> static inline odp_queue_t queue_from_id(uint32_t queue_id)
> {
> @@ -70,50 +72,6 @@ queue_entry_t *get_qentry(uint32_t queue_id)
> return &queue_tbl->queue[queue_id];
> }
>
> -static int queue_init(queue_entry_t *queue, const char *name,
> - const odp_queue_param_t *param)
> -{
> - if (name == NULL) {
> - queue->s.name[0] = 0;
> - } else {
> - strncpy(queue->s.name, name, ODP_QUEUE_NAME_LEN - 1);
> - queue->s.name[ODP_QUEUE_NAME_LEN - 1] = 0;
> - }
> - memcpy(&queue->s.param, param, sizeof(odp_queue_param_t));
> - if (queue->s.param.sched.lock_count > sched_fn->max_ordered_locks())
> - return -1;
> -
> - if (param->type == ODP_QUEUE_TYPE_SCHED) {
> - queue->s.param.deq_mode = ODP_QUEUE_OP_DISABLED;
> -
> - if (param->sched.sync == ODP_SCHED_SYNC_ORDERED) {
> - unsigned i;
> -
> - odp_atomic_init_u64(&queue->s.ordered.ctx, 0);
> - odp_atomic_init_u64(&queue->s.ordered.next_ctx, 0);
> -
> - for (i = 0; i < queue->s.param.sched.lock_count; i++)
> - odp_atomic_init_u64(&queue->s.ordered.lock[i],
> - 0);
> - }
> - }
> - queue->s.type = queue->s.param.type;
> -
> - queue->s.enqueue = _queue_enq;
> - queue->s.dequeue = _queue_deq;
> - queue->s.enqueue_multi = _queue_enq_multi;
> - queue->s.dequeue_multi = _queue_deq_multi;
> -
> - queue->s.pktin = PKTIN_INVALID;
> - queue->s.pktout = PKTOUT_INVALID;
> -
> - queue->s.head = NULL;
> - queue->s.tail = NULL;
> -
> - return 0;
> -}
> -
> -
Unnecessary change
> static int queue_init_global(void)
> {
> uint32_t i;
> @@ -204,27 +162,27 @@ static int queue_capability(odp_queue_capability_t
> *capa)
>
> static odp_queue_type_t queue_type(odp_queue_t handle)
> {
> - return qentry_from_int(queue_from_ext(handle))->s.type;
> + return handle_to_qentry(handle)->s.type;
No need to introduce another function.
qentry_from_int(queue_from_ext(handle)) clearly shows the current
status that there exists an internal handle. handle_to_qentry(handle)
hides that fact and makes code less readable. This comment applies to
all the instances of similar change.
> }
>
> static odp_schedule_sync_t queue_sched_type(odp_queue_t handle)
> {
> - return qentry_from_int(queue_from_ext(handle))->s.param.sched.sync;
> + return handle_to_qentry(handle)->s.param.sched.sync;
> }
>
> static odp_schedule_prio_t queue_sched_prio(odp_queue_t handle)
> {
> - return qentry_from_int(queue_from_ext(handle))->s.param.sched.prio;
> + return handle_to_qentry(handle)->s.param.sched.prio;
> }
>
> static odp_schedule_group_t queue_sched_group(odp_queue_t handle)
> {
> - return qentry_from_int(queue_from_ext(handle))->s.param.sched.group;
> + return handle_to_qentry(handle)->s.param.sched.group;
> }
>
> static int queue_lock_count(odp_queue_t handle)
> {
> - queue_entry_t *queue = qentry_from_int(queue_from_ext(handle));
> + queue_entry_t *queue = handle_to_qentry(handle);
>
> return queue->s.param.sched.sync == ODP_SCHED_SYNC_ORDERED ?
> (int)queue->s.param.sched.lock_count : -1;
> @@ -299,7 +257,7 @@ void sched_cb_queue_destroy_finalize(uint32_t queue_index)
> static int queue_destroy(odp_queue_t handle)
> {
> queue_entry_t *queue;
> - queue = qentry_from_int(queue_from_ext(handle));
> + queue = handle_to_qentry(handle);
>
> if (handle == ODP_QUEUE_INVALID)
> return -1;
> @@ -352,14 +310,14 @@ static int queue_context_set(odp_queue_t handle, void
> *context,
> uint32_t len ODP_UNUSED)
> {
> odp_mb_full();
> - qentry_from_int(queue_from_ext(handle))->s.param.context = context;
> + handle_to_qentry(handle)->s.param.context = context;
> odp_mb_full();
> return 0;
> }
>
> static void *queue_context(odp_queue_t handle)
> {
> - return qentry_from_int(queue_from_ext(handle))->s.param.context;
> + return handle_to_qentry(handle)->s.param.context;
> }
>
> static odp_queue_t queue_lookup(const char *name)
> @@ -385,7 +343,7 @@ static odp_queue_t queue_lookup(const char *name)
> return ODP_QUEUE_INVALID;
> }
>
> -static inline int enq_multi(queue_t handle, odp_buffer_hdr_t *buf_hdr[],
> +static inline int enq_multi(queue_t q_int, odp_buffer_hdr_t *buf_hdr[],
> int num)
q_int does not indicate that it is a handle. Change it to 'handle_int'.
> {
> int sched = 0;
> @@ -393,8 +351,8 @@ static inline int enq_multi(queue_t handle,
> odp_buffer_hdr_t *buf_hdr[],
> queue_entry_t *queue;
> odp_buffer_hdr_t *hdr, *tail, *next_hdr;
>
> - queue = qentry_from_int(handle);
> - if (sched_fn->ord_enq_multi(handle, (void **)buf_hdr, num, &ret))
> + queue = qentry_from_int(q_int);
> + if (sched_fn->ord_enq_multi(q_int, (void **)buf_hdr, num, &ret))
> return ret;
>
> /* Optimize the common case of single enqueue */
> @@ -462,17 +420,17 @@ static inline int enq_multi(queue_t handle,
> odp_buffer_hdr_t *buf_hdr[],
> return num; /* All events enqueued */
> }
>
> -static int _queue_enq_multi(queue_t handle, odp_buffer_hdr_t *buf_hdr[],
> - int num)
> +static int queue_int_enq_multi(queue_t q_int, odp_buffer_hdr_t *buf_hdr[],
> + int num)
No need to introduce another naming convention. The rest of the code
in ODP follows the convention of starting the function names with '_'.
For ex: take a look at odp_packet_io.c file.
> {
> - return enq_multi(handle, buf_hdr, num);
> + return enq_multi(q_int, buf_hdr, num);
> }
>
> -static int _queue_enq(queue_t handle, odp_buffer_hdr_t *buf_hdr)
> +static int queue_int_enq(queue_t q_int, odp_buffer_hdr_t *buf_hdr)
> {
> int ret;
>
> - ret = enq_multi(handle, &buf_hdr, 1);
> + ret = enq_multi(q_int, &buf_hdr, 1);
>
> if (ret == 1)
> return 0;
> @@ -489,7 +447,7 @@ static int queue_enq_multi(odp_queue_t handle, const
> odp_event_t ev[], int num)
> if (num > QUEUE_MULTI_MAX)
> num = QUEUE_MULTI_MAX;
>
> - queue = qentry_from_int(queue_from_ext(handle));
> + queue = handle_to_qentry(handle);
>
> for (i = 0; i < num; i++)
> buf_hdr[i] = buf_hdl_to_hdr(odp_buffer_from_event(ev[i]));
> @@ -503,13 +461,13 @@ static int queue_enq(odp_queue_t handle, odp_event_t ev)
> odp_buffer_hdr_t *buf_hdr;
> queue_entry_t *queue;
>
> - queue = qentry_from_int(queue_from_ext(handle));
> + queue = handle_to_qentry(handle);
> buf_hdr = buf_hdl_to_hdr(odp_buffer_from_event(ev));
>
> return queue->s.enqueue(qentry_to_int(queue), buf_hdr);
> }
>
> -static inline int deq_multi(queue_t handle, odp_buffer_hdr_t *buf_hdr[],
> +static inline int deq_multi(queue_t q_int, odp_buffer_hdr_t *buf_hdr[],
> int num)
'q_int' does not indicate that it is a handle. Change it to
'handle_int'. Applies to other instances of 'q_int'.
> {
> odp_buffer_hdr_t *hdr, *next;
> @@ -517,7 +475,7 @@ static inline int deq_multi(queue_t handle,
> odp_buffer_hdr_t *buf_hdr[],
> queue_entry_t *queue;
> int updated = 0;
>
> - queue = qentry_from_int(handle);
> + queue = qentry_from_int(q_int);
> LOCK(&queue->s.lock);
> if (odp_unlikely(queue->s.status < QUEUE_STATUS_READY)) {
> /* Bad queue, or queue has been destroyed.
> @@ -583,18 +541,18 @@ static inline int deq_multi(queue_t handle,
> odp_buffer_hdr_t *buf_hdr[],
> return i;
> }
>
> -static int _queue_deq_multi(queue_t handle, odp_buffer_hdr_t *buf_hdr[],
> - int num)
> +static int queue_int_deq_multi(queue_t q_int, odp_buffer_hdr_t *buf_hdr[],
> + int num)
> {
> - return deq_multi(handle, buf_hdr, num);
> + return deq_multi(q_int, buf_hdr, num);
> }
>
> -static odp_buffer_hdr_t *_queue_deq(queue_t handle)
> +static odp_buffer_hdr_t *queue_int_deq(queue_t q_int)
No need to introduce a new naming convention.
> {
> odp_buffer_hdr_t *buf_hdr = NULL;
> int ret;
>
> - ret = deq_multi(handle, &buf_hdr, 1);
> + ret = deq_multi(q_int, &buf_hdr, 1);
>
> if (ret == 1)
> return buf_hdr;
> @@ -611,7 +569,7 @@ static int queue_deq_multi(odp_queue_t handle,
> odp_event_t events[], int num)
> if (num > QUEUE_MULTI_MAX)
> num = QUEUE_MULTI_MAX;
>
> - queue = qentry_from_int(queue_from_ext(handle));
> + queue = handle_to_qentry(handle);
>
> ret = queue->s.dequeue_multi(qentry_to_int(queue), buf_hdr, num);
>
> @@ -627,7 +585,7 @@ static odp_event_t queue_deq(odp_queue_t handle)
> queue_entry_t *queue;
> odp_buffer_hdr_t *buf_hdr;
>
> - queue = qentry_from_int(queue_from_ext(handle));
> + queue = handle_to_qentry(handle);
> buf_hdr = queue->s.dequeue(qentry_to_int(queue));
>
> if (buf_hdr)
> @@ -636,6 +594,49 @@ static odp_event_t queue_deq(odp_queue_t handle)
> return ODP_EVENT_INVALID;
> }
>
> +static int queue_init(queue_entry_t *queue, const char *name,
> + const odp_queue_param_t *param)
> +{
> + if (name == NULL) {
> + queue->s.name[0] = 0;
> + } else {
> + strncpy(queue->s.name, name, ODP_QUEUE_NAME_LEN - 1);
> + queue->s.name[ODP_QUEUE_NAME_LEN - 1] = 0;
> + }
> + memcpy(&queue->s.param, param, sizeof(odp_queue_param_t));
> + if (queue->s.param.sched.lock_count > sched_fn->max_ordered_locks())
> + return -1;
> +
> + if (param->type == ODP_QUEUE_TYPE_SCHED) {
> + queue->s.param.deq_mode = ODP_QUEUE_OP_DISABLED;
> +
> + if (param->sched.sync == ODP_SCHED_SYNC_ORDERED) {
> + unsigned i;
> +
> + odp_atomic_init_u64(&queue->s.ordered.ctx, 0);
> + odp_atomic_init_u64(&queue->s.ordered.next_ctx, 0);
> +
> + for (i = 0; i < queue->s.param.sched.lock_count; i++)
> + odp_atomic_init_u64(&queue->s.ordered.lock[i],
> + 0);
> + }
> + }
> + queue->s.type = queue->s.param.type;
> +
> + queue->s.enqueue = queue_int_enq;
> + queue->s.dequeue = queue_int_deq;
> + queue->s.enqueue_multi = queue_int_enq_multi;
> + queue->s.dequeue_multi = queue_int_deq_multi;
> +
> + queue->s.pktin = PKTIN_INVALID;
> + queue->s.pktout = PKTOUT_INVALID;
> +
> + queue->s.head = NULL;
> + queue->s.tail = NULL;
> +
> + return 0;
> +}
> +
Unnecessary change.
> void queue_lock(queue_entry_t *queue)
> {
> LOCK(&queue->s.lock);
> @@ -776,64 +777,65 @@ static uint64_t queue_to_u64(odp_queue_t hdl)
> return _odp_pri(hdl);
> }
>
> -static odp_pktout_queue_t queue_get_pktout(queue_t handle)
> +static odp_pktout_queue_t queue_get_pktout(queue_t q_int)
> {
> - return qentry_from_int(handle)->s.pktout;
> + return qentry_from_int(q_int)->s.pktout;
> }
>
> -static void queue_set_pktout(queue_t handle, odp_pktio_t pktio, int index)
> +static void queue_set_pktout(queue_t q_int, odp_pktio_t pktio, int index)
> {
> - qentry_from_int(handle)->s.pktout.pktio = pktio;
> - qentry_from_int(handle)->s.pktout.index = index;
> + queue_entry_t *qentry = qentry_from_int(q_int);
> +
> + qentry->s.pktout.pktio = pktio;
> + qentry->s.pktout.index = index;
> }
>
> -static odp_pktin_queue_t queue_get_pktin(queue_t handle)
> +static odp_pktin_queue_t queue_get_pktin(queue_t q_int)
> {
> - return qentry_from_int(handle)->s.pktin;
> + return qentry_from_int(q_int)->s.pktin;
> }
>
> -static void queue_set_pktin(queue_t handle, odp_pktio_t pktio, int index)
> +static void queue_set_pktin(queue_t q_int, odp_pktio_t pktio, int index)
> {
> - qentry_from_int(handle)->s.pktin.pktio = pktio;
> - qentry_from_int(handle)->s.pktin.index = index;
> + queue_entry_t *qentry = qentry_from_int(q_int);
> +
> + qentry->s.pktin.pktio = pktio;
> + qentry->s.pktin.index = index;
> }
>
> -static void queue_set_enq_func(queue_t handle, queue_enq_fn_t func)
> +static void queue_set_enq_func(queue_t q_int, queue_enq_fn_t func)
> {
> - qentry_from_int(handle)->s.enqueue = func;
> + qentry_from_int(q_int)->s.enqueue = func;
> }
>
> -static void queue_set_enq_multi_func(queue_t handle, queue_enq_multi_fn_t
> func)
> +static void queue_set_enq_multi_func(queue_t q_int, queue_enq_multi_fn_t
> func)
> {
> - qentry_from_int(handle)->s.enqueue_multi = func;
> + qentry_from_int(q_int)->s.enqueue_multi = func;
> }
>
> -static void queue_set_deq_func(queue_t handle, queue_deq_fn_t func)
> +static void queue_set_deq_func(queue_t q_int, queue_deq_fn_t func)
> {
> - qentry_from_int(handle)->s.dequeue = func;
> + qentry_from_int(q_int)->s.dequeue = func;
> }
>
> -static void queue_set_deq_multi_func(queue_t handle, queue_deq_multi_fn_t
> func)
> +static void queue_set_deq_multi_func(queue_t q_int, queue_deq_multi_fn_t
> func)
> {
> - qentry_from_int(handle)->s.dequeue_multi = func;
> + qentry_from_int(q_int)->s.dequeue_multi = func;
> }
>
> -static void queue_set_type(queue_t handle, odp_queue_type_t type)
> +static void queue_set_type(queue_t q_int, odp_queue_type_t type)
> {
> - qentry_from_int(handle)->s.type = type;
> + qentry_from_int(q_int)->s.type = type;
> }
>
> static queue_t queue_from_ext(odp_queue_t handle)
> {
> - uint32_t queue_id;
> -
> - queue_id = queue_to_id(handle);
> - return qentry_to_int(get_qentry(queue_id));
> + return qentry_to_int(handle_to_qentry(handle));
'handle_to_qentry' is another extra level of wrapper. I think we
already have enough wrappers because of all the type casts. No need to
introduce one more.
> }
>
> -static odp_queue_t queue_to_ext(queue_t handle)
> +static odp_queue_t queue_to_ext(queue_t q_int)
> {
> - return qentry_from_int(handle)->s.handle;
> + return qentry_from_int(q_int)->s.handle;
> }
>
> /* API functions */
> @@ -866,10 +868,10 @@ queue_fn_t queue_default_fn = {
> .term_local = queue_term_local,
> .from_ext = queue_from_ext,
> .to_ext = queue_to_ext,
> - .enq = _queue_enq,
> - .enq_multi = _queue_enq_multi,
> - .deq = _queue_deq,
> - .deq_multi = _queue_deq_multi,
> + .enq = queue_int_enq,
> + .enq_multi = queue_int_enq_multi,
> + .deq = queue_int_deq,
> + .deq_multi = queue_int_deq_multi,
> .get_pktout = queue_get_pktout,
> .set_pktout = queue_set_pktout,
> .get_pktin = queue_get_pktin,
> diff --git a/platform/linux-generic/odp_schedule.c
> b/platform/linux-generic/odp_schedule.c
> index 011d4dc4..92634235 100644
> --- a/platform/linux-generic/odp_schedule.c
> +++ b/platform/linux-generic/odp_schedule.c
> @@ -712,12 +712,12 @@ static inline int copy_events(odp_event_t out_ev[],
> unsigned int max)
> return i;
> }
>
> -static int schedule_ord_enq_multi(queue_t handle, void *buf_hdr[],
> +static int schedule_ord_enq_multi(queue_t q_int, void *buf_hdr[],
> int num, int *ret)
> {
> int i;
> uint32_t stash_num = sched_local.ordered.stash_num;
> - queue_entry_t *dst_queue = qentry_from_int(handle);
> + queue_entry_t *dst_queue = qentry_from_int(q_int);
> queue_entry_t *src_queue = sched_local.ordered.src_queue;
>
> if (!sched_local.ordered.src_queue || sched_local.ordered.in_order)
> diff --git a/platform/linux-generic/odp_schedule_iquery.c
> b/platform/linux-generic/odp_schedule_iquery.c
> index bdf1a460..79086843 100644
> --- a/platform/linux-generic/odp_schedule_iquery.c
> +++ b/platform/linux-generic/odp_schedule_iquery.c
> @@ -1159,12 +1159,12 @@ static inline void schedule_release_context(void)
> schedule_release_atomic();
> }
>
> -static int schedule_ord_enq_multi(queue_t handle, void *buf_hdr[],
> +static int schedule_ord_enq_multi(queue_t q_int, void *buf_hdr[],
> int num, int *ret)
> {
> int i;
> uint32_t stash_num = thread_local.ordered.stash_num;
> - queue_entry_t *dst_queue = qentry_from_int(handle);
> + queue_entry_t *dst_queue = qentry_from_int(q_int);
> queue_entry_t *src_queue = thread_local.ordered.src_queue;
>
> if (!thread_local.ordered.src_queue || thread_local.ordered.in_order)
> diff --git a/platform/linux-generic/odp_schedule_sp.c
> b/platform/linux-generic/odp_schedule_sp.c
> index 91d70e3a..66715c13 100644
> --- a/platform/linux-generic/odp_schedule_sp.c
> +++ b/platform/linux-generic/odp_schedule_sp.c
> @@ -414,10 +414,10 @@ static int unsched_queue(uint32_t qi ODP_UNUSED)
> return 0;
> }
>
> -static int ord_enq_multi(queue_t handle, void *buf_hdr[], int num,
> +static int ord_enq_multi(queue_t q_int, void *buf_hdr[], int num,
> int *ret)
> {
> - (void)handle;
> + (void)q_int;
> (void)buf_hdr;
> (void)num;
> (void)ret;
> --
> 2.13.0
>
I think queue_t should be removed completely. We should just use
odp_queue_t for internal interfaces as well. Looking at the code,
using odp_queue_t would introduce 3 extra addition instructions and
the impact to cache does not change. We can run l2fwd benchmark and
compare the numbers. This would keep the code simple as well.
