[gegl] buffer: add gegl_tile_read_{lock,unlock}()
- From: Ell <ell src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gegl] buffer: add gegl_tile_read_{lock,unlock}()
- Date: Mon, 2 Jul 2018 06:00:06 +0000 (UTC)
commit 301b798b4b2d4a113a547df7ba29e6621a06357a
Author: Ell <ell_se yahoo com>
Date: Sat Jun 30 17:02:34 2018 -0400
buffer: add gegl_tile_read_{lock,unlock}()
Currently, we don't manage read access to tile data -- code that
reads from a tile simply calls gegl_tile_get_data(), and assumes
the returned pointer is valid throghout. This, however, might not
be the case in general, in a multithreaded context:
Suppose tiles A and B share the same data. Thread 1 begins reading
tile A through the pointer returned by gegl_tile_get_data().
Thread 2 then locks tile A for writing (intending to write to a
portion of the tile that doesn't overlap the portion thread 1 is
reading). Since the tile's data is shared, thread 2 unclones the
data before writing, and so the data thread 1 is reading is now
solely owned by tile B. At this point, two things can go wrong:
a concurrent write operation to tile B can overwrite the data
thread 1 is reading (which supposedly belongs to tile A), or,
tile B can be destroyed, freeing the data thread 1 is reading.
To fix this, we add the gegl_tile_read_lock() and
gegl_tile_read_unlock() functions, which "lock" the tile for read
access. Code that reads tile data should call these functions
before/after reading from the tile, if the read may happen in a
multithreaded context. Note that in spite of the functions' names,
they don't actually block execution in general. Instead, they make
sure that if gegl_tile_lock() is called on a cloned tile while it's
being read, the function will block until reading is done before
uncloning the tile.
One exception to the above are clones of the global empty tile, for
which we avoid the said blocking altogether, since their data may
never end up being owned by a single, mutable tile.
Using gegl_tile_lock/unlock() before/after writing, and
gegl_tile_read_lock/unlock() before/after reading, guarantees that
the following combinations of concurrent tile operations are safe:
- Read/read.
- Read/duplicate.
- Duplicate/duplicate.
- Non-overlapping read/write.
- Non-overlapping write/write.
gegl/buffer/gegl-buffer-private.h | 15 ++++--
gegl/buffer/gegl-tile-handler-empty.c | 5 +-
gegl/buffer/gegl-tile.c | 88 ++++++++++++++++++++++++++---------
gegl/buffer/gegl-tile.h | 10 ++++
4 files changed, 89 insertions(+), 29 deletions(-)
---
diff --git a/gegl/buffer/gegl-buffer-private.h b/gegl/buffer/gegl-buffer-private.h
index f43b76ecc..c6e7d316f 100644
--- a/gegl/buffer/gegl-buffer-private.h
+++ b/gegl/buffer/gegl-buffer-private.h
@@ -157,11 +157,16 @@ struct _GeglTile
guint stored_rev; /* what revision was we when we from tile_storage?
(currently set to 1 when loaded from disk */
- gint lock_count; /* number of outstanding write locks */
- guint is_zero_tile:1; /* whether the tile data is fully zeroed
- * (allowing for false negatives, but not
- * false positives)
- */
+ gint lock_count; /* number of outstanding write locks */
+ gint read_lock_count; /* number of outstanding read locks */
+ guint is_zero_tile:1; /* whether the tile data is fully zeroed
+ * (allowing for false negatives, but not
+ * false positives)
+ */
+ guint is_global_tile:1; /* whether the tile data is global (and
+ * therefore can never be owned by a
+ * single mutable tile)
+ */
gint clone_state; /* tile clone/unclone state & spinlock */
gint *n_clones; /* an array of two atomic counters, shared
diff --git a/gegl/buffer/gegl-tile-handler-empty.c b/gegl/buffer/gegl-tile-handler-empty.c
index ec1f40136..6338472f9 100644
--- a/gegl/buffer/gegl-tile-handler-empty.c
+++ b/gegl/buffer/gegl-tile-handler-empty.c
@@ -51,7 +51,7 @@ _new_empty_tile (const gint tile_size)
tile = gegl_tile_new (tile_size);
memset (gegl_tile_get_data (tile), 0x00, tile_size);
- tile->is_zero_tile = 1;
+ tile->is_zero_tile = TRUE;
}
else
{
@@ -65,7 +65,8 @@ _new_empty_tile (const gint tile_size)
allocated_tile->data = allocated_buffer;
allocated_tile->destroy_notify = NULL;
allocated_tile->size = common_empty_size;
- allocated_tile->is_zero_tile = 1;
+ allocated_tile->is_zero_tile = TRUE;
+ allocated_tile->is_global_tile = TRUE;
/* avoid counting duplicates of the empty tile towards the total
* cache size, both since this is unnecessary, and since they may
diff --git a/gegl/buffer/gegl-tile.c b/gegl/buffer/gegl-tile.c
index 933cbaeea..f878d3c16 100644
--- a/gegl/buffer/gegl-tile.c
+++ b/gegl/buffer/gegl-tile.c
@@ -98,14 +98,15 @@ void gegl_tile_unref (GeglTile *tile)
static inline GeglTile *
gegl_tile_new_bare_internal (void)
{
- GeglTile *tile = g_slice_new0 (GeglTile);
- tile->ref_count = 1;
- tile->tile_storage = NULL;
- tile->stored_rev = 1;
- tile->rev = 1;
- tile->lock_count = 0;
- tile->clone_state = CLONE_STATE_UNCLONED;
- tile->data = NULL;
+ GeglTile *tile = g_slice_new0 (GeglTile);
+ tile->ref_count = 1;
+ tile->tile_storage = NULL;
+ tile->stored_rev = 1;
+ tile->rev = 1;
+ tile->lock_count = 0;
+ tile->read_lock_count = 0;
+ tile->clone_state = CLONE_STATE_UNCLONED;
+ tile->data = NULL;
return tile;
}
@@ -133,13 +134,14 @@ gegl_tile_dup (GeglTile *src)
g_warn_if_fail (src->lock_count == 0);
g_warn_if_fail (! src->damage);
- src->clone_state = CLONE_STATE_CLONED;
+ src->clone_state = CLONE_STATE_CLONED;
- tile->data = src->data;
- tile->size = src->size;
- tile->is_zero_tile = src->is_zero_tile;
- tile->clone_state = CLONE_STATE_CLONED;
- tile->n_clones = src->n_clones;
+ tile->data = src->data;
+ tile->size = src->size;
+ tile->is_zero_tile = src->is_zero_tile;
+ tile->is_global_tile = src->is_global_tile;
+ tile->clone_state = CLONE_STATE_CLONED;
+ tile->n_clones = src->n_clones;
/* if the tile is not empty, mark it as dirty, since, even though the in-
* memory tile data is shared with the source tile, the stored tile data is
@@ -182,6 +184,17 @@ gegl_tile_unclone (GeglTile *tile)
{
GeglTileHandlerCache *notify_cache = NULL;
gboolean cached;
+ gboolean global;
+
+ global = tile->is_global_tile;
+ tile->is_global_tile = FALSE;
+
+ if (! global)
+ {
+ while (! g_atomic_int_compare_and_exchange (&tile->read_lock_count,
+ 0,
+ -1));
+ }
cached = tile->tile_storage && tile->tile_storage->cache;
@@ -205,9 +218,8 @@ gegl_tile_unclone (GeglTile *tile)
*/
*gegl_tile_n_clones (tile) = 1;
*gegl_tile_n_cached_clones (tile) = cached;
- if (notify_cache)
- gegl_tile_handler_cache_tile_uncloned (notify_cache, tile);
- return;
+
+ goto end;
}
tile->n_clones = gegl_calloc (INLINE_N_ELEMENTS_DATA_OFFSET +
@@ -228,9 +240,8 @@ gegl_tile_unclone (GeglTile *tile)
gegl_free (buf);
*gegl_tile_n_clones (tile) = 1;
*gegl_tile_n_cached_clones (tile) = cached;
- if (notify_cache)
- gegl_tile_handler_cache_tile_uncloned (notify_cache, tile);
- return;
+
+ goto end;
}
tile->n_clones = (gint *) buf;
@@ -238,14 +249,20 @@ gegl_tile_unclone (GeglTile *tile)
*gegl_tile_n_clones (tile) = 1;
*gegl_tile_n_cached_clones (tile) = cached;
- tile->data = (guchar *) tile->n_clones +
- INLINE_N_ELEMENTS_DATA_OFFSET;
+
+ g_atomic_pointer_set (&tile->data,
+ (guchar *) tile->n_clones +
+ INLINE_N_ELEMENTS_DATA_OFFSET);
tile->destroy_notify = (void*)&free_n_clones_directly;
tile->destroy_notify_data = NULL;
+end:
if (notify_cache)
gegl_tile_handler_cache_tile_uncloned (notify_cache, tile);
+
+ if (! global)
+ g_atomic_int_set (&tile->read_lock_count, 0);
}
}
@@ -377,6 +394,33 @@ gegl_tile_unlock_no_void (GeglTile *tile)
}
}
+void
+gegl_tile_read_lock (GeglTile *tile)
+{
+ while (TRUE)
+ {
+ gint count = g_atomic_int_get (&tile->read_lock_count);
+
+ if (count < 0)
+ {
+ continue;
+ }
+
+ if (g_atomic_int_compare_and_exchange (&tile->read_lock_count,
+ count,
+ count + 1))
+ {
+ break;
+ }
+ }
+}
+
+void
+gegl_tile_read_unlock (GeglTile *tile)
+{
+ g_atomic_int_dec_and_test (&tile->read_lock_count);
+}
+
void
gegl_tile_mark_as_stored (GeglTile *tile)
{
diff --git a/gegl/buffer/gegl-tile.h b/gegl/buffer/gegl-tile.h
index 27b8b6ea9..16e69d149 100644
--- a/gegl/buffer/gegl-tile.h
+++ b/gegl/buffer/gegl-tile.h
@@ -38,6 +38,16 @@ void gegl_tile_lock (GeglTile *tile);
*/
void gegl_tile_unlock (GeglTile *tile);
+/* lock a tile for reading, this would allow reading from buffers
+ * later gotten with get_data()
+ */
+void gegl_tile_read_lock (GeglTile *tile);
+
+/* unlock the tile notifying the tile that we're done reading
+ * the data.
+ */
+void gegl_tile_read_unlock (GeglTile *tile);
+
void gegl_tile_mark_as_stored (GeglTile *tile);
gboolean gegl_tile_is_stored (GeglTile *tile);
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]