Re: [PATCH 2/2] drm/i915: Use atomic waits for short non-atomic ones

Chris Wilson
2016-06-28T15:39:45Z

On Tue, Jun 28, 2016 at 03:40:24PM +0100, Tvrtko Ursulin wrote:
>
> On 28/06/16 15:14, Chris Wilson wrote:
> >On Tue, Jun 28, 2016 at 02:55:28PM +0100, Chris Wilson wrote:
> >>On Tue, Jun 28, 2016 at 02:29:33PM +0100, Tvrtko Ursulin wrote:
> >>>How would you implement it with cpu_clock? What would you do when
> >>>re-scheduled?
> >>
> >>unsigned long base;
> >>int cpu;
> >>int ret;
> >>
> >>preempt_disable();
> >>cpu = smp_processor_id();
> >>base = local_clock() >> 10;
> >>for (;;) {
> >> u64 now = local_clock() >> 10;
> >> preempt_enable();
> >>
> >> if (COND) {
> >> ret = 0;
> >> break;
> >> }
> >>
> >> if (now - base >= timeout) {
> >> ret = -ETIMEOUT;
> >> break;
> >> }
> >>
> >> cpu_relax();
> >>
> >> preempt_disable();
> >> if (unlikely(cpu != smp_processor_id()) {
> >> timeout -= now - base;
> >
> >For this, we should scale everything to ns (u64).
>
> In other words not scale. Is this type of loop more preferable to
> you guys vs how it looked in this original patch?
>
> Only difference is the preempt off section is shorter here, but I
> don't think that is interesting for the atomic waits case.

Right, for the current (correct) atomic users we simply don't care. It's
the sleepers where adding a 10us unpreemptible section is likely to be
frowned upon. I wonder if we should even go as far as adding a
cond_resched().
-Chris

--
Chris Wilson, Intel Open Source Technology Centre