On 28/06/2018 13:35, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2018-06-28 13:29:32)
On 28/06/2018 13:11, Chris Wilson wrote:
Quoting Chris Wilson (2018-06-28 13:07:51)
Quoting Tvrtko Ursulin (2018-06-28 12:56:56)
And tasklet kick from intel_enable_engine_stats, hm yep. But wouldn't
taking the timeline lock around active state reconstruction solve that
simpler?
Can you? We probably can. (That one was a very recent discovery and
quick fix.)
The biggest issue being whether or not the same locking applies equally
to all submission backends. That's not yet true, but then again we don't
use stats everywhere. So whether or not that's an issue, I don't know,
but it's enough to make me want to punt changing the locking inside
intel_enable_engine_stats to a separate patch.
Big benefit is removing the extra tasklet schedule from engine stats
which is in fact even racy.
It's racy, but the tasklet being run more often than required is just
wasted effort. Unless you think we can get ourselves into a loop here?
We need the state reconstruction to be
atomic so I think it really needs to be under the engine lock.
tasklet_disable/enable can then also be dropped I think.
To which patch in this series that belongs is the question. Last one I
think, when all is in place that port updates are protected by the
timeline lock.
I definitely support it being a new patch. I don't think the race is a
problem that requires preventative work.
I think it has to be part of this series as the last, or before it,
since otherwise the perf_pmu test could/would/should start failing.
Because it is this series which breaks the assumption in
intel_enable_engine_stats that ports can be sampled safely while the
tasklet is disabled.
I don't think the extra tasklet schedule fixes it - it cannot help
correcting the state once the above races with direct submission.
So that extra/new tasklet schedule also goes away.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
