Re: [PATCH v2 1/7] drm/i915: Flush submission tasklet after bumping priority

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Quoting Tvrtko Ursulin (2018-05-08 10:40:43)
> 
> On 07/05/2018 14:57, Chris Wilson wrote:
> > When called from process context tasklet_schedule() defers itself to
> > ksoftirqd. From experience this may cause unacceptable latencies of over
> > 200ms in executing the submission tasklet, our goal is to reprioritise
> > the HW execution queue and trigger HW preemption immediately, so disable
> > bh over the call to schedule and force the tasklet to run afterwards if
> > scheduled.
> > 
> > v2: Keep rcu_read_lock() around for PREEMPT_RCU
> > 
> > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> > Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
> > ---
> >   drivers/gpu/drm/i915/i915_gem.c | 4 +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index 5ece6ae4bdff..89bf5d67cb74 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -578,10 +578,12 @@ static void __fence_set_priority(struct dma_fence *fence,
> >       rq = to_request(fence);
> >       engine = rq->engine;
> >   
> > -     rcu_read_lock();
> > +     local_bh_disable();
> > +     rcu_read_lock(); /* RCU serialisation for set-wedged protection */
> >       if (engine->schedule)
> >               engine->schedule(rq, attr);
> >       rcu_read_unlock();
> > +     local_bh_enable(); /* kick the tasklets if queues were reprioritised */
> >   }
> >   
> >   static void fence_set_priority(struct dma_fence *fence,
> > 
> 
> Is the sequence not the wrong way around? I would expect disable 
> preemption, then disable softirq, order for disable, on ofc opposite for 
> enable.

We disable preemption (i.e. softirq) then mark ->schedule as being RCU
protected; unwrap and re-enable preemption (kicking softirq tasklets).

I felt it better to keep the RCU tight to ->schedule than let it wrap
local_bh_enable() suggesting that the tasklets might need additional
protection.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux