Quoting Mika Kuoppala (2020-03-09 15:05:45) > Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes: > > > [ 3783.276728] BUG: KCSAN: data-race in __i915_request_submit [i915] / i915_request_await_dma_fence [i915] > > [ 3783.276766] > > [ 3783.276787] write to 0xffff8881f1bc60a0 of 1 bytes by interrupt on cpu 2: > > [ 3783.277187] __i915_request_submit+0x47e/0x4a0 [i915] > > [ 3783.277580] __execlists_submission_tasklet+0x997/0x2780 [i915] > > [ 3783.277973] execlists_submission_tasklet+0xd3/0x170 [i915] > > [ 3783.278006] tasklet_action_common.isra.0+0x42/0xa0 > > [ 3783.278035] __do_softirq+0xd7/0x2cd > > [ 3783.278063] irq_exit+0xbe/0xe0 > > [ 3783.278089] do_IRQ+0x51/0x100 > > [ 3783.278114] ret_from_intr+0x0/0x1c > > [ 3783.278140] finish_task_switch+0x72/0x260 > > [ 3783.278170] __schedule+0x1e5/0x510 > > [ 3783.278198] schedule+0x45/0xb0 > > [ 3783.278226] smpboot_thread_fn+0x23e/0x300 > > [ 3783.278256] kthread+0x19a/0x1e0 > > [ 3783.278283] ret_from_fork+0x1f/0x30 > > [ 3783.278305] > > [ 3783.278327] read to 0xffff8881f1bc60a0 of 1 bytes by task 19440 on cpu 3: > > [ 3783.278724] i915_request_await_dma_fence+0x2a6/0x530 [i915] > > [ 3783.279130] i915_request_await_object+0x2fe/0x470 [i915] > > [ 3783.279524] i915_gem_do_execbuffer+0x45dc/0x4c20 [i915] > > [ 3783.279908] i915_gem_execbuffer2_ioctl+0x2c3/0x580 [i915] > > [ 3783.279940] drm_ioctl_kernel+0xe4/0x120 > > [ 3783.279968] drm_ioctl+0x297/0x4c7 > > [ 3783.279996] ksys_ioctl+0x89/0xb0 > > [ 3783.280021] __x64_sys_ioctl+0x42/0x60 > > [ 3783.280047] do_syscall_64+0x6e/0x2c0 > > [ 3783.280074] entry_SYSCALL_64_after_hwframe+0x44/0xa9 > > > > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > > --- > > drivers/gpu/drm/i915/i915_request.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c > > index 5de3989b6c4f..04b52bf347bf 100644 > > --- a/drivers/gpu/drm/i915/i915_request.c > > +++ b/drivers/gpu/drm/i915/i915_request.c > > @@ -907,7 +907,7 @@ already_busywaiting(struct i915_request *rq) > > * > > * See the are-we-too-late? check in __i915_request_submit(). > > */ > > - return rq->sched.semaphores | rq->engine->saturated; > > + return rq->sched.semaphores | READ_ONCE(rq->engine->saturated); > > This seem to be a one way ticket to saturation (until parking it is). > Usually there should be the WRITE_ONCE counterpair! > > So now the question is that does it really matter if the race happens. It's a safety net, to turn off semaphore interengine synchronisation when fully occupied (polling the semaphore caused noticeable extra bus traffic). There's a small disparity in that we do want the switch to be global, as any process still trying to utilise semaphores is disadvantaged by the scheduler, which results in unfair scheduling. However, such transition will always be asynchronous with the work already queued, with new work being preferred over older queues. :( I do want to reinstate semaphores as there are measurable performance improvements (both in throughput as well as obvious latency improvement) and this safety net spoils the fun. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
