linux-generic: rwlock: fix odp_rwlock_read_trylock()

Message ID 1493226006-7600-1-git-send-email-odpbot@yandex.ru
State New
Headers show

Commit Message

Github ODP bot April 26, 2017, 5 p.m.
From: Dmitry Eremin-Solenikov <dmitry.ereminsolenikov@linaro.org>


odp_rwlock_read_trylock() currently works only if there are no readers
(and writers) as it compares counter with 0. Make it actually work in
case there are other active readers.

Fixes: https://bugs.linaro.org/show_bug.cgi?id=2974

Signed-off-by: Dmitry Eremin-Solenikov <dmitry.ereminsolenikov@linaro.org>

---
/** Email created from pull request 19 (lumag:fix-rwlock)
 ** https://github.com/Linaro/odp/pull/19
 ** Patch: https://github.com/Linaro/odp/pull/19.patch
 ** Base sha: 9b993a1531c94782b48292adff54a95de9d2be5c
 ** Merge commit sha: 3af3b11b695d157ce1649955b52e8a165cc82937
 **/
 platform/linux-generic/odp_rwlock.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Peltonen, Janne (Nokia - FI/Espoo) April 26, 2017, 5:54 p.m. | #1
That does not work. Since the value of cnt is not checked, the code
would happily take the lock even when a writer already has it.

	Janne

> -----Original Message-----

> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of Github ODP bot

> Sent: Wednesday, April 26, 2017 8:00 PM

> To: lng-odp@lists.linaro.org

> Subject: [lng-odp] [PATCH] linux-generic: rwlock: fix odp_rwlock_read_trylock()

> 

> From: Dmitry Eremin-Solenikov <dmitry.ereminsolenikov@linaro.org>

> 

> odp_rwlock_read_trylock() currently works only if there are no readers

> (and writers) as it compares counter with 0. Make it actually work in

> case there are other active readers.

> 

> Fixes: https://bugs.linaro.org/show_bug.cgi?id=2974

> 

> Signed-off-by: Dmitry Eremin-Solenikov <dmitry.ereminsolenikov@linaro.org>

> ---

> /** Email created from pull request 19 (lumag:fix-rwlock)

>  ** https://github.com/Linaro/odp/pull/19

>  ** Patch: https://github.com/Linaro/odp/pull/19.patch

>  ** Base sha: 9b993a1531c94782b48292adff54a95de9d2be5c

>  ** Merge commit sha: 3af3b11b695d157ce1649955b52e8a165cc82937

>  **/

>  platform/linux-generic/odp_rwlock.c | 4 ++--

>  1 file changed, 2 insertions(+), 2 deletions(-)

> 

> diff --git a/platform/linux-generic/odp_rwlock.c b/platform/linux-generic/odp_rwlock.c

> index 13c17a2..e0c46ce 100644

> --- a/platform/linux-generic/odp_rwlock.c

> +++ b/platform/linux-generic/odp_rwlock.c

> @@ -33,9 +33,9 @@ void odp_rwlock_read_lock(odp_rwlock_t *rwlock)

> 

>  int odp_rwlock_read_trylock(odp_rwlock_t *rwlock)

>  {

> -	uint32_t zero = 0;

> +	uint32_t cnt = odp_atomic_load_u32(&rwlock->cnt);

> 

> -	return odp_atomic_cas_acq_u32(&rwlock->cnt, &zero, (uint32_t)1);

> +	return odp_atomic_cas_acq_u32(&rwlock->cnt, &cnt, cnt + 1);

>  }

> 

>  void odp_rwlock_read_unlock(odp_rwlock_t *rwlock)
Dmitry Eremin-Solenikov April 26, 2017, 6:33 p.m. | #2
On 26.04.2017 20:54, Peltonen, Janne (Nokia - FI/Espoo) wrote:
> 

> That does not work. Since the value of cnt is not checked, the code

> would happily take the lock even when a writer already has it.


Nice catch. Updated to check for writers.

-- 
With best wishes
Dmitry

Patch

diff --git a/platform/linux-generic/odp_rwlock.c b/platform/linux-generic/odp_rwlock.c
index 13c17a2..e0c46ce 100644
--- a/platform/linux-generic/odp_rwlock.c
+++ b/platform/linux-generic/odp_rwlock.c
@@ -33,9 +33,9 @@  void odp_rwlock_read_lock(odp_rwlock_t *rwlock)
 
 int odp_rwlock_read_trylock(odp_rwlock_t *rwlock)
 {
-	uint32_t zero = 0;
+	uint32_t cnt = odp_atomic_load_u32(&rwlock->cnt);
 
-	return odp_atomic_cas_acq_u32(&rwlock->cnt, &zero, (uint32_t)1);
+	return odp_atomic_cas_acq_u32(&rwlock->cnt, &cnt, cnt + 1);
 }
 
 void odp_rwlock_read_unlock(odp_rwlock_t *rwlock)