On Mon, Apr 29, 2024 at 09:29:15AM -0700, Ashutosh Dixit wrote: > Switching from xe_device_mem_access_get/put to xe_pm_runtime_get/put > results in the following WARNING in xe_oa: > > [11614.356168] xe 0000:00:02.0: Missing outer runtime PM protection > [11614.356187] WARNING: CPU: 1 PID: 13075 at drivers/gpu/drm/xe/xe_pm.c:549 xe_pm_runtime_get_noresume+0x60/0x80 [xe] > ... > [11614.356377] Call Trace: > [11614.356379] <TASK> > [11614.356381] ? __warn+0x7e/0x180 > [11614.356387] ? xe_pm_runtime_get_noresume+0x60/0x80 [xe] > [11614.356507] xe_ggtt_remove_node+0x22/0x80 [xe] > [11614.356546] xe_ttm_bo_destroy+0xea/0xf0 [xe] > [11614.356579] xe_oa_stream_destroy+0xf7/0x120 [xe] > [11614.356627] xe_oa_release+0x35/0xc0 [xe] > [11614.356673] __fput+0xa1/0x2d0 > [11614.356679] __x64_sys_close+0x37/0x80 > [11614.356697] do_syscall_64+0x6d/0x140 > [11614.356700] entry_SYSCALL_64_after_hwframe+0x71/0x79 > [11614.356702] RIP: 0033:0x7f2b37314f67 > > There seems to be no reason to use xe_pm_runtime_get_noresume in xe_ggtt > functions. Just use xe_pm_runtime_get. > > Signed-off-by: Ashutosh Dixit <ashutosh.dixit@xxxxxxxxx> > --- > drivers/gpu/drm/xe/xe_ggtt.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/xe/xe_ggtt.c b/drivers/gpu/drm/xe/xe_ggtt.c > index 0d541f55b4fc..8548a2eb3b32 100644 > --- a/drivers/gpu/drm/xe/xe_ggtt.c > +++ b/drivers/gpu/drm/xe/xe_ggtt.c > @@ -404,7 +404,7 @@ static int __xe_ggtt_insert_bo_at(struct xe_ggtt *ggtt, struct xe_bo *bo, > if (err) > return err; > > - xe_pm_runtime_get_noresume(tile_to_xe(ggtt->tile)); > + xe_pm_runtime_get(tile_to_xe(ggtt->tile)); > mutex_lock(&ggtt->lock); > err = drm_mm_insert_node_in_range(&ggtt->mm, &bo->ggtt_node, bo->size, > alignment, 0, start, end, 0); > @@ -433,7 +433,7 @@ int xe_ggtt_insert_bo(struct xe_ggtt *ggtt, struct xe_bo *bo) > void xe_ggtt_remove_node(struct xe_ggtt *ggtt, struct drm_mm_node *node, > bool invalidate) > { > - xe_pm_runtime_get_noresume(tile_to_xe(ggtt->tile)); > + xe_pm_runtime_get(tile_to_xe(ggtt->tile)); we cannot do this as this place gets called from locked places. This is a deadlock risk. We need to ensure to have an outer caller of the xe_pm_runtime_get that will ensure to get the device waked first, then then we continue with the _noresume variant here that only ensures that we have an extra reference. These warnings are indeed poping up in multiple places, and this is a good thing since we killed the mem_access... at least now we know and have a backtrace of the places that are putting our device at risk of deadlock and can use this information to now find the right outer place protections. https://gitlab.freedesktop.org/drm/xe/kernel/issues/1705 > > mutex_lock(&ggtt->lock); > xe_ggtt_clear(ggtt, node->start, node->size); > -- > 2.41.0 >