| Message ID | 1418908495-5409-1-git-send-email-maxim.uvarov@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 12/18/2014 05:24 PM, Stuart Haslam wrote: > On Thu, Dec 18, 2014 at 01:14:55PM +0000, Maxim Uvarov wrote: >> Correctly remove queue from packet i/o and remove it from scheduler. >> >> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> >> --- >> v6: set pktin to INVALID. >> >> platform/linux-generic/odp_packet_io.c | 29 ++++++++++++++++++++++++++++- >> 1 file changed, 28 insertions(+), 1 deletion(-) >> >> diff --git a/platform/linux-generic/odp_packet_io.c b/platform/linux-generic/odp_packet_io.c >> index 3ca8100..9b934b2 100644 >> --- a/platform/linux-generic/odp_packet_io.c >> +++ b/platform/linux-generic/odp_packet_io.c >> @@ -391,7 +391,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; > I had this comment on the last version; > > But I'm not convinced just doing that is enough. The queue still isn't > being removed from the scheduler, so I think you'll find it still calls > pktin_deq_multi and fails when it reaches the invalid pktin. > > and you've not done anything to address that. I'm referring to the fact > that PKTIN queues get treated specially by the scheduler. > > Try this - with this patch applied, add an assert in pktin_deq_multi if > queue->s.pktin == ODP_PKTIO_INVALID, reinstate the remdef test in the > unit test, then after calling remdef invoke the scheduler a few times. I > just did this and the assert is hit. > > I am not sure of the best way to resolve this, but can you please spend > some time thinking about it and testing the change before sending out a > new version? > > -- > Stuart. > OK, understand the point. Will do more testing on that. Maxim.
diff --git a/platform/linux-generic/odp_packet_io.c b/platform/linux-generic/odp_packet_io.c index 3ca8100..9b934b2 100644 --- a/platform/linux-generic/odp_packet_io.c +++ b/platform/linux-generic/odp_packet_io.c @@ -391,7 +391,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; } odp_queue_t odp_pktio_inq_getdef(odp_pktio_t id)
Correctly remove queue from packet i/o and remove it from scheduler. Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> --- v6: set pktin to INVALID. platform/linux-generic/odp_packet_io.c | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-)