| Message ID | 20190204084116.3013-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/12] drm/i915: Allow normal clients to always preempt idle priority clients | expand |
On 04/02/2019 08:41, Chris Wilson wrote: > When first enabling preemption, we hesitated from making it a free-for-all > where every higher priority client would force a preempt-to-idle cycle > and take over from all lower priority clients. We hesitated because we > were uncertain just how well preemption would work in practice, whether > the preemption latency itself would detract from the latency gains for > higher priority tasks and whether it would work at all. Since > introducing preemption, we have been enabling it for more common tasks, > even giving normal clients a small preemptive boost when they first > start (to aide fairness and improve interactivity). Now lets take one > step further and give permission for all normal (priority:0) clients to > preempt any idle (priority:<0) task so that users running long compute > jobs do not overly impact other jobs (i.e. their desktop) and the system > remains responsive under such idle loads. > > References: f6322eddaff7 ("drm/i915/preemption: Allow preemption between submission ports") > References: b16c765122f9 ("drm/i915: Priority boost for new clients") > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > Cc: Michał Winiarski <michal.winiarski@intel.com> > Cc: "Bloomfield, Jon" <jon.bloomfield@intel.com> > Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > --- > drivers/gpu/drm/i915/intel_ringbuffer.h | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h > index 34d0a148e664..983ad1e7914d 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h > @@ -592,7 +592,20 @@ intel_engine_has_preemption(const struct intel_engine_cs *engine) > > static inline bool __execlists_need_preempt(int prio, int last) > { > - return prio > max(0, last); > + /* > + * Allow preemption of low -> normal -> high, but we do > + * not allow low priority tasks to preempt other low priority > + * tasks under the impression that latency for low priority > + * tasks does not matter (as much as background throughput), > + * so kiss. > + * > + * More naturally we would write > + * prio >= max(0, last); > + * except that we wish to prevent triggering preemption at the same > + * priority level: the task that is running should remain running > + * to preserve FIFO ordering of dependencies. > + */ > + return prio > max(I915_PRIORITY_NORMAL - 1, last); > } > > static inline void > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Regards, Tvrtko
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 34d0a148e664..983ad1e7914d 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -592,7 +592,20 @@ intel_engine_has_preemption(const struct intel_engine_cs *engine) static inline bool __execlists_need_preempt(int prio, int last) { - return prio > max(0, last); + /* + * Allow preemption of low -> normal -> high, but we do + * not allow low priority tasks to preempt other low priority + * tasks under the impression that latency for low priority + * tasks does not matter (as much as background throughput), + * so kiss. + * + * More naturally we would write + * prio >= max(0, last); + * except that we wish to prevent triggering preemption at the same + * priority level: the task that is running should remain running + * to preserve FIFO ordering of dependencies. + */ + return prio > max(I915_PRIORITY_NORMAL - 1, last); } static inline void
When first enabling preemption, we hesitated from making it a free-for-all where every higher priority client would force a preempt-to-idle cycle and take over from all lower priority clients. We hesitated because we were uncertain just how well preemption would work in practice, whether the preemption latency itself would detract from the latency gains for higher priority tasks and whether it would work at all. Since introducing preemption, we have been enabling it for more common tasks, even giving normal clients a small preemptive boost when they first start (to aide fairness and improve interactivity). Now lets take one step further and give permission for all normal (priority:0) clients to preempt any idle (priority:<0) task so that users running long compute jobs do not overly impact other jobs (i.e. their desktop) and the system remains responsive under such idle loads. References: f6322eddaff7 ("drm/i915/preemption: Allow preemption between submission ports") References: b16c765122f9 ("drm/i915: Priority boost for new clients") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: Michał Winiarski <michal.winiarski@intel.com> Cc: "Bloomfield, Jon" <jon.bloomfield@intel.com> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> --- drivers/gpu/drm/i915/intel_ringbuffer.h | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)