Attachment 122070 Details for Bug 90318 – Proposed patch

[patch] Proposed patch

0001-Bug-90318-Add-_cairo_atomic_int_get_relaxed-and.patch (text/plain), 6.25 KB, created by Wan-Teh Chang on 2016-03-02 02:07:51 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Wan-Teh Chang

Created: 2016-03-02 02:07:51 UTC

Size: 6.25 KB

patch

obsolete

>From f805136edfa423959a1f90010952a636cd1ae72f Mon Sep 17 00:00:00 2001
>From: Wan-Teh Chang <wtc@google.com>
>Date: Tue, 1 Mar 2016 17:48:55 -0800
>Subject: [PATCH] Bug 90318: Add _cairo_atomic_int_get_relaxed and
> _cairo_atomic_int_set_relaxed and use them to fix the data races with
> freed_pool_t's |top| data member.
>
>Signed-off-by: Wan-Teh Chang <wtc@google.com>
>---
> src/cairo-atomic-private.h     | 36 ++++++++++++++++++++++++++++++++++++
> src/cairo-atomic.c             | 14 ++++++++++++++
> src/cairo-freed-pool-private.h | 10 +++++-----
> src/cairo-freed-pool.c         | 10 +++++-----
> 4 files changed, 60 insertions(+), 10 deletions(-)
>
>diff --git a/src/cairo-atomic-private.h b/src/cairo-atomic-private.h
>index 11b2887..5001290 100644
>--- a/src/cairo-atomic-private.h
>+++ b/src/cairo-atomic-private.h
>@@ -75,6 +75,18 @@ _cairo_atomic_int_get (cairo_atomic_int_t *x)
>     return __atomic_load_n(x, __ATOMIC_SEQ_CST);
> }
> 
>+static cairo_always_inline cairo_atomic_int_t
>+_cairo_atomic_int_get_relaxed (cairo_atomic_int_t *x)
>+{
>+    return __atomic_load_n(x, __ATOMIC_RELAXED);
>+}
>+
>+static cairo_always_inline void
>+_cairo_atomic_int_set_relaxed (cairo_atomic_int_t *x, cairo_atomic_int_t val)
>+{
>+    __atomic_store_n(x, val, __ATOMIC_RELAXED);
>+}
>+
> static cairo_always_inline void *
> _cairo_atomic_ptr_get (void **x)
> {
>@@ -157,6 +169,18 @@ _cairo_atomic_int_get (cairo_atomic_int_t *x)
>     return *x;
> }
> 
>+static cairo_always_inline cairo_atomic_int_t
>+_cairo_atomic_int_get_relaxed (cairo_atomic_int_t *x)
>+{
>+    return *x;
>+}
>+
>+static cairo_always_inline void
>+_cairo_atomic_int_set_relaxed (cairo_atomic_int_t *x, cairo_atomic_int_t val)
>+{
>+    *x = val;
>+}
>+
> static cairo_always_inline void *
> _cairo_atomic_ptr_get (void **x)
> {
>@@ -165,6 +189,8 @@ _cairo_atomic_ptr_get (void **x)
> }
> #else
> # define _cairo_atomic_int_get(x) (*x)
>+# define _cairo_atomic_int_get_relaxed(x) (*x)
>+# define _cairo_atomic_int_set_relaxed(x, val) (*x) = (val)
> # define _cairo_atomic_ptr_get(x) (*x)
> #endif
> 
>@@ -200,6 +226,8 @@ typedef long long cairo_atomic_intptr_t;
> typedef  AO_t cairo_atomic_int_t;
> 
> # define _cairo_atomic_int_get(x) (AO_load_full (x))
>+# define _cairo_atomic_int_get_relaxed(x) (AO_load_full (x))
>+# define _cairo_atomic_int_set_relaxed(x, val) (AO_store_full ((x), (val)))
> 
> # define _cairo_atomic_int_inc(x) ((void) AO_fetch_and_add1_full(x))
> # define _cairo_atomic_int_dec(x) ((void) AO_fetch_and_sub1_full(x))
>@@ -230,6 +258,8 @@ typedef unsigned long long cairo_atomic_intptr_t;
> typedef int32_t cairo_atomic_int_t;
> 
> # define _cairo_atomic_int_get(x) (OSMemoryBarrier(), *(x))
>+# define _cairo_atomic_int_get_relaxed(x) *(x)
>+# define _cairo_atomic_int_set_relaxed(x, val) *(x) = (val)
> 
> # define _cairo_atomic_int_inc(x) ((void) OSAtomicIncrement32Barrier (x))
> # define _cairo_atomic_int_dec(x) ((void) OSAtomicDecrement32Barrier (x))
>@@ -288,9 +318,15 @@ _cairo_atomic_ptr_cmpxchg_return_old_impl (void **x, void *oldv, void *newv);
> #ifdef ATOMIC_OP_NEEDS_MEMORY_BARRIER
> cairo_private cairo_atomic_int_t
> _cairo_atomic_int_get (cairo_atomic_int_t *x);
>+cairo_private cairo_atomic_int_t
>+_cairo_atomic_int_get_relaxed (cairo_atomic_int_t *x);
>+void
>+_cairo_atomic_int_set_relaxed (cairo_atomic_int_t *x, cairo_atomic_int_t val);
> # define _cairo_atomic_ptr_get(x) (void *) _cairo_atomic_int_get((cairo_atomic_int_t *) x)
> #else
> # define _cairo_atomic_int_get(x) (*x)
>+# define _cairo_atomic_int_get_relaxed(x) (*x)
>+# define _cairo_atomic_int_set_relaxed(x, val) (*x) = (val)
> # define _cairo_atomic_ptr_get(x) (*x)
> #endif
> 
>diff --git a/src/cairo-atomic.c b/src/cairo-atomic.c
>index 909cfea..2af50cd 100644
>--- a/src/cairo-atomic.c
>+++ b/src/cairo-atomic.c
>@@ -101,6 +101,20 @@ _cairo_atomic_int_get (cairo_atomic_intptr_t *x)
> 
>     return ret;
> }
>+
>+cairo_atomic_intptr_t
>+_cairo_atomic_int_get_relaxed (cairo_atomic_intptr_t *x)
>+{
>+    return _cairo_atomic_int_get (x);
>+}
>+
>+void
>+_cairo_atomic_int_set_relaxed (cairo_atomic_intptr_t *x, cairo_atomic_intptr_t val)
>+{
>+    CAIRO_MUTEX_LOCK (_cairo_atomic_mutex);
>+    *x = val;
>+    CAIRO_MUTEX_UNLOCK (_cairo_atomic_mutex);
>+}
> #endif
> 
> #endif
>diff --git a/src/cairo-freed-pool-private.h b/src/cairo-freed-pool-private.h
>index 0ec6de3..8a7af52 100644
>--- a/src/cairo-freed-pool-private.h
>+++ b/src/cairo-freed-pool-private.h
>@@ -51,7 +51,7 @@ CAIRO_BEGIN_DECLS
> #define MAX_FREED_POOL_SIZE 16
> typedef struct {
>     void *pool[MAX_FREED_POOL_SIZE];
>-    int top;
>+    cairo_atomic_int_t top;
> } freed_pool_t;
> 
> static cairo_always_inline void *
>@@ -81,13 +81,13 @@ _freed_pool_get (freed_pool_t *pool)
>     void *ptr;
>     int i;
> 
>-    i = pool->top - 1;
>+    i = _cairo_atomic_int_get_relaxed (&pool->top) - 1;
>     if (i < 0)
> 	i = 0;
> 
>     ptr = _atomic_fetch (&pool->pool[i]);
>     if (likely (ptr != NULL)) {
>-	pool->top = i;
>+	_cairo_atomic_int_set_relaxed (&pool->top, i);
> 	return ptr;
>     }
> 
>@@ -103,11 +103,11 @@ _freed_pool_put (freed_pool_t *pool, void *ptr)
> {
>     int i;
> 
>-    i = pool->top;
>+    i = _cairo_atomic_int_get_relaxed (&pool->top);
>     if (likely (i < ARRAY_LENGTH (pool->pool) &&
> 		_atomic_store (&pool->pool[i], ptr)))
>     {
>-	pool->top = i + 1;
>+	_cairo_atomic_int_set_relaxed (&pool->top, i + 1);
> 	return;
>     }
> 
>diff --git a/src/cairo-freed-pool.c b/src/cairo-freed-pool.c
>index cfdc8e9..5b1c4c0 100644
>--- a/src/cairo-freed-pool.c
>+++ b/src/cairo-freed-pool.c
>@@ -50,13 +50,13 @@ _freed_pool_get_search (freed_pool_t *pool)
>     for (i = ARRAY_LENGTH (pool->pool); i--;) {
> 	ptr = _atomic_fetch (&pool->pool[i]);
> 	if (ptr != NULL) {
>-	    pool->top = i;
>+	    _cairo_atomic_int_set_relaxed (&pool->top, i);
> 	    return ptr;
> 	}
>     }
> 
>     /* empty */
>-    pool->top = 0;
>+    _cairo_atomic_int_set_relaxed (&pool->top, 0);
>     return NULL;
> }
> 
>@@ -67,13 +67,13 @@ _freed_pool_put_search (freed_pool_t *pool, void *ptr)
> 
>     for (i = 0; i < ARRAY_LENGTH (pool->pool); i++) {
> 	if (_atomic_store (&pool->pool[i], ptr)) {
>-	    pool->top = i + 1;
>+	    _cairo_atomic_int_set_relaxed (&pool->top, i + 1);
> 	    return;
> 	}
>     }
> 
>     /* full */
>-    pool->top = i;
>+    _cairo_atomic_int_set_relaxed (&pool->top, i);
>     free (ptr);
> }
> 
>@@ -87,7 +87,7 @@ _freed_pool_reset (freed_pool_t *pool)
> 	pool->pool[i] = NULL;
>     }
> 
>-    pool->top = 0;
>+    _cairo_atomic_int_set_relaxed (&pool->top, 0);
> }
> 
> #endif
>-- 
>2.7.0.rc3.207.g0ac5344
>

Actions: View | Splinter Review

Attachments on bug 90318: 115548 | 121989 | 122070

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.