On 31 July 2015 at 20:03, Bill Fischofer <[email protected]> wrote:
> > > On Fri, Jul 31, 2015 at 6:52 AM, Bala Manoharan <[email protected] > > wrote: > >> >> >> On 31 July 2015 at 17:48, Bill Fischofer <[email protected]> >> wrote: >> >>> >>> >>> On Fri, Jul 31, 2015 at 1:38 AM, Bala Manoharan < >>> [email protected]> wrote: >>> >>>> Hi, >>>> >>>> Comments inline... >>>> >>>> On 31 July 2015 at 08:11, Bill Fischofer <[email protected]> >>>> wrote: >>>> >>>>> Signed-off-by: Bill Fischofer <[email protected]> >>>>> --- >>>>> include/odp/api/schedule.h | 38 ++++++++++++++++++++++++-------------- >>>>> 1 file changed, 24 insertions(+), 14 deletions(-) >>>>> >>>>> diff --git a/include/odp/api/schedule.h b/include/odp/api/schedule.h >>>>> index 95fc8df..0ab91e4 100644 >>>>> --- a/include/odp/api/schedule.h >>>>> +++ b/include/odp/api/schedule.h >>>>> @@ -147,21 +147,31 @@ void odp_schedule_resume(void); >>>>> void odp_schedule_release_atomic(void); >>>>> >>>>> /** >>>>> - * Release the current ordered context >>>>> - * >>>>> - * This call is valid only for source queues with ordered >>>>> synchronization. It >>>>> - * hints the scheduler that the user has done all enqueues that need >>>>> to maintain >>>>> - * event order in the current ordered context. The scheduler is >>>>> allowed to >>>>> - * release the ordered context of this thread and avoid reordering >>>>> any following >>>>> - * enqueues. However, the context may be still held until the next >>>>> - * odp_schedule() or odp_schedule_multi() call - this call allows but >>>>> does not >>>>> - * force the scheduler to release the context early. >>>>> - * >>>>> - * Early ordered context release may increase parallelism and thus >>>>> system >>>>> - * performance, since scheduler may start reordering events sooner >>>>> than the next >>>>> - * schedule call. >>>>> + * Release the order associated with an event >>>>> + * >>>>> + * This call tells the scheduler that order no longer needs to be >>>>> maintained >>>>> + * for the specified event. This call is needed if, for example, the >>>>> caller >>>>> >>>> >>>> [Bala] This release ordered is for the specified event in this ordered >>>> context only. coz it is always possible >>>> that this event may get enqueued into other ordered/Atomic queue and >>>> the ordering should be maintained >>>> in that context. >>>> >>> >>> Not sure I understand your point here. Ordering is something that is >>> inherited from a source queue and each queue is independent. If you pass >>> events through a series of ordered queues then you get end-to-end ordering, >>> however if you release an event and then add it to another ordered queue it >>> will maintain order from that point however might be reordered with respect >>> to the original queue from which it was released. >>> >> >> I agree with your description above. My point was that the new >> description says >> >> *" This call tells the scheduler that order no longer needs to be >> maintained for the specified event" * >> but the interpretation means that the order will not be maintained for >> the specified event hereafter. But actually what this call does is it >> removes ordering for this event in this specific ordered queue (ordered >> context ) as you explained above the ordering will be maintained by the >> scheduler for this specified event when it gets enqueued into another >> ordered/ atomic queue. so IMO the description should be* " This call >> tells the scheduler that order no longer needs to be maintained for the >> specified event in the current ordered context" * >> >> > OK, I agree. I'll expand the documentation to make that point clear that > the ordering is released only for the current context. > > > >> >>> >>>> >>>> + * will free or otherwise dispose of an event that came from an >>>>> ordered queue >>>>> + * without enqueuing it to another queue. This call does not effect >>>>> the >>>>> >>>> >>>> [Bala] The use-case of freeing or disposing the event can be handled >>>> implicitly by the implementation >>>> since freeing should be done by calling odp_event_free() API and the >>>> implementation is free to release >>>> ordered context during free. >>>> >>> >>> I thought of that, however that would add a lot of unnecessary checking >>> overhead to the free path, which is itself a performance path. If we're >>> going to provide the ability to release order then I don't think it's >>> unreasonable to ask the application to use that in a disciplined manner. >>> >> >> Yes. But lets say if an application does odp_event_free() without calling >> this call then the implementation will have to release the ordering for >> that event. coz this specific case will cause a queue halt if application >> calls odp_event_free() without calling release ordered() API. >> >> > No different than any number of other possible programming errors on the > part of the application (e.g., leaks memory, tries to double-free, forgets > to release a lock, etc.). Why should this one be special? I think we just > need to make it clear that if the application wishes to use ordered queues > that it needs to use them correctly, and explain how to use this API > properly in that context. > But this release ordered context is different because ODP expectation is that the ordered context is released implicitly incase of odp_schedule() call so extending from logic in seems reasonable to release the ordered context during odp_event_free() > To the point Alex keeps rightly bringing up, we need to consider this in > terms of things like fragmentation and reassembly. With this definition > odp_schedule_release_ordered() permits proper fragment reassembly since > each fragment can be released from ordering as it enters the reassembly > buffer and then when the final piece arrives the reassembled packet can > continue using the final fragment's sequence number. > > We seem to need a corresponding odp_schedule_insert_ordered() call to > cover the case of one packet in (with a sequence number) that needs to be > split into multiple parts that are correctly sequenced. I need to think a > bit more about the syntax and semantics of such a capability. Is this > something that either Cavium or Freescale currently support in your HW? > That would be a good basis for constructing the semantics (and > restrictions) surrounding such an API. > > >> Regards, >> Bala >> >>> >>> >>>> >>>> >>>>> + * ordering associated with any other event held by the caller. >>>>> + * >>>>> + * Order release may increase parallelism and thus system >>>>> performance, since >>>>> + * the scheduler may start resolving reordered events sooner than the >>>>> next >>>>> + * odp_queue_enq() call. >>>>> + * >>>>> + * @param ev The event to be released from order preservation. >>>>> + * >>>>> + * @retval 0 Success. Upon return ev behaves as if it originated >>>>> + * from a parallel rather than an ordered queue. >>>>> + * >>>>> + * @retval <0 Failure. This can occur if the event did not >>>>> originate >>>>> + * from an ordered queue (caller error) or the >>>>> implementation >>>>> + * is unable to release order at this time. In this >>>>> case, >>>>> + * the caller must not dispose of ev without enqueing >>>>> it >>>>> + * first to avoid deadlocking other events originating >>>>> from >>>>> + * ev's ordered queue. >>>>> */ >>>>> -void odp_schedule_release_ordered(void); >>>>> +int odp_schedule_release_ordered(odp_event_t ev); >>>>> >>>>> /** >>>>> * Prefetch events for next schedule call >>>>> >>>> >>>> Regards, >>>> Bala >>>> >>>>> -- >>>>> 2.1.4 >>>>> >>>>> _______________________________________________ >>>>> 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
