[Mesa-dev] [PATCH 7/9] i965/batch: Ensure we use a consistent offset in relocs

[Kenneth Graunke]

From: Daniel Vetter <daniel.vetter at ffwll.ch>

In theory gcc is free to re-load them, and if a concurrent
execbuf races and updates bo->offset64 then we have a problem:
execbuffer api requires that the ->presumed_offset and the one
we used for the reloc matches. It does not require that the value
is sensible, which means no locks needed, just a consistent load.

Ken said his next series will nuke this, so just hand-roll the
kernel's READ_ONCE idea inline.

FIXME: Most callers of brw_emit_reloc recompute the relocation
themselves, which means this doesn't really fix the race. But the long
term plan is to move to per-context relocation handling, which will
fix this all properly. So leave this for now as just a reminder.

Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
---
 src/mesa/drivers/dri/i965/intel_batchbuffer.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
index 4c653341506..8ccc5a276b9 100644
--- a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
+++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
@@ -733,6 +733,8 @@ brw_emit_reloc(struct intel_batchbuffer *batch, uint32_t batch_offset,
                struct brw_bo *target, uint32_t target_offset,
                uint32_t read_domains, uint32_t write_domain)
 {
+   uint64_t offset64;
+
    if (batch->reloc_count == batch->reloc_array_size) {
       batch->reloc_array_size *= 2;
       batch->relocs = realloc(batch->relocs,
@@ -752,18 +754,20 @@ brw_emit_reloc(struct intel_batchbuffer *batch, uint32_t batch_offset,
 
    batch->reloc_count++;
 
+   /* ensure gcc doesn't reload */
+   offset64 = *((volatile uint64_t *)&target->offset64);
    reloc->offset = batch_offset;
    reloc->delta = target_offset;
    reloc->target_handle = target->gem_handle;
    reloc->read_domains = read_domains;
    reloc->write_domain = write_domain;
-   reloc->presumed_offset = target->offset64;
+   reloc->presumed_offset = offset64;
 
    /* Using the old buffer offset, write in what the right data would be, in
     * case the buffer doesn't move and we can short-circuit the relocation
     * processing in the kernel
     */
-   return target->offset64 + target_offset;
+   return offset64 + target_offset;
 }
 
 void
-- 
2.12.1