All of 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.