GVT Scheduler

[Wang, Zhi A]

Hi Julian:

Thanks for your message. 

Every workload is addressed with a GPU context. The workload in i915 is submitted by the HW interfaces called execlist. When the workload is finished, the HW will notice the i915 by an interrupt and switch out the context. Then i915 will process the CSB (context status block) and call the shadow_context_status_change through the notifier in the interrupt handler (or tasklet).

I915 also put a breadcrumb after the workload inside the context to figure out if a workload is finished or not.

Now let's see the timeline:

- GVT-g submits a workload to i915.
- i915 append the breadcrumb to the workload
- i915 submits the workload to HW.
- GVT-g called i915_wait_request to wait for the GPU execution passed the breadcrumb. (But the context might not be switched out at this time)
- GVT-g waits for the context to be switched out by the shadow_context_status_change. (Because GVT-g need to copy the content in the shadow context back to the guest context. The shadow context must be idle at this time.)
- No one is going to touch the shadow context anymore and GVT-g call complete_current_workload. 

The race between shadow_context_status_change and complete_current_workload should be addressed in our design. So this problem might be caused by i915 change, e.g. the timing of call shadow_context_status_change is changed. But we will double confirm in GVT-g as well.

The patch you mentioned is for a corner case in GPU reset. But this shouldn't happen in a normal submission flow unless someone breaks the flow above.

Thanks,
Zhi.

-----Original Message-----
From: intel-gvt-dev <intel-gvt-dev-bounces at lists.freedesktop.org> On Behalf Of Julian Stecklina
Sent: Wednesday, September 30, 2020 2:29 PM
To: Intel GVT Dev <intel-gvt-dev at lists.freedesktop.org>
Cc: Thomas Prescher <thomas.prescher at cyberus-technology.de>
Subject: Re: GVT Scheduler

Hello,

we've just found this discussion from 2017 that looks directly related:
https://lists.freedesktop.org/archives/intel-gvt-dev/2017-February/000063.html

Especially, the race between complete_current_workload and shadow_context_status_change looks problematic and reading the current code I cannot convince myself that it's race free.

In our testing we have seen page faults in shadow_context_status_change in the final wake_up(&workload->shadow_ctx_status_wq); call that are hard to explain without a race like that. The backtraces always look like the one below.

We are currently testing with v5.4.68, but I don't see anything relevant being changed in newer versions.

Any pointers are appreciated.

[ 2594.865440] BUG: unable to handle page fault for address: 00000000000263e0 [ 2594.865448] RIP: 0010:[<ffffffff814b3fcb>]
queued_spin_lock_slowpath+0x17b/0x1c0
[...]
[ 2594.865484] Call Trace:
[ 2594.865487]  <IRQ>
[ 2594.865490] _raw_spin_lock_irqsave (kernel/locking/spinlock.c:159) [ 2594.865494] __wake_up_common_lock (kernel/sched/wait.c:123) [ 2594.865499] shadow_context_status_change
(drivers/gpu/drm/i915/gvt/scheduler.c:286)
[ 2594.865501] notifier_call_chain (kernel/notifier.c:104) [ 2594.865504] atomic_notifier_call_chain (kernel/notifier.c:203) [ 2594.865507] process_csb (drivers/gpu/drm/i915/gt/intel_lrc.c:610
drivers/gpu/drm/i915/gt/intel_lrc.c:640
drivers/gpu/drm/i915/gt/intel_lrc.c:1590)
[ 2594.865510] execlists_submission_tasklet
(drivers/gpu/drm/i915/gt/intel_lrc.c:1637)
[ 2594.865514] tasklet_action_common (./arch/x86/include/asm/bitops.h:75
./include/asm-generic/bitops-instrumented.h:57 ./include/linux/interrupt.h:624
kernel/softirq.c:523)
[ 2594.865517] __do_softirq (./arch/x86/include/asm/jump_label.h:25
./include/linux/jump_label.h:200 ./include/trace/events/irq.h:142
kernel/softirq.c:293)
[ 2594.865520] irq_exit (kernel/softirq.c:373 kernel/softirq.c:413) [ 2594.865523] do_IRQ (arch/x86/kernel/irq.c:267) [ 2594.865526] common_interrupt (arch/x86/entry/entry_64.S:890)

Julian

On Tue, 2020-09-29 at 15:03 +0200, Julian Stecklina wrote:
> Hello everyone!
> 
> I'm currently trying to understand the GVT scheduler (gvt/scheduler.c) better.
> I'm specifically trying to understand how the 
> shadow_context_status_change() callback is synchronized with other 
> code that modifies the current_workload array. I would be very 
> grateful, if someone has a couple of minutes to shed some light here. 
> :)
> 
> Can shadow_context_status_change[1] run concurrently with other code 
> that modifies scheduler->current_workload[ring_id]? I see other 
> functions holding
> gvt->sched_lock, but the callback does not.
> 
> If sched_lock is not required in the callback, what currently prevents 
> concurrent exection, e.g. with workload_thread()?
> 
> Thanks!
> Julian
> 
> [1]
> https://elixir.bootlin.com/linux/v5.9-rc7/source/drivers/gpu/drm/i915/
> gvt/scheduler.c#L268
> 
> 
> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev

_______________________________________________
intel-gvt-dev mailing list
intel-gvt-dev at lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev