[Intel-gfx] [PATCH 3/7] drm/i915: Keep a count of requests submitted from userspace

[Chris Wilson]

Quoting Tvrtko Ursulin (2018-04-09 12:43:50)
> 
> On 09/04/2018 11:51, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2018-04-09 11:40:08)
> >>
> >> On 09/04/2018 11:27, Chris Wilson wrote:
> >>> Quoting Tvrtko Ursulin (2018-04-09 11:17:04)
> >>>>
> >>>> On 09/04/2018 10:25, Chris Wilson wrote:
> >>>>> Downside being that we either then use atomic64 throughout or we mix
> >>>>> atomic32/atomic64 knowing that we're on x86. (I feel like someone else
> >>>>> must have solved this problem in a much neater way, before they went to
> >>>>> per-cpu stats ;)
> >>>>
> >>>> Is the winky implying you know who and where? :) We have three potential
> >>>> solutions now, even for if the winky is suggesting something.
> >>>
> >>> Nah, just that atomic/locked counters are so old hat. Not sure if there
> >>> remain any good examples for hotpath counters that remain applicable to
> >>> our code.
> >>
> >> Leave it as is then for now and improve if we discover it is not good
> >> enough?
> > 
> > I did have an ulterior motive in that the cmpxchg did resolve one issue
> > that irked me with the two counters being updated out of sync. Minor,
> > minor glitches :)
> > 
> > I don't have a strong preference either way. These instructions on the
> > submit are not likely to stand out, as compared to the biggest fish of
> > ksoftirqd, execlists_schedule() and execlists_dequeue().
> 
> I could move the queued decrement from submit_notify to backends, right 
> next to runnable++? Then both would be under the engine->timeline->lock 
> so any inconsistencies in readout I'd hope should be dismissable?

Fair. I have this itch to add a request->state,
	switch (request->state) {
	case QUEUED:
		stats->queued--;
		switch (now) {
			case QUEUED:
				BUG();
			case: READY:
				stats->runnable++;
			case EXEC:
				break;
		}
		break;
	case ...
	}
	request->state = now;

Stop me. Please, stop me.
-Chris