On Thu, Apr 14, 2016 at 11:35 AM, Maxim Uvarov <[email protected]> wrote:
> - use odp_config_queues() to get number of queues and int cycle > iterrator. That makes it more easy to reuse that function for > other platfroms.; > - remove declaring variables to invalid at the bottom and check for > them down the code; > - make code more easy to read; > > Signed-off-by: Maxim Uvarov <[email protected]> > --- > platform/linux-generic/odp_queue.c | 36 > +++++++++++++++--------------------- > 1 file changed, 15 insertions(+), 21 deletions(-) > > diff --git a/platform/linux-generic/odp_queue.c > b/platform/linux-generic/odp_queue.c > index 342ffa2..0f7fd15 100644 > --- a/platform/linux-generic/odp_queue.c > +++ b/platform/linux-generic/odp_queue.c > @@ -234,10 +234,8 @@ int odp_queue_lock_count(odp_queue_t handle) > > odp_queue_t odp_queue_create(const char *name, const odp_queue_param_t > *param) > { > - uint32_t i; > + int i; > queue_entry_t *queue; > - odp_queue_t handle = ODP_QUEUE_INVALID; > - odp_queue_type_t type; > odp_queue_param_t default_param; > > if (param == NULL) { > @@ -245,7 +243,7 @@ odp_queue_t odp_queue_create(const char *name, const > odp_queue_param_t *param) > param = &default_param; > } > > - for (i = 0; i < ODP_CONFIG_QUEUES; i++) { > + for (i = 0; i < odp_config_queues(); i++) { > This is needlessly inefficient. The #defines are perfectly OK for implementation internal use. This makes an extra function call for each iteration. > queue = &queue_tbl->queue[i]; > > if (queue->s.status != QUEUE_STATUS_FREE) > @@ -253,33 +251,29 @@ odp_queue_t odp_queue_create(const char *name, const > odp_queue_param_t *param) > > LOCK(&queue->s.lock); > if (queue->s.status == QUEUE_STATUS_FREE) { > - if (queue_init(queue, name, param)) { > + if (queue_init(queue, name, param) || > + queue->s.handle == ODP_QUEUE_INVALID) { > This is a redundant check as this field is initialized part part of queue_init_global() and does not change > UNLOCK(&queue->s.lock); > - return handle; > + return ODP_QUEUE_INVALID; > } > > - type = queue->s.type; > - > - if (type == ODP_QUEUE_TYPE_SCHED) > + if (queue->s.type == ODP_QUEUE_TYPE_SCHED) { > queue->s.status = QUEUE_STATUS_NOTSCHED; > - else > + if (schedule_queue_init(queue)) { > In the original code this routine is called with the queue unlocked. While holding the lock across this call should still work, in general we want to release locks as soon as possible so I don't see this as an improvement. > + ODP_ERR("schedule queue init > failed\n"); > + UNLOCK(&queue->s.lock); > + return ODP_QUEUE_INVALID; > + } > + } else { > queue->s.status = QUEUE_STATUS_READY; > - > - handle = queue->s.handle; > + } > UNLOCK(&queue->s.lock); > - break; > + return queue->s.handle; > } > UNLOCK(&queue->s.lock); > } > > - if (handle != ODP_QUEUE_INVALID && type == ODP_QUEUE_TYPE_SCHED) { > - if (schedule_queue_init(queue)) { > - ODP_ERR("schedule queue init failed\n"); > - return ODP_QUEUE_INVALID; > - } > - } > - > - return handle; > + return ODP_QUEUE_INVALID; > } > > void queue_destroy_finalize(queue_entry_t *queue) > -- > 2.7.1.250.gff4ea60 > > _______________________________________________ > lng-odp mailing list > [email protected] > https://lists.linaro.org/mailman/listinfo/lng-odp >
_______________________________________________ lng-odp mailing list [email protected] https://lists.linaro.org/mailman/listinfo/lng-odp
