For all return values, it would be better to define !0 == success (instead of 
1) - this is inline with the cas_acq which is used to implement the lock and 
potentially more efficient. Also spinlock_trylock should be changed to the same.

Couple of minor comments under.


> -----Original Message-----
> From: lng-odp [mailto:[email protected]] On Behalf Of EXT
> Bill Fischofer
> Sent: Friday, March 04, 2016 1:03 AM
> To: [email protected]
> Subject: [lng-odp] [API-NEXT PATCHv2 1/3] api: rwlock: add trylock
> variants for rwlock and rwlock_recursive
> 
> Add the following APIs:
> - odp_rwlock_read_trylock()
> - odp_rwlock_write_trylock()
> - odp_rwlock_recursive_read_trylock()
> - odp_rwlock_recursive_write_trylock()
> 
> Suggested-by: Sorin Vultureanu <[email protected]>
> Signed-off-by: Bill Fischofer <[email protected]>
> ---
>  include/odp/api/spec/rwlock.h           | 23 ++++++++++++++++++++++-
>  include/odp/api/spec/rwlock_recursive.h | 23 +++++++++++++++++++++++
>  2 files changed, 45 insertions(+), 1 deletion(-)
> 
> diff --git a/include/odp/api/spec/rwlock.h b/include/odp/api/spec/rwlock.h
> index 54f426f..ef91e4a 100644
> --- a/include/odp/api/spec/rwlock.h
> +++ b/include/odp/api/spec/rwlock.h
> @@ -25,7 +25,8 @@ extern "C" {
>   * A reader/writer lock allows multiple simultaneous readers but only one
>   * writer at a time. A thread that wants write access will have to wait
> until
>   * there are no threads that want read access. This casues a risk for
> - * starvation.
> + * starvation. The trylock variants can be used to return an indication
> + * of whether the lock and access mode was immediately available.

I'd formulate it like this:

"The trylock variants can be used to avoid blocking when the lock is not 
immediately available."



>   * @{
>   */
> 
> @@ -50,6 +51,16 @@ void odp_rwlock_init(odp_rwlock_t *rwlock);
>  void odp_rwlock_read_lock(odp_rwlock_t *rwlock);
> 
>  /**
> + * Try to acquire read permission to a reader/writer lock.
> + *
> + * $param rwlock Pointer to a reader/writer lock

@param (not $)


> + *
> + * @retval 0 Lock was not available for read access
> + * @retval 1 Read access to lock acquired
> + */
> +int odp_rwlock_read_trylock(odp_rwlock_t *rwlock);
> +
> +/**
>   * Release read permission on a reader/writer lock.
>   *
>   * @param rwlock Pointer to a reader/writer lock
> @@ -64,6 +75,16 @@ void odp_rwlock_read_unlock(odp_rwlock_t *rwlock);
>  void odp_rwlock_write_lock(odp_rwlock_t *rwlock);
> 
>  /**
> + * Try to acquire write permisssion to a reader/writer lock.

Typo                           ^^^^^^


> + *
> + * @param rwlock Pointer to a reader/writer lock
> + *
> + * @retval 0 Lock was not available for write access
> + * @retval 1 Write access to lock acquired
> + */
> +int odp_rwlock_write_trylock(odp_rwlock_t *rwlock);
> +
> +/**
>   * Release write permission on a reader/writer lock.
>   *
>   * @param rwlock Pointer to a reader/writer lock
> diff --git a/include/odp/api/spec/rwlock_recursive.h
> b/include/odp/api/spec/rwlock_recursive.h
> index 10b2f79..58d5e54 100644
> --- a/include/odp/api/spec/rwlock_recursive.h
> +++ b/include/odp/api/spec/rwlock_recursive.h
> @@ -35,6 +35,9 @@ extern "C" {
>   *
>   * ... but this is not supported.
>   *   * read_lock(); write_lock(); write_unlock(); read_unlock();
> + *
> + * The trylock variants can be used to return an indication of whether
> the
> + * lock and access mode was immediately available.

Same suggestion here.

-Petri



>   * @{
>   */
> 
> @@ -62,6 +65,16 @@ void odp_rwlock_recursive_init(odp_rwlock_recursive_t
> *lock);
>  void odp_rwlock_recursive_read_lock(odp_rwlock_recursive_t *lock);
> 
>  /**
> + * Try to acquire recursive rwlock for reading
> + *
> + * @param lock    Pointer to a lock
> + *
> + * @retval 0      Lock was not available for read access
> + * @retval 1      Read access to lock acquired
> + */
> +int odp_rwlock_recursive_read_trylock(odp_rwlock_recursive_t *lock);
> +
> +/**
>   * Release recursive rwlock after reading
>   *
>   * @param lock    Pointer to a lock
> @@ -80,6 +93,16 @@ void
> odp_rwlock_recursive_read_unlock(odp_rwlock_recursive_t *lock);
>  void odp_rwlock_recursive_write_lock(odp_rwlock_recursive_t *lock);
> 
>  /**
> + * Try to acquire recursive rwlock for writing
> + *
> + * @param lock    Pointer to a lock
> + *
> + * @retval 0      Lock was not available for write access
> + * @retval 1      Write access to lock acquired
> + */
> +int odp_rwlock_recursive_write_trylock(odp_rwlock_recursive_t *lock);
> +
> +/**
>   * Release recursive rwlock after writing
>   *
>   * @param lock    Pointer to a lock
> --
> 2.5.0
> 
> _______________________________________________
> 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

Reply via email to