OK. I've posted v3 with these changes. On Wed, Sep 16, 2015 at 6:11 AM, Savolainen, Petri (Nokia - FI/Espoo) < petri.savolai...@nokia.com> wrote:
> Hi, > > > > I think still that since we need to prepare only for few locks per queue > (32k should be more than enough) and types should reflect that. Specific > widths (uintxx_t) should be reserved for use cases which actually need > those. I propose to use ‘unsigned’ for the index type. It’s at least 16 > bits as well as int is. > > > > /** Lock count */ > > unsigned lock_count; /**< Ordered lock count for this ordered queue */ > > > > int odp_queue_lock_count(odp_queue_t queue); > > void odp_schedule_order_lock(unsigned lock_index); > > void odp_schedule_order_unlock(unsigned lock_index); > > > > > > An typical use case is: > > > > sched.lock_count = 2; > > > > > > odp_schedule_order_lock(0); > > foo++; > > odp_schedule_order_unlock(0); > > > > odp_schedule_order_lock(1); > > bar++; > > odp_schedule_order_unlock(1); > > > > > > > > What comes to the binary compatibility. One binary image need to be > compatible only within one ABI spec (the one it was compiled against). E.g. > when I compile a Linux app for ARMv8, I expect it to run on Linux on any > ARMv8 SoCs - but not e.g. on x86, or on baremetal of a ARMv8 SoC. > > > > > > -Petri > > > > > > > > > > *From:* EXT Bill Fischofer [mailto:bill.fischo...@linaro.org] > *Sent:* Saturday, September 12, 2015 7:16 AM > *To:* Savolainen, Petri (Nokia - FI/Espoo) > *Cc:* lng-odp@lists.linaro.org > *Subject:* Re: [lng-odp] [API-NEXT PATCH 5/5] api: schedule: revise > definition of ordered locks > > > > I've posted a v2 of the patch to reflect incorporation of these suggested > changes. In particular odp_queue_lock_count() now returns an int rather > than uint32_t as suggested, however I believe the lock/unlock calls should > still take a uint32_t as lock indexes can never be negative and there's no > need to add additional checking code for this. It's an ODP convention to > specify parameter widths explicitly for potential ABI compatibility > reasons, so it's better to say something precise like uint32_t than > unsigned int whose width may vary across platforms. > > > > On Thu, Sep 10, 2015 at 9:22 AM, Savolainen, Petri (Nokia - FI/Espoo) < > petri.savolai...@nokia.com> wrote: > > > > > -----Original Message----- > > From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of > > ext Bill Fischofer > > Sent: Sunday, September 06, 2015 5:50 AM > > To: lng-odp@lists.linaro.org > > Subject: [lng-odp] [API-NEXT PATCH 5/5] api: schedule: revise > > definition of ordered locks > > > > Revise the definition of odp_schedule_order_lock() and > > odp_schedule_order_unlock(). These APIs now take as an argument the > > index value of the ordered lock within the current context to be locked > > or unlocked. Because the API is changed, this patch also updates the > > linux-generic implementation of these APIs as well as the scheduler > > validation test that uses them. > > > > Signed-off-by: Bill Fischofer <bill.fischo...@linaro.org> > > --- > > include/odp/api/schedule.h | 44 ++++++++++---- > > - > > .../include/odp/plat/schedule_types.h | 2 - > > .../linux-generic/include/odp_buffer_internal.h | 2 +- > > .../linux-generic/include/odp_queue_internal.h | 19 ++++--- > > platform/linux-generic/odp_queue.c | 66 > > ++++++++++++++-------- > > platform/linux-generic/odp_schedule.c | 15 +++-- > > test/validation/scheduler/scheduler.c | 61 > > +++++++++++++++----- > > 7 files changed, 141 insertions(+), 68 deletions(-) > > > > diff --git a/include/odp/api/schedule.h b/include/odp/api/schedule.h > > index 11f85ad..679d5b8 100644 > > --- a/include/odp/api/schedule.h > > +++ b/include/odp/api/schedule.h > > @@ -31,11 +31,6 @@ extern "C" { > > */ > > > > /** > > - * @typedef odp_schedule_order_lock_t > > - * Scheduler ordered context lock > > - */ > > - > > -/** > > * @def ODP_SCHED_WAIT > > * Wait infinitely > > */ > > @@ -299,15 +294,31 @@ int > > odp_schedule_group_thrmask(odp_schedule_group_t group, > > /** > > * 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 > > + * This call is valid only when holding an ordered synchronization > > context. > > + * Ordered locks are used to protect critical sections that are > > 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 using the > > lock. > > + * > > + * The number of ordered locks available is set by the lock_count > > parameter of > > + * the schedule parameters passed to odp_queue_create(), which must be > > less > > + * than or equal to the ODP_CONFIG_MAX_ORDERED_LOCKS_PER_QUEUE > > configuration > > + * option. If this routine is called outside of an ordered context or > > with a > > + * lock_index that exceeds the number of available ordered locks in > > this > > + * context results are undefined. The number of ordered locks > > associated with > > + * a given ordered queue may be queried by the odp_queue_lock_count() > > API. > > + * > > + * Each ordered lock may be used only once per ordered context. If > > events > > + * are to be processed with multiple ordered critical sections, each > > should > > + * be protected by its own ordered lock. This promotes maximum > > parallelism by > > + * allowing order to maintained on a more granular basis. If an > > ordered lock > > + * is used multiple times in the same event results are undefined. > > ... multiple times in the same *ordered context* ... > > > > + * > > + * @param lock_index Index of the ordered lock in the current context > > to be > > + * acquired. Must be in the range > > 0..odp_queue_lock_count() > > + * - 1 > > */ > > -void odp_schedule_order_lock(odp_schedule_order_lock_t *lock); > > +void odp_schedule_order_lock(uint32_t lock_index); > > > CPU and thread IDs are type 'int'. This may the first time we define an > "index" in any of the APIs. I think 'int' would be sufficient, 'unsigned > int' and uint32_t are also possible. It'll be a small number anyway. > > > -Petri > > > > > > > /** > > * Release ordered context lock > > @@ -315,9 +326,12 @@ void > > odp_schedule_order_lock(odp_schedule_order_lock_t *lock); > > * This call is valid only when holding an ordered synchronization > > context. > > * Release a previously locked ordered context lock. > > * > > - * @param lock Ordered context lock > > + * @param lock_index Index of the ordered lock in the current context > > to be > > + * released. Results are undefined if the caller > > does not > > + * hold this lock. Must be in the range > > + * 0..odp_queue_lock_count() - 1 > > */ > > -void odp_schedule_order_unlock(odp_schedule_order_lock_t *lock); > > +void odp_schedule_order_unlock(uint32_t lock_index); > > > > >
_______________________________________________ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
