Intel-XE Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Matthew Brost <matthew.brost@intel.com>
To: Matthew Auld <matthew.auld@intel.com>
Cc: Dnyaneshwar Bhadane <dnyaneshwar.bhadane@intel.com>,
	<intel-xe@lists.freedesktop.org>
Subject: Re: [PATCH] xe/userptr: TLB invalidation new pm_runtime ref before submit
Date: Wed, 15 Jul 2026 15:11:02 -0700	[thread overview]
Message-ID: <algFdkgrDdGGsjS/@gsse-cloud1.jf.intel.com> (raw)
In-Reply-To: <alfmLCNnyyXZgOo0@gsse-cloud1.jf.intel.com>

On Wed, Jul 15, 2026 at 12:57:32PM -0700, Matthew Brost wrote:
> On Wed, Jul 15, 2026 at 04:02:58PM +0100, Matthew Auld wrote:
> > On 15/07/2026 15:01, Dnyaneshwar Bhadane wrote:
> > > Take a proper runtime PM reference before TLB invalidation.
> > > The MMU notifier path has no outer xe_pm_runtime_get()
> > > protection, but xe_tlb_inval_fence_init() called from
> > > xe_vm_invalidate_vma_submit() uses xe_pm_runtime_get_noresume()
> > > which requires an existing reference. The fence itself will
> > > hold its own noresume ref until it signals.
> > 
> > Is this fixing a real issue? Do you have a link to a bug report or a splat?
> > 
> > I think fault_mode implies LR, and LR vm is always holding an RPM ref until
> > destroyed?
> 
> Yes, we should a PM reference if a LR VM is open.
> 
> > 
> > > 
> > > Signed-off-by: Dnyaneshwar Bhadane <dnyaneshwar.bhadane@intel.com>
> > > ---
> > >   drivers/gpu/drm/xe/xe_userptr.c | 2 ++
> > >   1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/xe/xe_userptr.c b/drivers/gpu/drm/xe/xe_userptr.c
> > > index 1d2ab678faf5..fc7e4b00fe17 100644
> > > --- a/drivers/gpu/drm/xe/xe_userptr.c
> > > +++ b/drivers/gpu/drm/xe/xe_userptr.c
> > > @@ -8,6 +8,7 @@
> > >   #include <linux/mm.h>
> > > +#include "xe_pm.h"
> > >   #include "xe_tlb_inval.h"
> > >   #include "xe_trace_bo.h"
> > > @@ -103,6 +104,7 @@ xe_vma_userptr_do_inval(struct xe_vm *vm, struct xe_userptr_vma *uvma, bool is_d
> > >   	XE_WARN_ON(err <= 0);
> > >   	if (xe_vm_in_fault_mode(vm) && userptr->initial_bind) {
> > > +		guard(xe_pm_runtime)(vm->xe);
> 
> Also, this is illegal as far as I know. If taking a PM reference wakes
> the device, we may allocate memory and acquire a number of locks that
> are not permitted in this path, since we are in the reclaim path here.
> Lockdep should complain about this, but CI is passing, which suggests
> to me that we have an issue somewhere in the lockdep annotations. We
> should probably investigate why this is not triggering failures in CI.
> 

I quickly checked, and this patch does trigger a lockdep splat on my
BMG system (pasted below).

The only explanation I can think of is that CI does not have any
devices that support D3cold. If that's the case, we should look into
adding such devices to CI.

Matt

[  117.955089] [IGT] xe_exec_fault_mode: starting subtest twice-userptr-invalidate-imm
[  117.966389] ======================================================
[  117.966392] WARNING: possible circular locking dependency detected
[  117.966394] 7.2.0-rc3-xe+ #1223 Tainted: G        W
[  117.966397] ------------------------------------------------------
[  117.966400] xe_exec_fault_m/3038 is trying to acquire lock:
[  117.966402] ffffffffa04dd940 (xe_rpm_d3cold_map){+.+.}-{0:0}, at: xe_pm_runtime_get+0x6d/0x150 [xe]
[  117.966545]
               but task is already holding lock:
[  117.966548] ffff888110b11238 (&gpusvm->notifier_lock){++++}-{3:3}, at: xe_vma_userptr_invalidate_start+0x101/0x5f0 [xe]
[  117.966642]
               which lock already depends on the new lock.

[  117.966646]
               the existing dependency chain (in reverse order) is:
[  117.966649]
               -> #2 (&gpusvm->notifier_lock){++++}-{3:3}:
[  117.966655]        down_read+0x2c/0x140
[  117.966661]        xe_pt_svm_userptr_pre_commit+0xa0/0x2b0 [xe]
[  117.966777]        xe_migrate_update_pgtables+0x53/0x9b0 [xe]
[  117.966889]        xe_pt_update_ops_run+0x242/0x880 [xe]
[  117.966996]        ops_execute+0x291/0x8d0 [xe]
[  117.967119]        vm_bind_ioctl_ops_execute+0x2c5/0x5a0 [xe]
[  117.967245]        xe_vm_bind_ioctl+0x19ec/0x1af0 [xe]
[  117.967368]        drm_ioctl_kernel+0xa6/0x100
[  117.967373]        drm_ioctl+0x219/0x450
[  117.967377]        xe_drm_ioctl+0x5d/0xa0 [xe]
[  117.967466]        __x64_sys_ioctl+0x7f/0xd0
[  117.967471]        do_syscall_64+0xca/0x530
[  117.967476]        entry_SYSCALL_64_after_hwframe+0x76/0x7e
[  117.967481]
               -> #1 (reservation_ww_class_mutex){+.+.}-{3:3}:
[  117.967487]        __ww_mutex_lock.constprop.0+0xc3/0x1900
[  117.967492]        ww_mutex_lock+0x26/0x90
[  117.967496]        xe_pm_runtime_lockdep_prime+0x51/0xe0 [xe]
[  117.967610]        match_bound_vga+0x19/0x40 [snd_hda_codec_hdmi]
[  117.967616]        __pfx_is_hdmi_pcm_attached+0xb/0x10 [snd_hda_codec_hdmi]
[  117.967621]        do_one_initcall+0x59/0x2b0
[  117.967626]        do_init_module+0x5f/0x240
[  117.967631]        init_module_from_file+0xc7/0xe0
[  117.967636]        idempotent_init_module+0x176/0x270
[  117.967641]        __x64_sys_finit_module+0x61/0xb0
[  117.967646]        do_syscall_64+0xca/0x530
[  117.967651]        entry_SYSCALL_64_after_hwframe+0x76/0x7e
[  117.967655]
               -> #0 (xe_rpm_d3cold_map){+.+.}-{0:0}:
[  117.967660]        __lock_acquire+0x139d/0x23b0
[  117.967665]        lock_acquire+0xbd/0x2e0
[  117.967668]        xe_pm_runtime_get+0x8b/0x150 [xe]
[  117.967783]        xe_vma_userptr_do_inval+0xaa/0x170 [xe]
[  117.967873]        xe_vma_userptr_invalidate_start+0x30e/0x5f0 [xe]
[  117.967959]        __mmu_notifier_invalidate_range_start+0x21f/0x2f0
[  117.967966]        unmap_vmas+0x1a8/0x1c0
[  117.967971]        unmap_region+0x82/0xe0
[  117.967975]        __mmap_region+0xcb1/0x1000
[  117.967980]        do_mmap+0x4fb/0x660
[  117.967983]        vm_mmap_pgoff+0xae/0x190
[  117.967989]        do_syscall_64+0xca/0x530
[  117.967994]        entry_SYSCALL_64_after_hwframe+0x76/0x7e
[  117.967998]
               other info that might help us debug this:

[  117.968002] Chain exists of:
                 xe_rpm_d3cold_map --> reservation_ww_class_mutex --> &gpusvm->notifier_lock

[  117.968011]  Possible unsafe locking scenario:

[  117.968014]        CPU0                    CPU1
[  117.968017]        ----                    ----
[  117.968019]   lock(&gpusvm->notifier_lock);
[  117.968022]                                lock(reservation_ww_class_mutex);
[  117.968027]                                lock(&gpusvm->notifier_lock);
[  117.968031]   lock(xe_rpm_d3cold_map);
[  117.968035]
                *** DEADLOCK ***

[  117.968038] 3 locks held by xe_exec_fault_m/3038:
[  117.968041]  #0: ffff8881119f8fb8 (&mm->mmap_lock){++++}-{3:3}, at: vm_mmap_pgoff+0x6a/0x190
[  117.968051]  #1: ffffffff82baade0 (mmu_notifier_invalidate_range_start){+.+.}-{0:0}, at: unmap_vmas+0x75/0x1c0
[  117.968059]  #2: ffff888110b11238 (&gpusvm->notifier_lock){++++}-{3:3}, at: xe_vma_userptr_invalidate_start+0x101/0x5f0 [xe]
[  117.968147]
               stack backtrace:
[  117.968150] CPU: 8 UID: 0 PID: 3038 Comm: xe_exec_fault_m Tainted: G        W           7.2.0-rc3-xe+ #1223 PREEMPT(full)
[  117.968153] Tainted: [W]=WARN
[  117.968153] Hardware name: Intel Corporation Raptor Lake Client Platform/RPL-S ADP-S DDR5 UDIMM CRB, BIOS RPLSFWI1.R00.5045.A00.2401260733 01/26/2024
[  117.968154] Call Trace:
[  117.968155]  <TASK>
[  117.968156]  dump_stack_lvl+0x68/0xa0
[  117.968160]  print_circular_bug+0x2c6/0x3f0
[  117.968163]  check_noncircular+0x12d/0x150
[  117.968167]  __lock_acquire+0x139d/0x23b0
[  117.968169]  lock_acquire+0xbd/0x2e0
[  117.968171]  ? xe_pm_runtime_get+0x6d/0x150 [xe]
[  117.968275]  ? dma_resv_iter_first_unlocked+0x189/0x230
[  117.968278]  ? xe_pm_runtime_get+0x6d/0x150 [xe]
[  117.968376]  xe_pm_runtime_get+0x8b/0x150 [xe]
[  117.968475]  ? xe_pm_runtime_get+0x6d/0x150 [xe]
[  117.968574]  xe_vma_userptr_do_inval+0xaa/0x170 [xe]
[  117.968648]  xe_vma_userptr_invalidate_start+0x30e/0x5f0 [xe]
[  117.968718]  __mmu_notifier_invalidate_range_start+0x21f/0x2f0
[  117.968720]  ? lock_acquire+0xbd/0x2e0
[  117.968722]  unmap_vmas+0x1a8/0x1c0
[  117.968725]  unmap_region+0x82/0xe0
[  117.968727]  ? vms_gather_munmap_vmas+0x2f0/0x340
[  117.968729]  __mmap_region+0xcb1/0x1000
[  117.968739]  do_mmap+0x4fb/0x660
[  117.968741]  vm_mmap_pgoff+0xae/0x190
[  117.968745]  do_syscall_64+0xca/0x530
[  117.968748]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
[  117.968749] RIP: 0033:0x7f0fdf5331ec
[  117.968750] Code: 1e fa 41 f7 c1 ff 0f 00 00 75 33 55 48 89 e5 41 54 41 89 cc 53 48 89 fb 48 85 ff 74 41 45 89 e2 48 89 df b8 09 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 7c 5b 41 5c 5d c3 0f 1f 80 00 00 00 00 48 8b
[  117.968752] RSP: 002b:00007ffc9e966020 EFLAGS: 00000206 ORIG_RAX: 0000000000000009
[  117.968753] RAX: ffffffffffffffda RBX: 00007fadeadbe000 RCX: 00007f0fdf5331ec
[  117.968754] RDX: 0000000000000003 RSI: 0000000000002000 RDI: 00007fadeadbe000
[  117.968755] RBP: 00007ffc9e966030 R08: 00000000ffffffff R09: 0000000000000000
[  117.968756] R10: 0000000000000031 R11: 0000000000000206 R12: 0000000000000031
[  117.968756] R13: 0000000000000025 R14: 00007fadeadbe000 R15: 0000000000000000
[  117.968759]  </TASK>

> If we need a PM reference here, xe_pm_runtime_get_noresume() has the
> correct semantics. In the TLB invalidation code paths, we do exactly
> this when arming the TLB invalidation fence; see
> xe_tlb_inval_fence_init(), if that is failing then we have problem
> somewhere else in the code that this code path can be reached without a
> PM reference.
> 
> Matt
> 
> > >   		if (!userptr->finish_inuse) {
> > >   			/*
> > >   			 * Defer the TLB wait to an extra pass so the caller
> > 

  reply	other threads:[~2026-07-15 22:11 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-15 14:01 [PATCH] xe/userptr: TLB invalidation new pm_runtime ref before submit Dnyaneshwar Bhadane
2026-07-15 14:12 ` ✓ CI.KUnit: success for " Patchwork
2026-07-15 15:02 ` [PATCH] " Matthew Auld
2026-07-15 19:57   ` Matthew Brost
2026-07-15 22:11     ` Matthew Brost [this message]
2026-07-15 15:06 ` ✓ Xe.CI.BAT: success for " Patchwork
2026-07-15 16:36 ` ✓ Xe.CI.FULL: " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=algFdkgrDdGGsjS/@gsse-cloud1.jf.intel.com \
    --to=matthew.brost@intel.com \
    --cc=dnyaneshwar.bhadane@intel.com \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=matthew.auld@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox