[PATCH] dix: Fix Client starvation with SmartScheduler priorities

[Chris Wilson]

On Wed, Jan 22, 2014 at 11:04:28AM -0800, Keith Packard wrote:
> Chris Wilson <chris at chris-wilson.co.uk> writes:
> 
> > The solution employed here is to undo the busy demotion after a period
> > of idleness. Currently the idle boost is applied only once for a fresh
> > client when it becomes selectable. The change is to apply that boost for
> > all clients that remain ready for the entire duration of idle timeslice -
> > eventually these old busy clients will be promoted sufficiently to
> > be granted a ScheduleSlice.
> 
> I think your analysis is spot-on, but the fix doesn't seem to match. By
> changing smart_check_tick only when it is older than 'idle', you're
> effectively just bumping smart_priority every other pass through the
> loop. Which explains why you get no starvation; all ready clients get priority
> bumps every other pass through the loop. But that includes the clients
> which are actually running, which definitely isn't what we want.
> 
> I think what we want to do is simply increase the priority of clients
> which haven't run in a while, instead of clients which haven't been
> ready for a while. That's easy to check; we know the last time a client
> ran as that's stored in smart_start_tick.

Wouldn't smart_stop_tick be a slightly better choice as that gives us
the time the client became idle? Only promoting idle ready clients on
each pass (rather than every ready client every other pass) seems far
more in keeping with SmartScheduler!

> Please give this patch a try and see if it has the same effect.

Indeed it does. Not all jitter between identical clients is eliminated,
but I have not had the same issues whereby a set of clients would
entirely stop updating.

Running 16 copies of the fish-demo for 60 seconds,
unpatched: 6136 +- 5586 frames (incl. ~50% stalled clients)
this patch: 5915 +- 264 frames (no stalled clients)

> From a96c954443ab7cb907abda12c540d8b60e78e803 Mon Sep 17 00:00:00 2001
> From: Keith Packard <keithp at keithp.com>
> Date: Wed, 22 Jan 2014 11:01:59 -0800
> Subject: [PATCH] dix: Praise clients which haven't run for a while, rather
>  than idle clients
> 
> A client which is ready, but hasn't run for a while, should receive
> the same benefit as one which has simply been idle for a while. Use
> the smart_start_tick to see how long it has been since a client has
> run instead of smart_check_tick, which got reset each time a client
> was ready, even if it didn't get to run.
> 
> Signed-off-by: Keith Packard <keithp at keithp.com>
Tested-by: Chris Wilson <chris at chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre