Yes, abort on schedule queue failures (scheduler internal errors). -Petri
From: ext Ola Liljedahl [mailto:[email protected]] Sent: Tuesday, June 30, 2015 1:18 PM To: Zoltan Kiss Cc: Savolainen, Petri (Nokia - FI/Espoo); [email protected] Subject: Re: [lng-odp] [API-NEXT PATCH v3 8/9] queue: handle return value of odp_queue_enq() On 29 June 2015 at 20:16, Zoltan Kiss <[email protected]<mailto:[email protected]>> wrote: Hi, So you say instead of returning I should use an ODP_ABORT()? If an ODP function cannot succeed or fail cleanly (e.g. internal side effects unrolled, internal state consistent), it must not return at all. Calling ODP_ABORT is one way of achieving this. Zoli On 09/06/15 14:46, Savolainen, Petri (Nokia - FI/Espoo) wrote: Although implementation (linux-generic) is using the API calls, it can do short cuts e.g. ensure that queue_enq will never fail (abort instead of fail) and then exploit that knowledge. The implementation gets more complex if any "API call" it uses can fail as specified by the API spec. --- a/platform/linux-generic/odp_queue.c +++ b/platform/linux-generic/odp_queue.c @@ -355,8 +355,8 @@ int queue_enq(queue_entry_t *queue, odp_buffer_hdr_t *buf_hdr) UNLOCK(&queue->s.lock); /* Add queue to scheduling */ - if (sched) - schedule_queue(queue); + if (sched && schedule_queue(queue)) + return -1; For example here, the event has been already inserted to the queue (linked list) and this last bit activates scheduling if needed. A this point odp_queue_enq() cannot return "failure" without removing the event from list first. Otherwise both the user and the queue has ownership of the event. Next user would free it and the queue (linked list) is corrupted. return 0; } @@ -395,8 +395,8 @@ int queue_enq_multi(queue_entry_t *queue, odp_buffer_hdr_t *buf_hdr[], int num) UNLOCK(&queue->s.lock); /* Add queue to scheduling */ - if (sched) - schedule_queue(queue); + if (sched && schedule_queue(queue)) + return -1; Same here, too late to just return fail. return num; /* All events enqueued */ } diff --git a/platform/linux-generic/odp_schedule.c b/platform/linux- generic/odp_schedule.c index 9206d5c..9e20fda 100644 --- a/platform/linux-generic/odp_schedule.c +++ b/platform/linux-generic/odp_schedule.c @@ -355,7 +355,10 @@ int schedule_pktio_start(odp_pktio_t pktio, int prio) pri_queue = pri_set_pktio(pktio, prio); - odp_queue_enq(pri_queue, odp_buffer_to_event(buf)); + if (odp_queue_enq(pri_queue, odp_buffer_to_event(buf))) { + odp_buffer_free(buf); + return -1; + } return 0; } @@ -365,7 +368,8 @@ void odp_schedule_release_atomic(void) if (sched_local.pri_queue != ODP_QUEUE_INVALID && sched_local.num == 0) { /* Release current atomic queue */ - odp_queue_enq(sched_local.pri_queue, sched_local.cmd_ev); + if (odp_queue_enq(sched_local.pri_queue, sched_local.cmd_ev)) + odp_event_free(sched_local.cmd_ev); If this happens the atomic queue is (silently) deadlocked. Schedule queues should never fail, just crash/abort. sched_local.pri_queue = ODP_QUEUE_INVALID; } } @@ -456,7 +460,8 @@ static int schedule(odp_queue_t *out_queue, odp_event_t out_ev[], odp_buffer_free(buf); } else { /* Continue scheduling the pktio */ - odp_queue_enq(pri_q, ev); + if (odp_queue_enq(pri_q, ev)) + odp_event_free(ev); Pktio is deadlocked... } continue; @@ -487,7 +492,8 @@ static int schedule(odp_queue_t *out_queue, odp_event_t out_ev[], sched_local.cmd_ev = ev; } else { /* Continue scheduling the queue */ - odp_queue_enq(pri_q, ev); + if (odp_queue_enq(pri_q, ev)) + odp_event_free(ev); Queue is never scheduled again... -Petri _______________________________________________ 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
