Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes: > Quoting Mika Kuoppala (2020-02-11 15:23:24) >> Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes: >> > + /* Kick the tasklet to process the error */ >> > + intel_engine_flush_submission(engine); >> >> This pattern of usage in selftests makes me want to add mb(); to >> intel_engine_flush_submission(), as it does not seem to do it >> implicitly. >> >> We want to flush submission and observe the effects, thus >> it seems like a good place. > > Hmm. From the cpu perspective the memory barrier would be on the other > side in clear_bit_unlock(), and flush_submission does unlock_wait. > > But, then, we have to factor in that we have to communicate with an > external client the GPU, so perhaps an explicit memory barrier... > > We certainly do perform the memory barrier in order to set the GPU in > motion, but have not relied on them for observing effects (aside from > the CSB ringbuf). > > I don't see a strong argument that adding a 'mb/rmb' here will make any > difference, I don't see what we are pairing with from the GPU > perspective. But maybe there is? I don't have a strong argument from gpu side. But what if the flush only does the nonatomic wait and returns, and our CPU has read the state up front for the next comparison. Or now thinking it, the saving grace is that if we don't need to flush in here, the tasklet has finished and finish has the barrier? -Mika _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
