diff options
| author | Daniel Vetter <[email protected]> | 2017-04-06 09:13:47 +0200 |
|---|---|---|
| committer | Kenneth Graunke <[email protected]> | 2017-04-10 14:33:18 -0700 |
| commit | a99a4979fdb84a41cc3626b27695f02b32ebb98a (patch) | |
| tree | ef0d8b913ed2ebc771a8ec66b0b9b185bedfeaa3 | |
| parent | 7f3c85c21e0e18bbedb2f59ba13838d963f0a106 (diff) | |
i965/batch: Ensure we use a consistent offset in relocs
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 <[email protected]>
Reviewed-by: Kenneth Graunke <[email protected]>
| -rw-r--r-- | src/mesa/drivers/dri/i965/intel_batchbuffer.c | 8 |
1 files 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 |