From 94c9115aa0a44ae18d90a60129a0b44517896e3d Mon Sep 17 00:00:00 2001 From: Asahi Lina Date: Wed, 10 May 2023 16:02:27 +0900 Subject: [PATCH] asahi: Make bo->writer_syncobj atomic 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 Part-of: --- src/asahi/lib/agx_bo.c | 2 +- src/asahi/lib/agx_bo.h | 2 +- src/asahi/lib/agx_device.c | 7 ++++--- src/gallium/drivers/asahi/agx_batch.c | 9 +++------ 4 files changed, 9 insertions(+), 11 deletions(-) diff --git a/src/asahi/lib/agx_bo.c b/src/asahi/lib/agx_bo.c index 3e9e608..d3f1059 100644 --- a/src/asahi/lib/agx_bo.c +++ b/src/asahi/lib/agx_bo.c @@ -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); diff --git a/src/asahi/lib/agx_bo.h b/src/asahi/lib/agx_bo.h index 58daf57..9135290 100644 --- a/src/asahi/lib/agx_bo.h +++ b/src/asahi/lib/agx_bo.h @@ -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, diff --git a/src/asahi/lib/agx_device.c b/src/asahi/lib/agx_device.c index a3f5100..bcfe49a 100644 --- a/src/asahi/lib/agx_device.c +++ b/src/asahi/lib/agx_device.c @@ -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); diff --git a/src/gallium/drivers/asahi/agx_batch.c b/src/gallium/drivers/asahi/agx_batch.c index e0eab52..4a4065b 100644 --- a/src/gallium/drivers/asahi/agx_batch.c +++ b/src/gallium/drivers/asahi/agx_batch.c @@ -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); -- 2.7.4