On 29 June 2015 at 20:16, Zoltan Kiss <[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]
> https://lists.linaro.org/mailman/listinfo/lng-odp
>
_______________________________________________
lng-odp mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/lng-odp
