diff mbox

[5/6] api:odp_ticketlock.h: Update doxygen comments, renaming of function params

Message ID 1417616254-31760-6-git-send-email-ola.liljedahl@linaro.org
State New
Headers show

Commit Message

Ola Liljedahl Dec. 3, 2014, 2:17 p.m. UTC
Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org>
---
Update doxygen comments, renaming of function params.

 .../linux-generic/include/api/odp_ticketlock.h     | 43 ++++++++++------------
 1 file changed, 19 insertions(+), 24 deletions(-)

Comments

Mike Holmes Dec. 3, 2014, 9:28 p.m. UTC | #1
On 3 December 2014 at 09:17, Ola Liljedahl <ola.liljedahl@linaro.org> wrote:

> Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org>
>

---
> Update doxygen comments, renaming of function params.
>
>  .../linux-generic/include/api/odp_ticketlock.h     | 43
> ++++++++++------------
>  1 file changed, 19 insertions(+), 24 deletions(-)
>
> diff --git a/platform/linux-generic/include/api/odp_ticketlock.h
> b/platform/linux-generic/include/api/odp_ticketlock.h
> index 619816e..39a0187 100644
> --- a/platform/linux-generic/include/api/odp_ticketlock.h
> +++ b/platform/linux-generic/include/api/odp_ticketlock.h
> @@ -23,11 +23,15 @@ extern "C" {
>  #include <odp_atomic.h>
>
>  /** @addtogroup odp_synchronizers
> + * Operations on ticket locks.
> + * Acquiring a ticket lock happens in two phases. First the threads takes
> a
> + * ticket. Second it waits (spins) until it is its turn.
> + * Ticket locks are believed to be more fair than spin locks.
>   *  @{
>   */
>
>  /**
> - * ODP ticketlock
> + * ODP ticket lock
>   */
>  typedef struct odp_ticketlock_t {
>         odp_atomic_u32_t  next_ticket; /**< @private Next ticket */
> @@ -36,47 +40,38 @@ typedef struct odp_ticketlock_t {
>
>
>  /**
> - * Init ticketlock
> + * Initialize ticket lock.
>   *
> - * @param ticketlock  Ticketlock
> + * @param[out] p_tklck Pointer to ticket lock
>   */
> -void odp_ticketlock_init(odp_ticketlock_t *ticketlock);
> +void odp_ticketlock_init(odp_ticketlock_t *p_tklck);
>
>
>  /**
> - * Lock ticketlock
> - *
> - * @param ticketlock  Ticketlock
> - */
> -void odp_ticketlock_lock(odp_ticketlock_t *ticketlock);
> -
> -
> -/**
> - * Try to lock ticketlock
> - *
> - * @param ticketlock  Ticketlock
> + * Acquire ticket lock.
>   *
> - * @return 1 if the lock was taken, otherwise 0.
> + * @param[in,out] p_tklck Pointer to ticket lock
>   */
> -int odp_ticketlock_trylock(odp_ticketlock_t *ticketlock);
>

Where is odp_ticketlock_trylock?
I did not expect an api change in a patched named "Update doxygen comments,
renaming of function params"


> +void odp_ticketlock_lock(odp_ticketlock_t *p_tklck);
>
>
>  /**
> - * Unlock ticketlock
> + * Release ticket lock.
>   *
> - * @param ticketlock  Ticketlock
> + * @param[in,out] p_tklck Pointer to ticket lock
>   */
> -void odp_ticketlock_unlock(odp_ticketlock_t *ticketlock);
> +void odp_ticketlock_unlock(odp_ticketlock_t *p_tklck);
>
>
>  /**
> - * Test if ticketlock is locked
> + * Check if ticket lock is locked.
>   *
> - * @param ticketlock  Ticketlock
> + * @param[in] p_tklck Pointer to ticket lock
>   *
> - * @return 1 if the lock is locked, otherwise 0.
> + * @retval 1 the lock is busy (locked)
> + * @retval 0 the lock is available (unlocked)
>   */
> -int odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock);
> +int odp_ticketlock_is_locked(odp_ticketlock_t *p_tklck);
>
>  /**
>   * @}
> --
> 1.9.1
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
>
vkamensky Dec. 3, 2014, 9:53 p.m. UTC | #2
On 3 December 2014 at 13:28, Mike Holmes <mike.holmes@linaro.org> wrote:
>
>
> On 3 December 2014 at 09:17, Ola Liljedahl <ola.liljedahl@linaro.org> wrote:
>>
>> Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org>
>
>
>> ---
>> Update doxygen comments, renaming of function params.
>>
>>  .../linux-generic/include/api/odp_ticketlock.h     | 43
>> ++++++++++------------
>>  1 file changed, 19 insertions(+), 24 deletions(-)
>>
>> diff --git a/platform/linux-generic/include/api/odp_ticketlock.h
>> b/platform/linux-generic/include/api/odp_ticketlock.h
>> index 619816e..39a0187 100644
>> --- a/platform/linux-generic/include/api/odp_ticketlock.h
>> +++ b/platform/linux-generic/include/api/odp_ticketlock.h
>> @@ -23,11 +23,15 @@ extern "C" {
>>  #include <odp_atomic.h>
>>
>>  /** @addtogroup odp_synchronizers
>> + * Operations on ticket locks.
>> + * Acquiring a ticket lock happens in two phases. First the threads takes
>> a
>> + * ticket. Second it waits (spins) until it is its turn.
>> + * Ticket locks are believed to be more fair than spin locks.
>>   *  @{
>>   */
>>
>>  /**
>> - * ODP ticketlock
>> + * ODP ticket lock
>>   */
>>  typedef struct odp_ticketlock_t {
>>         odp_atomic_u32_t  next_ticket; /**< @private Next ticket */
>> @@ -36,47 +40,38 @@ typedef struct odp_ticketlock_t {
>>
>>
>>  /**
>> - * Init ticketlock
>> + * Initialize ticket lock.
>>   *
>> - * @param ticketlock  Ticketlock
>> + * @param[out] p_tklck Pointer to ticket lock
>>   */
>> -void odp_ticketlock_init(odp_ticketlock_t *ticketlock);
>> +void odp_ticketlock_init(odp_ticketlock_t *p_tklck);
>>
>>
>>  /**
>> - * Lock ticketlock
>> - *
>> - * @param ticketlock  Ticketlock
>> - */
>> -void odp_ticketlock_lock(odp_ticketlock_t *ticketlock);
>> -
>> -
>> -/**
>> - * Try to lock ticketlock
>> - *
>> - * @param ticketlock  Ticketlock
>> + * Acquire ticket lock.
>>   *
>> - * @return 1 if the lock was taken, otherwise 0.
>> + * @param[in,out] p_tklck Pointer to ticket lock
>>   */
>> -int odp_ticketlock_trylock(odp_ticketlock_t *ticketlock);
>
>
> Where is odp_ticketlock_trylock?
> I did not expect an api change in a patched named "Update doxygen comments,
> renaming of function params"

And btw with current odp_ticketlock_t structure it would
be very hard to implement it, because in this case both
current and next ticket should be checked/updated
atomically. But currently both defined as u32.

For that reason many Linux kernel ticket locks implementations
define current and next as short type packed into the same
word.

>>
>> +void odp_ticketlock_lock(odp_ticketlock_t *p_tklck);
>>
>>
>>  /**
>> - * Unlock ticketlock
>> + * Release ticket lock.
>>   *
>> - * @param ticketlock  Ticketlock
>> + * @param[in,out] p_tklck Pointer to ticket lock
>>   */
>> -void odp_ticketlock_unlock(odp_ticketlock_t *ticketlock);
>> +void odp_ticketlock_unlock(odp_ticketlock_t *p_tklck);
>>
>>
>>  /**
>> - * Test if ticketlock is locked
>> + * Check if ticket lock is locked.
>>   *
>> - * @param ticketlock  Ticketlock
>> + * @param[in] p_tklck Pointer to ticket lock
>>   *
>> - * @return 1 if the lock is locked, otherwise 0.
>> + * @retval 1 the lock is busy (locked)
>> + * @retval 0 the lock is available (unlocked)
>>   */
>> -int odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock);
>> +int odp_ticketlock_is_locked(odp_ticketlock_t *p_tklck);

I know it is not directly relevant to the patch:
but please note this function seems to be quite broken
with current and previous implementations, where current
and next tickets are read separately. A lot can happen
between those reads. Again Linux kernel misc CPU ticket
lock implementation deal with
it correctly only because it reads current, next
atomically as one integer, for that particular moment
of such read answer is consistent.

Since nobody uses such function now. I would
suggest maybe remove it for now ... Or at least
we need to log bug to fix it latter.

Thanks,
Victor

>>
>>  /**
>>   * @}
>> --
>> 1.9.1
>>
>>
>> _______________________________________________
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> http://lists.linaro.org/mailman/listinfo/lng-odp
>
>
>
>
> --
> Mike Holmes
> Linaro  Sr Technical Manager
> LNG - ODP
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
>
Ola Liljedahl Dec. 3, 2014, 10:57 p.m. UTC | #3
On 3 December 2014 at 22:28, Mike Holmes <mike.holmes@linaro.org> wrote:
>
>
> On 3 December 2014 at 09:17, Ola Liljedahl <ola.liljedahl@linaro.org> wrote:
>>
>> Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org>
>
>
>> ---
>> Update doxygen comments, renaming of function params.
>>
>>  .../linux-generic/include/api/odp_ticketlock.h     | 43
>> ++++++++++------------
>>  1 file changed, 19 insertions(+), 24 deletions(-)
>>
>> diff --git a/platform/linux-generic/include/api/odp_ticketlock.h
>> b/platform/linux-generic/include/api/odp_ticketlock.h
>> index 619816e..39a0187 100644
>> --- a/platform/linux-generic/include/api/odp_ticketlock.h
>> +++ b/platform/linux-generic/include/api/odp_ticketlock.h
>> @@ -23,11 +23,15 @@ extern "C" {
>>  #include <odp_atomic.h>
>>
>>  /** @addtogroup odp_synchronizers
>> + * Operations on ticket locks.
>> + * Acquiring a ticket lock happens in two phases. First the threads takes
>> a
>> + * ticket. Second it waits (spins) until it is its turn.
>> + * Ticket locks are believed to be more fair than spin locks.
>>   *  @{
>>   */
>>
>>  /**
>> - * ODP ticketlock
>> + * ODP ticket lock
>>   */
>>  typedef struct odp_ticketlock_t {
>>         odp_atomic_u32_t  next_ticket; /**< @private Next ticket */
>> @@ -36,47 +40,38 @@ typedef struct odp_ticketlock_t {
>>
>>
>>  /**
>> - * Init ticketlock
>> + * Initialize ticket lock.
>>   *
>> - * @param ticketlock  Ticketlock
>> + * @param[out] p_tklck Pointer to ticket lock
>>   */
>> -void odp_ticketlock_init(odp_ticketlock_t *ticketlock);
>> +void odp_ticketlock_init(odp_ticketlock_t *p_tklck);
>>
>>
>>  /**
>> - * Lock ticketlock
>> - *
>> - * @param ticketlock  Ticketlock
>> - */
>> -void odp_ticketlock_lock(odp_ticketlock_t *ticketlock);
>> -
>> -
>> -/**
>> - * Try to lock ticketlock
>> - *
>> - * @param ticketlock  Ticketlock
>> + * Acquire ticket lock.
>>   *
>> - * @return 1 if the lock was taken, otherwise 0.
>> + * @param[in,out] p_tklck Pointer to ticket lock
>>   */
>> -int odp_ticketlock_trylock(odp_ticketlock_t *ticketlock);
>
>
> Where is odp_ticketlock_trylock?
> I did not expect an api change in a patched named "Update doxygen comments,
> renaming of function params"
Well the function isn't actually implemented... I wrote a separate
email about that. Do we actually need a ticketlock_trylock()?
As Victor also noted, implementing ticketlock_trylock() is slightly complicated.

Do you still think I should keep the declaration (with updated doxygen
comments)?

>
>>
>> +void odp_ticketlock_lock(odp_ticketlock_t *p_tklck);
>>
>>
>>  /**
>> - * Unlock ticketlock
>> + * Release ticket lock.
>>   *
>> - * @param ticketlock  Ticketlock
>> + * @param[in,out] p_tklck Pointer to ticket lock
>>   */
>> -void odp_ticketlock_unlock(odp_ticketlock_t *ticketlock);
>> +void odp_ticketlock_unlock(odp_ticketlock_t *p_tklck);
>>
>>
>>  /**
>> - * Test if ticketlock is locked
>> + * Check if ticket lock is locked.
>>   *
>> - * @param ticketlock  Ticketlock
>> + * @param[in] p_tklck Pointer to ticket lock
>>   *
>> - * @return 1 if the lock is locked, otherwise 0.
>> + * @retval 1 the lock is busy (locked)
>> + * @retval 0 the lock is available (unlocked)
>>   */
>> -int odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock);
>> +int odp_ticketlock_is_locked(odp_ticketlock_t *p_tklck);
>>
>>  /**
>>   * @}
>> --
>> 1.9.1
>>
>>
>> _______________________________________________
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> http://lists.linaro.org/mailman/listinfo/lng-odp
>
>
>
>
> --
> Mike Holmes
> Linaro  Sr Technical Manager
> LNG - ODP
Ola Liljedahl Dec. 3, 2014, 11:18 p.m. UTC | #4
On 3 December 2014 at 22:53, Victor Kamensky <victor.kamensky@linaro.org> wrote:
> On 3 December 2014 at 13:28, Mike Holmes <mike.holmes@linaro.org> wrote:
>>
>>
>> On 3 December 2014 at 09:17, Ola Liljedahl <ola.liljedahl@linaro.org> wrote:
>>>
>>> Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org>
>>
>>
>>> ---
>>> Update doxygen comments, renaming of function params.
>>>
>>>  .../linux-generic/include/api/odp_ticketlock.h     | 43
>>> ++++++++++------------
>>>  1 file changed, 19 insertions(+), 24 deletions(-)
>>>
>>> diff --git a/platform/linux-generic/include/api/odp_ticketlock.h
>>> b/platform/linux-generic/include/api/odp_ticketlock.h
>>> index 619816e..39a0187 100644
>>> --- a/platform/linux-generic/include/api/odp_ticketlock.h
>>> +++ b/platform/linux-generic/include/api/odp_ticketlock.h
>>> @@ -23,11 +23,15 @@ extern "C" {
>>>  #include <odp_atomic.h>
>>>
>>>  /** @addtogroup odp_synchronizers
>>> + * Operations on ticket locks.
>>> + * Acquiring a ticket lock happens in two phases. First the threads takes
>>> a
>>> + * ticket. Second it waits (spins) until it is its turn.
>>> + * Ticket locks are believed to be more fair than spin locks.
>>>   *  @{
>>>   */
>>>
>>>  /**
>>> - * ODP ticketlock
>>> + * ODP ticket lock
>>>   */
>>>  typedef struct odp_ticketlock_t {
>>>         odp_atomic_u32_t  next_ticket; /**< @private Next ticket */
>>> @@ -36,47 +40,38 @@ typedef struct odp_ticketlock_t {
>>>
>>>
>>>  /**
>>> - * Init ticketlock
>>> + * Initialize ticket lock.
>>>   *
>>> - * @param ticketlock  Ticketlock
>>> + * @param[out] p_tklck Pointer to ticket lock
>>>   */
>>> -void odp_ticketlock_init(odp_ticketlock_t *ticketlock);
>>> +void odp_ticketlock_init(odp_ticketlock_t *p_tklck);
>>>
>>>
>>>  /**
>>> - * Lock ticketlock
>>> - *
>>> - * @param ticketlock  Ticketlock
>>> - */
>>> -void odp_ticketlock_lock(odp_ticketlock_t *ticketlock);
>>> -
>>> -
>>> -/**
>>> - * Try to lock ticketlock
>>> - *
>>> - * @param ticketlock  Ticketlock
>>> + * Acquire ticket lock.
>>>   *
>>> - * @return 1 if the lock was taken, otherwise 0.
>>> + * @param[in,out] p_tklck Pointer to ticket lock
>>>   */
>>> -int odp_ticketlock_trylock(odp_ticketlock_t *ticketlock);
>>
>>
>> Where is odp_ticketlock_trylock?
>> I did not expect an api change in a patched named "Update doxygen comments,
>> renaming of function params"
>
> And btw with current odp_ticketlock_t structure it would
> be very hard to implement it, because in this case both
> current and next ticket should be checked/updated
> atomically. But currently both defined as u32.
We could use a 64-bit atomic variable. Or rely on DCAS support (which
is not always available).
But since 2^16 threads should be enough (a thread can only hold one
ticket at a time), we could use 16-bit variables for next and current.
I.e. do it the Linux way as you describe below.

There is one small thing that makes me blink an extra time. I am
looking at some code (a screen shot from a video so quality is not
great) which I think is derived from Linux. When taking the ticket,
the combined variable is atomically read, next incremented and the
combined value written back and we can also check if current matches
the old next value, then we have acquired the lock. On AArch64 this is
done using ldaxr/stxr (load-acquire-exclusive/store-exclusive) on the
32-bit word, then when releasing the ticket lock, stlrh
(store-release-halfword) is used to write only the current field (the
value of the next field must be left untouched).

So we do a load-acquire on a 32-bit word and then store-release on a
16-bit halfword within this word (same base address or different
depending on e.g. endian). Is this perfectly OK?


>
> For that reason many Linux kernel ticket locks implementations
> define current and next as short type packed into the same
> word.
I was planning to prototype such an implementation using 32-bit
atomics. But it seems I would either need a 16-bit atomic type as well
or have to use e.g. CAS when releasing the ticket lock. And CAS is a
bit overkill when all we need is a store with release ordering.

>
>>>
>>> +void odp_ticketlock_lock(odp_ticketlock_t *p_tklck);
>>>
>>>
>>>  /**
>>> - * Unlock ticketlock
>>> + * Release ticket lock.
>>>   *
>>> - * @param ticketlock  Ticketlock
>>> + * @param[in,out] p_tklck Pointer to ticket lock
>>>   */
>>> -void odp_ticketlock_unlock(odp_ticketlock_t *ticketlock);
>>> +void odp_ticketlock_unlock(odp_ticketlock_t *p_tklck);
>>>
>>>
>>>  /**
>>> - * Test if ticketlock is locked
>>> + * Check if ticket lock is locked.
>>>   *
>>> - * @param ticketlock  Ticketlock
>>> + * @param[in] p_tklck Pointer to ticket lock
>>>   *
>>> - * @return 1 if the lock is locked, otherwise 0.
>>> + * @retval 1 the lock is busy (locked)
>>> + * @retval 0 the lock is available (unlocked)
>>>   */
>>> -int odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock);
>>> +int odp_ticketlock_is_locked(odp_ticketlock_t *p_tklck);
>
> I know it is not directly relevant to the patch:
> but please note this function seems to be quite broken
> with current and previous implementations, where current
> and next tickets are read separately. A lot can happen
> between those reads. Again Linux kernel misc CPU ticket
> lock implementation deal with
> it correctly only because it reads current, next
> atomically as one integer, for that particular moment
> of such read answer is consistent.
>
> Since nobody uses such function now. I would
> suggest maybe remove it for now ... Or at least
> we need to log bug to fix it latter.
>
> Thanks,
> Victor
>
>>>
>>>  /**
>>>   * @}
>>> --
>>> 1.9.1
>>>
>>>
>>> _______________________________________________
>>> lng-odp mailing list
>>> lng-odp@lists.linaro.org
>>> http://lists.linaro.org/mailman/listinfo/lng-odp
>>
>>
>>
>>
>> --
>> Mike Holmes
>> Linaro  Sr Technical Manager
>> LNG - ODP
>>
>> _______________________________________________
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> http://lists.linaro.org/mailman/listinfo/lng-odp
>>
Mike Holmes Dec. 4, 2014, 1:24 a.m. UTC | #5
I think the the patch needs to clearly say it alters the public API and not
just the documentation.

Strictly it is a separate patch as it is possible that unknown
implementations are using 0.3.0 and would want to to see this clearly in
the git log.

On 3 December 2014 at 17:57, Ola Liljedahl <ola.liljedahl@linaro.org> wrote:

> On 3 December 2014 at 22:28, Mike Holmes <mike.holmes@linaro.org> wrote:
> >
> >
> > On 3 December 2014 at 09:17, Ola Liljedahl <ola.liljedahl@linaro.org>
> wrote:
> >>
> >> Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org>
> >
> >
> >> ---
> >> Update doxygen comments, renaming of function params.
> >>
> >>  .../linux-generic/include/api/odp_ticketlock.h     | 43
> >> ++++++++++------------
> >>  1 file changed, 19 insertions(+), 24 deletions(-)
> >>
> >> diff --git a/platform/linux-generic/include/api/odp_ticketlock.h
> >> b/platform/linux-generic/include/api/odp_ticketlock.h
> >> index 619816e..39a0187 100644
> >> --- a/platform/linux-generic/include/api/odp_ticketlock.h
> >> +++ b/platform/linux-generic/include/api/odp_ticketlock.h
> >> @@ -23,11 +23,15 @@ extern "C" {
> >>  #include <odp_atomic.h>
> >>
> >>  /** @addtogroup odp_synchronizers
> >> + * Operations on ticket locks.
> >> + * Acquiring a ticket lock happens in two phases. First the threads
> takes
> >> a
> >> + * ticket. Second it waits (spins) until it is its turn.
> >> + * Ticket locks are believed to be more fair than spin locks.
> >>   *  @{
> >>   */
> >>
> >>  /**
> >> - * ODP ticketlock
> >> + * ODP ticket lock
> >>   */
> >>  typedef struct odp_ticketlock_t {
> >>         odp_atomic_u32_t  next_ticket; /**< @private Next ticket */
> >> @@ -36,47 +40,38 @@ typedef struct odp_ticketlock_t {
> >>
> >>
> >>  /**
> >> - * Init ticketlock
> >> + * Initialize ticket lock.
> >>   *
> >> - * @param ticketlock  Ticketlock
> >> + * @param[out] p_tklck Pointer to ticket lock
> >>   */
> >> -void odp_ticketlock_init(odp_ticketlock_t *ticketlock);
> >> +void odp_ticketlock_init(odp_ticketlock_t *p_tklck);
> >>
> >>
> >>  /**
> >> - * Lock ticketlock
> >> - *
> >> - * @param ticketlock  Ticketlock
> >> - */
> >> -void odp_ticketlock_lock(odp_ticketlock_t *ticketlock);
> >> -
> >> -
> >> -/**
> >> - * Try to lock ticketlock
> >> - *
> >> - * @param ticketlock  Ticketlock
> >> + * Acquire ticket lock.
> >>   *
> >> - * @return 1 if the lock was taken, otherwise 0.
> >> + * @param[in,out] p_tklck Pointer to ticket lock
> >>   */
> >> -int odp_ticketlock_trylock(odp_ticketlock_t *ticketlock);
> >
> >
> > Where is odp_ticketlock_trylock?
> > I did not expect an api change in a patched named "Update doxygen
> comments,
> > renaming of function params"
> Well the function isn't actually implemented... I wrote a separate
> email about that. Do we actually need a ticketlock_trylock()?
> As Victor also noted, implementing ticketlock_trylock() is slightly
> complicated.
>
> Do you still think I should keep the declaration (with updated doxygen
> comments)?
>
> >
> >>
> >> +void odp_ticketlock_lock(odp_ticketlock_t *p_tklck);
> >>
> >>
> >>  /**
> >> - * Unlock ticketlock
> >> + * Release ticket lock.
> >>   *
> >> - * @param ticketlock  Ticketlock
> >> + * @param[in,out] p_tklck Pointer to ticket lock
> >>   */
> >> -void odp_ticketlock_unlock(odp_ticketlock_t *ticketlock);
> >> +void odp_ticketlock_unlock(odp_ticketlock_t *p_tklck);
> >>
> >>
> >>  /**
> >> - * Test if ticketlock is locked
> >> + * Check if ticket lock is locked.
> >>   *
> >> - * @param ticketlock  Ticketlock
> >> + * @param[in] p_tklck Pointer to ticket lock
> >>   *
> >> - * @return 1 if the lock is locked, otherwise 0.
> >> + * @retval 1 the lock is busy (locked)
> >> + * @retval 0 the lock is available (unlocked)
> >>   */
> >> -int odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock);
> >> +int odp_ticketlock_is_locked(odp_ticketlock_t *p_tklck);
> >>
> >>  /**
> >>   * @}
> >> --
> >> 1.9.1
> >>
> >>
> >> _______________________________________________
> >> lng-odp mailing list
> >> lng-odp@lists.linaro.org
> >> http://lists.linaro.org/mailman/listinfo/lng-odp
> >
> >
> >
> >
> > --
> > Mike Holmes
> > Linaro  Sr Technical Manager
> > LNG - ODP
>
diff mbox

Patch

diff --git a/platform/linux-generic/include/api/odp_ticketlock.h b/platform/linux-generic/include/api/odp_ticketlock.h
index 619816e..39a0187 100644
--- a/platform/linux-generic/include/api/odp_ticketlock.h
+++ b/platform/linux-generic/include/api/odp_ticketlock.h
@@ -23,11 +23,15 @@  extern "C" {
 #include <odp_atomic.h>
 
 /** @addtogroup odp_synchronizers
+ * Operations on ticket locks.
+ * Acquiring a ticket lock happens in two phases. First the threads takes a
+ * ticket. Second it waits (spins) until it is its turn.
+ * Ticket locks are believed to be more fair than spin locks.
  *  @{
  */
 
 /**
- * ODP ticketlock
+ * ODP ticket lock
  */
 typedef struct odp_ticketlock_t {
 	odp_atomic_u32_t  next_ticket; /**< @private Next ticket */
@@ -36,47 +40,38 @@  typedef struct odp_ticketlock_t {
 
 
 /**
- * Init ticketlock
+ * Initialize ticket lock.
  *
- * @param ticketlock  Ticketlock
+ * @param[out] p_tklck Pointer to ticket lock
  */
-void odp_ticketlock_init(odp_ticketlock_t *ticketlock);
+void odp_ticketlock_init(odp_ticketlock_t *p_tklck);
 
 
 /**
- * Lock ticketlock
- *
- * @param ticketlock  Ticketlock
- */
-void odp_ticketlock_lock(odp_ticketlock_t *ticketlock);
-
-
-/**
- * Try to lock ticketlock
- *
- * @param ticketlock  Ticketlock
+ * Acquire ticket lock.
  *
- * @return 1 if the lock was taken, otherwise 0.
+ * @param[in,out] p_tklck Pointer to ticket lock
  */
-int odp_ticketlock_trylock(odp_ticketlock_t *ticketlock);
+void odp_ticketlock_lock(odp_ticketlock_t *p_tklck);
 
 
 /**
- * Unlock ticketlock
+ * Release ticket lock.
  *
- * @param ticketlock  Ticketlock
+ * @param[in,out] p_tklck Pointer to ticket lock
  */
-void odp_ticketlock_unlock(odp_ticketlock_t *ticketlock);
+void odp_ticketlock_unlock(odp_ticketlock_t *p_tklck);
 
 
 /**
- * Test if ticketlock is locked
+ * Check if ticket lock is locked.
  *
- * @param ticketlock  Ticketlock
+ * @param[in] p_tklck Pointer to ticket lock
  *
- * @return 1 if the lock is locked, otherwise 0.
+ * @retval 1 the lock is busy (locked)
+ * @retval 0 the lock is available (unlocked)
  */
-int odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock);
+int odp_ticketlock_is_locked(odp_ticketlock_t *p_tklck);
 
 /**
  * @}