On 04/14/16 23:29, Bill Fischofer wrote:
On Thu, Apr 14, 2016 at 2:50 PM, Maxim Uvarov <[email protected]
<mailto:[email protected]>> wrote:
On 14 April 2016 at 22:16, Bill Fischofer
<[email protected] <mailto:[email protected]>> wrote:
On Thu, Apr 14, 2016 at 11:35 AM, Maxim Uvarov
<[email protected] <mailto:[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]
<mailto:[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.
ah, that is main comment. If we want schedule_queue_init() run
without lock than it's better to use current code, I think, so I
will apply your patch.
One small note about current code which I just realized (might be
it's too late for today).
schedule_queue_init() may fail. Should we in that case set
queue->s.status back to QUEUE_STATUS_FREE before
That's a good catch. However that should be done by
odp_queue_destroy() since at the point of the failure the queue lock
is not held. It's a one line fix. If you want to add it to this
patch I'm fine with that or I can submit a follow-up bug and patch.
I think separate patch for that to not depend on original patch.
Maxim.
return ODP_QUEUE_INVALID ?
Maxim.
+ 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] <mailto:[email protected]>
https://lists.linaro.org/mailman/listinfo/lng-odp
_______________________________________________
lng-odp mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/lng-odp
