On 01/12/2015 06:07 PM, Stuart Haslam wrote:
On Thu, Dec 25, 2014 at 02:25:29PM +0000, Maxim Uvarov wrote:
Correctly remove queue from packet i/o and remove it from scheduler.
Signed-off-by: Maxim Uvarov <[email protected]>
---
.../linux-generic/include/odp_queue_internal.h | 11 ++++++++
platform/linux-generic/odp_packet_io.c | 29 +++++++++++++++++++++-
platform/linux-generic/odp_schedule.c | 7 +++++-
3 files changed, 45 insertions(+), 2 deletions(-)
diff --git a/platform/linux-generic/include/odp_queue_internal.h
b/platform/linux-generic/include/odp_queue_internal.h
index d5c8e4e..16734ee 100644
--- a/platform/linux-generic/include/odp_queue_internal.h
+++ b/platform/linux-generic/include/odp_queue_internal.h
@@ -129,6 +129,17 @@ static inline int queue_is_destroyed(odp_queue_t handle)
return queue->s.status == QUEUE_STATUS_DESTROYED;
}
+
+static inline int queue_is_sched(odp_queue_t handle)
+{
+ queue_entry_t *queue;
+
+ queue = queue_to_qentry(handle);
+
+ return ((queue->s.status == QUEUE_STATUS_DESTROYED) ||
+ (queue->s.status == QUEUE_STATUS_NOTSCHED) ||
+ queue->s.pktin == ODP_PKTIO_INVALID) ? 0 : 1;
This logic isn't right, it'll return 0 for any queue that has an invalid
pktin, and return 1 for QUEUE_STATUS_FREE and QUEUE_STATUS_READY queues.
I think you get away with it for now because you check the queue type
before calling this function, but perhaps it should be;
queue->s.status == QUEUE_STATUS_SCHED || queue->s.pktin != ODP_PKTIO_INVALID
in v8 I used &&. Because queue_is_sched if STATUS_SCHED AND pktion !=
INVALID, not ||.
Maxim.
+}
#ifdef __cplusplus
}
#endif
diff --git a/platform/linux-generic/odp_packet_io.c
b/platform/linux-generic/odp_packet_io.c
index 59590d2..dd069aa 100644
--- a/platform/linux-generic/odp_packet_io.c
+++ b/platform/linux-generic/odp_packet_io.c
@@ -381,7 +381,34 @@ int odp_pktio_inq_setdef(odp_pktio_t id, odp_queue_t queue)
int odp_pktio_inq_remdef(odp_pktio_t id)
{
- return odp_pktio_inq_setdef(id, ODP_QUEUE_INVALID);
+ pktio_entry_t *pktio_entry = get_pktio_entry(id);
+ odp_queue_t queue;
+ queue_entry_t *qentry;
+
+ if (pktio_entry == NULL)
+ return -1;
+
+ lock_entry(pktio_entry);
+ queue = pktio_entry->s.inq_default;
+ qentry = queue_to_qentry(queue);
+
+ queue_lock(qentry);
+ if (qentry->s.status == QUEUE_STATUS_FREE) {
+ queue_unlock(qentry);
+ unlock_entry(pktio_entry);
+ return -1;
+ }
+
+ qentry->s.enqueue = queue_enq_dummy;
+ qentry->s.enqueue_multi = queue_enq_multi_dummy;
+ qentry->s.status = QUEUE_STATUS_NOTSCHED;
+ qentry->s.pktin = ODP_PKTIO_INVALID;
+ queue_unlock(qentry);
+
+ pktio_entry->s.inq_default = ODP_QUEUE_INVALID;
+ unlock_entry(pktio_entry);
+
+ return 0;
}
This looks OK.
odp_queue_t odp_pktio_inq_getdef(odp_pktio_t id)
diff --git a/platform/linux-generic/odp_schedule.c
b/platform/linux-generic/odp_schedule.c
index a14de4f..365dc98 100644
--- a/platform/linux-generic/odp_schedule.c
+++ b/platform/linux-generic/odp_schedule.c
@@ -286,6 +286,11 @@ static int schedule(odp_queue_t *out_queue, odp_buffer_t
out_buf[],
desc = odp_buffer_addr(desc_buf);
queue = desc->queue;
+ if (odp_queue_type(queue) ==
+ ODP_QUEUE_TYPE_PKTIN &&
+ !queue_is_sched(queue))
+ continue;
+
I don't fully understand why this hunk above is required, or at least
why both this and the change below are. And what happens if remdef is
called in another thread between these two queue_is_sched() checks?
num = odp_queue_deq_multi(queue,
sched_local.buf,
max_deq);
@@ -296,7 +301,7 @@ static int schedule(odp_queue_t *out_queue, odp_buffer_t
out_buf[],
*/
if (odp_queue_type(queue) ==
ODP_QUEUE_TYPE_PKTIN &&
- !queue_is_destroyed(queue))
+ queue_is_sched(queue))
odp_queue_enq(pri_q, desc_buf);
continue;
--
1.8.5.1.163.gd7aced9
--
Stuart.
_______________________________________________
lng-odp mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/lng-odp
