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
> +}
> #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
