| Message ID | 1438948036-31868-13-git-send-email-bill.fischofer@linaro.org |
|---|---|
| State | New |
| Headers | show |
Hi, Comments inline... On 7 August 2015 at 17:17, Bill Fischofer <bill.fischofer@linaro.org> wrote: > Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org> > --- > include/odp/api/schedule.h | 81 > ++++++++++++++++++++++++---------------------- > 1 file changed, 42 insertions(+), 39 deletions(-) > > diff --git a/include/odp/api/schedule.h b/include/odp/api/schedule.h > index 61d3b16..f715144 100644 > --- a/include/odp/api/schedule.h > +++ b/include/odp/api/schedule.h > @@ -174,6 +174,41 @@ void odp_schedule_release_atomic(void); > int odp_schedule_release_ordered(odp_event_t ev); > > /** > + * Ordered event lock > + * > + * Provide reusable in-order serialization for scheduled ordered events. > Upon > + * return the caller is in order with respect to other events originating > from > + * the current ordered context and will retain order until a subsequent > + * odp_schedule_order_unlock() call is made, or until order is resolved > for > + * the event via a call to odp_queue_enq() or > odp_schedule_release_ordered(). > [Bala] The idea of this API is to be able to provide for a small amount of atomic critical section within a fat ordered flow. I believe we can add the description in the note that this lock/unlock API is provided to avoid the *scheduler overhead* if the application wants provide in-order serial access to a small critical section within an ordered flow. Regards, Bala + * If the caller holds multiple events, e.g., as received from > + * odp_schedule_multi(), then the list of events may only be locked in > their > + * received order to avoid deadlock. That is, if the caller received > events > + * 1, 2, and 3 from odp_schedule_multi(), it is an error to attempt to > lock > + * event 2 before locking and unlocking event 1 or otherwise resolving > event > + * 1's order. > + * > + * @param ev The event whose order is to be locked. This event MUST have > + * originated from an ordered queue. > + */ > +void odp_schedule_order_lock(odp_event_t ev); > + > +/** > + * Ordered event unlock > + * > + * Release an ordered lock previously acquired by > odp_schedule_order_lock(). > + * It is an error to attempt to release a lock that had not been > previously > + * acquired for the specified event. Following this call, the thread will > + * exit the critical section protected by the ordered lock and will resume > + * parallel execution until the next call to odp_schedule_order_lock() for > + * that event, or until the event's order is resolved by a call to > + * odp_queue_enq() or odp_schedule_release_ordered(). > + * > + * @param ev The event whose order is to be unlocked. > + */ > +void odp_schedule_order_unlock(odp_event_t ev); > + > +/** > * Copy order from one event to another > * > * This call copies the order associated with an event originating from an > @@ -181,14 +216,19 @@ int odp_schedule_release_ordered(odp_event_t ev); > * associated ordering at input to the call. If the target is ordered, the > * prior ordering must first be released by an > odp_schedule_release_ordered() > * call. Successfuly copying an order implicitly sets the order sustain > - * attribute for the src_event as if odp_schedule_order_sustain() were > called > - * for it. > + * attribute for the src_event as if odp_schedule_order_sustain_set() were > + * called for it. > * > * @param src_event Event whose ordering is to be copied > * @param dst_event Event to receive the same ordering as src_event. > * > * @retval 0 Success > * @retval <0 Failure. Order not copied. > + * > + * @note If multiple events share the same order as a result of > + * odp_schedule_order_copy(), only one of them should attempt to lock the > + * order via calls to odp_schedule_order_lock(). Results are undefined if > this > + * restriction is not observed. > */ > int odp_schedule_order_copy(odp_event_t src_event, odp_event_t dst_event); > > @@ -336,43 +376,6 @@ int odp_schedule_group_leave(odp_schedule_group_t > group, > int odp_schedule_group_count(odp_schedule_group_t group); > > /** > - * Initialize ordered context lock > - * > - * Initialize an ordered queue context lock. The lock can be associated > only > - * with ordered queues and used only within an ordered synchronization > context. > - * > - * @param queue Ordered queue > - * @param lock Ordered context lock > - * > - * @retval 0 on success > - * @retval <0 on failure > - */ > -int odp_schedule_olock_init(odp_queue_t queue, odp_schedule_olock_t > *lock); > - > -/** > - * Acquire ordered context lock > - * > - * This call is valid only when holding an ordered synchronization > context. The > - * lock is used to protect a critical section that is executed within an > - * ordered context. Threads enter the critical section in the order > determined > - * by the context (source queue). Lock ordering is automatically skipped > for > - * threads that release the context instead of calling the lock. > - * > - * @param lock Ordered context lock > - */ > -void odp_schedule_olock_lock(odp_schedule_olock_t *lock); > - > -/** > - * Release ordered context lock > - * > - * This call is valid only when holding an ordered synchronization > context. > - * Release a previously locked ordered context lock. > - * > - * @param lock Ordered context lock > - */ > -void odp_schedule_olock_unlock(odp_schedule_olock_t *lock); > - > -/** > * @} > */ > > -- > 2.1.4 > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp >
Yes, that was what I hadn't fully appreciated with the previous proposed odp_schedule_order_sync() API. I believe these two APIs reflect the semantics you describe. We can add additional color to the doxygen. What we really need is a section of the user describe that relates all of this and how these APIs are intended to be used. That's next up on the "to do" list. :) On Fri, Aug 7, 2015 at 1:13 PM, Bala Manoharan <bala.manoharan@linaro.org> wrote: > Hi, > > Comments inline... > > On 7 August 2015 at 17:17, Bill Fischofer <bill.fischofer@linaro.org> > wrote: > >> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org> >> --- >> include/odp/api/schedule.h | 81 >> ++++++++++++++++++++++++---------------------- >> 1 file changed, 42 insertions(+), 39 deletions(-) >> >> diff --git a/include/odp/api/schedule.h b/include/odp/api/schedule.h >> index 61d3b16..f715144 100644 >> --- a/include/odp/api/schedule.h >> +++ b/include/odp/api/schedule.h >> @@ -174,6 +174,41 @@ void odp_schedule_release_atomic(void); >> int odp_schedule_release_ordered(odp_event_t ev); >> >> /** >> + * Ordered event lock >> + * >> + * Provide reusable in-order serialization for scheduled ordered events. >> Upon >> + * return the caller is in order with respect to other events >> originating from >> + * the current ordered context and will retain order until a subsequent >> + * odp_schedule_order_unlock() call is made, or until order is resolved >> for >> + * the event via a call to odp_queue_enq() or >> odp_schedule_release_ordered(). >> > > [Bala] The idea of this API is to be able to provide for a small amount > of atomic critical section within a fat ordered flow. > I believe we can add the description in the note that this lock/unlock API > is provided to avoid the *scheduler overhead* if the application wants > provide in-order serial access to a small critical section within an > ordered flow. > > Regards, > Bala > > + * If the caller holds multiple events, e.g., as received from >> + * odp_schedule_multi(), then the list of events may only be locked in >> their >> + * received order to avoid deadlock. That is, if the caller received >> events >> + * 1, 2, and 3 from odp_schedule_multi(), it is an error to attempt to >> lock >> + * event 2 before locking and unlocking event 1 or otherwise resolving >> event >> + * 1's order. >> + * >> + * @param ev The event whose order is to be locked. This event MUST >> have >> + * originated from an ordered queue. >> + */ >> +void odp_schedule_order_lock(odp_event_t ev); >> + >> +/** >> + * Ordered event unlock >> + * >> + * Release an ordered lock previously acquired by >> odp_schedule_order_lock(). >> + * It is an error to attempt to release a lock that had not been >> previously >> + * acquired for the specified event. Following this call, the thread >> will >> + * exit the critical section protected by the ordered lock and will >> resume >> + * parallel execution until the next call to odp_schedule_order_lock() >> for >> + * that event, or until the event's order is resolved by a call to >> + * odp_queue_enq() or odp_schedule_release_ordered(). >> + * >> + * @param ev The event whose order is to be unlocked. >> + */ >> +void odp_schedule_order_unlock(odp_event_t ev); >> + >> +/** >> * Copy order from one event to another >> * >> * This call copies the order associated with an event originating from >> an >> @@ -181,14 +216,19 @@ int odp_schedule_release_ordered(odp_event_t ev); >> * associated ordering at input to the call. If the target is ordered, >> the >> * prior ordering must first be released by an >> odp_schedule_release_ordered() >> * call. Successfuly copying an order implicitly sets the order sustain >> - * attribute for the src_event as if odp_schedule_order_sustain() were >> called >> - * for it. >> + * attribute for the src_event as if odp_schedule_order_sustain_set() >> were >> + * called for it. >> * >> * @param src_event Event whose ordering is to be copied >> * @param dst_event Event to receive the same ordering as src_event. >> * >> * @retval 0 Success >> * @retval <0 Failure. Order not copied. >> + * >> + * @note If multiple events share the same order as a result of >> + * odp_schedule_order_copy(), only one of them should attempt to lock the >> + * order via calls to odp_schedule_order_lock(). Results are undefined >> if this >> + * restriction is not observed. >> */ >> int odp_schedule_order_copy(odp_event_t src_event, odp_event_t >> dst_event); >> >> @@ -336,43 +376,6 @@ int odp_schedule_group_leave(odp_schedule_group_t >> group, >> int odp_schedule_group_count(odp_schedule_group_t group); >> >> /** >> - * Initialize ordered context lock >> - * >> - * Initialize an ordered queue context lock. The lock can be associated >> only >> - * with ordered queues and used only within an ordered synchronization >> context. >> - * >> - * @param queue Ordered queue >> - * @param lock Ordered context lock >> - * >> - * @retval 0 on success >> - * @retval <0 on failure >> - */ >> -int odp_schedule_olock_init(odp_queue_t queue, odp_schedule_olock_t >> *lock); >> - >> -/** >> - * Acquire ordered context lock >> - * >> - * This call is valid only when holding an ordered synchronization >> context. The >> - * lock is used to protect a critical section that is executed within an >> - * ordered context. Threads enter the critical section in the order >> determined >> - * by the context (source queue). Lock ordering is automatically skipped >> for >> - * threads that release the context instead of calling the lock. >> - * >> - * @param lock Ordered context lock >> - */ >> -void odp_schedule_olock_lock(odp_schedule_olock_t *lock); >> - >> -/** >> - * Release ordered context lock >> - * >> - * This call is valid only when holding an ordered synchronization >> context. >> - * Release a previously locked ordered context lock. >> - * >> - * @param lock Ordered context lock >> - */ >> -void odp_schedule_olock_unlock(odp_schedule_olock_t *lock); >> - >> -/** >> * @} >> */ >> >> -- >> 2.1.4 >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> https://lists.linaro.org/mailman/listinfo/lng-odp >> > >
diff --git a/include/odp/api/schedule.h b/include/odp/api/schedule.h index 61d3b16..f715144 100644 --- a/include/odp/api/schedule.h +++ b/include/odp/api/schedule.h @@ -174,6 +174,41 @@ void odp_schedule_release_atomic(void); int odp_schedule_release_ordered(odp_event_t ev); /** + * Ordered event lock + * + * Provide reusable in-order serialization for scheduled ordered events. Upon + * return the caller is in order with respect to other events originating from + * the current ordered context and will retain order until a subsequent + * odp_schedule_order_unlock() call is made, or until order is resolved for + * the event via a call to odp_queue_enq() or odp_schedule_release_ordered(). + * If the caller holds multiple events, e.g., as received from + * odp_schedule_multi(), then the list of events may only be locked in their + * received order to avoid deadlock. That is, if the caller received events + * 1, 2, and 3 from odp_schedule_multi(), it is an error to attempt to lock + * event 2 before locking and unlocking event 1 or otherwise resolving event + * 1's order. + * + * @param ev The event whose order is to be locked. This event MUST have + * originated from an ordered queue. + */ +void odp_schedule_order_lock(odp_event_t ev); + +/** + * Ordered event unlock + * + * Release an ordered lock previously acquired by odp_schedule_order_lock(). + * It is an error to attempt to release a lock that had not been previously + * acquired for the specified event. Following this call, the thread will + * exit the critical section protected by the ordered lock and will resume + * parallel execution until the next call to odp_schedule_order_lock() for + * that event, or until the event's order is resolved by a call to + * odp_queue_enq() or odp_schedule_release_ordered(). + * + * @param ev The event whose order is to be unlocked. + */ +void odp_schedule_order_unlock(odp_event_t ev); + +/** * Copy order from one event to another * * This call copies the order associated with an event originating from an @@ -181,14 +216,19 @@ int odp_schedule_release_ordered(odp_event_t ev); * associated ordering at input to the call. If the target is ordered, the * prior ordering must first be released by an odp_schedule_release_ordered() * call. Successfuly copying an order implicitly sets the order sustain - * attribute for the src_event as if odp_schedule_order_sustain() were called - * for it. + * attribute for the src_event as if odp_schedule_order_sustain_set() were + * called for it. * * @param src_event Event whose ordering is to be copied * @param dst_event Event to receive the same ordering as src_event. * * @retval 0 Success * @retval <0 Failure. Order not copied. + * + * @note If multiple events share the same order as a result of + * odp_schedule_order_copy(), only one of them should attempt to lock the + * order via calls to odp_schedule_order_lock(). Results are undefined if this + * restriction is not observed. */ int odp_schedule_order_copy(odp_event_t src_event, odp_event_t dst_event); @@ -336,43 +376,6 @@ int odp_schedule_group_leave(odp_schedule_group_t group, int odp_schedule_group_count(odp_schedule_group_t group); /** - * Initialize ordered context lock - * - * Initialize an ordered queue context lock. The lock can be associated only - * with ordered queues and used only within an ordered synchronization context. - * - * @param queue Ordered queue - * @param lock Ordered context lock - * - * @retval 0 on success - * @retval <0 on failure - */ -int odp_schedule_olock_init(odp_queue_t queue, odp_schedule_olock_t *lock); - -/** - * Acquire ordered context lock - * - * This call is valid only when holding an ordered synchronization context. The - * lock is used to protect a critical section that is executed within an - * ordered context. Threads enter the critical section in the order determined - * by the context (source queue). Lock ordering is automatically skipped for - * threads that release the context instead of calling the lock. - * - * @param lock Ordered context lock - */ -void odp_schedule_olock_lock(odp_schedule_olock_t *lock); - -/** - * Release ordered context lock - * - * This call is valid only when holding an ordered synchronization context. - * Release a previously locked ordered context lock. - * - * @param lock Ordered context lock - */ -void odp_schedule_olock_unlock(odp_schedule_olock_t *lock); - -/** * @} */
Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org> --- include/odp/api/schedule.h | 81 ++++++++++++++++++++++++---------------------- 1 file changed, 42 insertions(+), 39 deletions(-)