[Intel-gfx] [PATCH i-g-t 2/3] lib: Align ring measurement to timer

[Chris Wilson]

Quoting Antonio Argenziano (2018-05-31 15:42:03)
> 
> 
> On 30/05/18 12:52, Chris Wilson wrote:
> > Quoting Antonio Argenziano (2018-05-30 18:30:36)
> >>
> >>
> >> On 30/05/18 03:33, Chris Wilson wrote:
> >>> After hitting the SIGINT from execbuf, wait until the next timer signal
> >>> before trying again. This aligns the start of the ioctl to the timer,
> >>> hopefully maximising the amount of time we have for processing before
> >>> the next signal -- trying to prevent the case where we are scheduled out
> >>> in the middle of processing and so hit the timer signal too early.
> >>>
> >>> References: https://bugs.freedesktop.org/show_bug.cgi?id=106695
> >>> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> >>
> >> Not sure I understand what is the sequence of events, is the problem we
> >> get a signal in the middle of a 'good' execbuf and exit the while loop
> >> prematurely? If so maybe we can also think of making the timer 'VIRTUAL'
> >> so that it would decrement only when the process is executing.
> > 
> > If it's VIRTUAL it'll never fire when we wait for space (as being asleep
> > no user/sys time is consumed).
> > 
> > The only way I can explain 106695 would be with some very strange
> > scheduler behaviour, but even then it requires us to hit a path where we
> > actually check for a pending signal -- which should only happen when we
> > run out of ring space for this setup. Not even the device being wedged
> > (which it wasn't) would cause the ring to drain. Possibly going over 10s
> > and the cork being unplugged? Very stange.
> 
> Just a bit concerned that we might be covering up some weird corner case 
> where we are sleeping where we shouldn't.

The bugs are exactly the opposite, where there's a signal pending and we
ignore it ;)

And ignore them we do. If one day someone cares signal latency, we have
a lot of explaining to do...

It's just worrying because the only signal_pending() check we expect to
hit is in wait-for-space (i915_request_wait to be precise); and that
should be consistent between calls to execbuf. However, it's not meant
to be a defining test of user behaviour, just exploiting the limitation
of the implementation to report said limitation. All that we must do
is to be sure that we don't over-report or else the callers will hang
during their setup and fail. Under reporting is a nuisance, but not a
huge issue.
-Chris