asahi: Make bo->writer_syncobj atomic
authorAsahi Lina <lina@asahilina.net>
Wed, 10 May 2023 07:02:27 +0000 (16:02 +0900)
committerMarge Bot <emma+marge@anholt.net>
Thu, 11 May 2023 23:24:48 +0000 (23:24 +0000)
BOs can be written from several contexts, so writing to this member is
racy. We only care about this for the purposes of exporting BOs after a
submission (and if the app is racing writers/submissions at that point
all bets are off), so just keeping track of the last written value is
sufficient.

Switch to atomic operations to eliminate the race, and drop the assert
in the batch cleanup path that no longer holds when the BO might have
been written to from another context.

Fixes: asahi/mesa#20

Signed-off-by: Asahi Lina <lina@asahilina.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/22971>

src/asahi/lib/agx_bo.c
src/asahi/lib/agx_bo.h
src/asahi/lib/agx_device.c
src/gallium/drivers/asahi/agx_batch.c

index 3e9e608..d3f1059 100644 (file)
@@ -193,7 +193,7 @@ agx_bo_unreference(struct agx_bo *bo)
     * lock, let's make sure it's still not referenced before freeing it.
     */
    if (p_atomic_read(&bo->refcnt) == 0) {
-      assert(!bo->writer_syncobj);
+      assert(!p_atomic_read_relaxed(&bo->writer_syncobj));
 
       if (dev->debug & AGX_DBG_TRACE)
          agxdecode_track_free(bo);
index 58daf57..9135290 100644 (file)
@@ -81,7 +81,7 @@ struct agx_bo {
    int prime_fd;
 
    /* Syncobj handle of the current writer, if any */
-   int writer_syncobj;
+   uint32_t writer_syncobj;
 
    /* Globally unique value (system wide) for tracing. Exists for resources,
     * command buffers, GPU submissions, segments, segmentent lists, encoders,
index a3f5100..bcfe49a 100644 (file)
@@ -252,10 +252,11 @@ agx_bo_export(struct agx_bo *bo)
       /* If there is a pending writer to this BO, import it into the buffer
        * for implicit sync.
        */
-      if (bo->writer_syncobj) {
+      uint32_t writer_syncobj = p_atomic_read_relaxed(&bo->writer_syncobj);
+      if (writer_syncobj) {
          int out_sync_fd = -1;
-         int ret = drmSyncobjExportSyncFile(bo->dev->fd, bo->writer_syncobj,
-                                            &out_sync_fd);
+         int ret =
+            drmSyncobjExportSyncFile(bo->dev->fd, writer_syncobj, &out_sync_fd);
          assert(ret >= 0);
          assert(out_sync_fd >= 0);
 
index e0eab52..4a4065b 100644 (file)
@@ -165,13 +165,10 @@ agx_batch_cleanup(struct agx_context *ctx, struct agx_batch *batch, bool reset)
          /* There is no more writer on this context for anything we wrote */
          struct agx_batch *writer = agx_writer_get(ctx, handle);
 
-         if (writer == batch) {
-            assert(bo->writer_syncobj == batch->syncobj);
+         if (writer == batch)
             agx_writer_remove(ctx, handle);
-         }
 
-         if (bo->writer_syncobj == batch->syncobj)
-            bo->writer_syncobj = 0;
+         p_atomic_cmpxchg(&bo->writer_syncobj, batch->syncobj, 0);
 
          agx_bo_unreference(agx_lookup_bo(dev, handle));
       }
@@ -654,7 +651,7 @@ agx_batch_submit(struct agx_context *ctx, struct agx_batch *batch,
 
       /* But any BOs written by active batches are ours */
       assert(writer == batch && "exclusive writer");
-      bo->writer_syncobj = batch->syncobj;
+      p_atomic_set(&bo->writer_syncobj, batch->syncobj);
    }
 
    free(in_syncs);