2017-07-11 Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>: > Quoting Chris Wilson (2017-02-14 12:40:01) > > [ 236.821534] WARNING: kmemcheck: Caught 64-bit read from uninitialized memory (ffff8802538683d0) > > [ 236.828642] 420000001e7f0000000000000000000000080000000000000000000000000000 > > [ 236.839543] i i i i u u u u i i i i i i i i u u u u u u u u u u u u u u u u > > [ 236.850420] ^ > > [ 236.854123] RIP: 0010:[<ffffffff81396f07>] [<ffffffff81396f07>] fence_signal+0x17/0xd0 > > [ 236.861313] RSP: 0018:ffff88024acd7ba0 EFLAGS: 00010282 > > [ 236.865027] RAX: ffffffff812f6a90 RBX: ffff8802527ca800 RCX: ffff880252cb30e0 > > [ 236.868801] RDX: ffff88024ac5d918 RSI: ffff880252f780e0 RDI: ffff880253868380 > > [ 236.872579] RBP: ffff88024acd7bc0 R08: ffff88024acd7be0 R09: 0000000000000000 > > [ 236.876407] R10: 0000000000000000 R11: 0000000000000000 R12: ffff880253868380 > > [ 236.880185] R13: ffff8802538684d0 R14: ffff880253868380 R15: ffff88024cd48e00 > > [ 236.883983] FS: 00007f1646d1a740(0000) GS:ffff88025d000000(0000) knlGS:0000000000000000 > > [ 236.890959] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > > [ 236.894702] CR2: ffff880251360318 CR3: 000000024ad21000 CR4: 00000000001406f0 > > [ 236.898481] [<ffffffff8130d1ad>] i915_gem_request_retire+0x1cd/0x230 > > [ 236.902439] [<ffffffff8130e2b3>] i915_gem_request_alloc+0xa3/0x2f0 > > [ 236.906435] [<ffffffff812fb1bd>] i915_gem_do_execbuffer.isra.41+0xb6d/0x18b0 > > [ 236.910434] [<ffffffff812fc265>] i915_gem_execbuffer2+0x95/0x1e0 > > [ 236.914390] [<ffffffff812ad625>] drm_ioctl+0x1e5/0x460 > > [ 236.918275] [<ffffffff8110d4cf>] do_vfs_ioctl+0x8f/0x5c0 > > [ 236.922168] [<ffffffff8110da3c>] SyS_ioctl+0x3c/0x70 > > [ 236.926090] [<ffffffff814b7a5f>] entry_SYSCALL_64_fastpath+0x17/0x93 > > [ 236.930045] [<ffffffffffffffff>] 0xffffffffffffffff > > Ah something that I didn't take into account, and indeed gives this a bit > more urgency than I realised, is that the timestamp is exposed to > userspace. As such we are feeding it garbage, at best. > > The trivial option is just to clear it in dma_fence_init(). I still have > the slight preference for the extra complication here (for the reader) as > it should be quicker for the more common path of signaling the fence. > > > We only set the timestamp before we mark the fence as signaled. It is > > done before to avoid observers having a window in which they may see the > > fence as complete but no timestamp. Having it does incur a potential for > > the timestamp to be written twice, and even for it to be corrupted if > > the u64 write is not atomic. Instead use a new bit to record the > > presence of the timestamp, and teach the readers to wait until it is set > > if the fence is complete. There still remains a race where the timestamp > > for the signaled fence may be shown before the fence is reported as > > signaled, but that's a pre-existing error. > > Now deserves a > Reported-by: Rafael Antognolli <rafael.antognolli@xxxxxxxxx> > > > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > > Cc: Sumit Semwal <sumit.semwal@xxxxxxxxxx> > > Cc: Gustavo Padovan <gustavo@xxxxxxxxxxx> > > Cc: Daniel Vetter <daniel.vetter@xxxxxxxxx> > Cc: Rafael Antognolli <rafael.antognolli@xxxxxxxxx> > > > --- > > drivers/dma-buf/dma-fence.c | 17 ++++++----------- > > drivers/dma-buf/sync_debug.c | 2 +- > > drivers/dma-buf/sync_file.c | 8 +++++++- > > include/linux/dma-fence.h | 2 ++ > > 4 files changed, 16 insertions(+), 13 deletions(-) Finally pushed this one to drm-misc-fixes. Thanks. Gustavo _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
