Re: [PATCH] drm/xe/pxp: Don't kill queues while holding the spinlock

[Dan Carpenter <dan.carpenter@linaro.org>]

On Wed, Feb 12, 2025 at 05:26:55PM -0800, Matthew Brost wrote:
> On Wed, Feb 12, 2025 at 04:40:32PM -0800, Daniele Ceraolo Spurio wrote:
> > xe_exec_queue_kill can sleep, so we can't call it from under the lock.
> > We can instead move the queues to a separate list and then kill them all
> > after we release the lock.
> > 
> > Since being in the list is used to track whether RPM cleanup is needed,
> > we can no longer defer that to queue_destroy, so we perform it
> > immediately instead.
> > 
> > Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> > Fixes: f8caa80154c4 ("drm/xe/pxp: Add PXP queue tracking and session start")
> > Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> 
> Patch LGTM but can this actually happen though? i.e. Can or do we enable
> PXP on LR queues?
> 

This isn't really an answer to your question, but when I reported this
bug I didn't notice the if (xe_vm_in_preempt_fence_mode()) check in
xe_vm_remove_compute_exec_queue().  So it's possible that this was a
false positive?

> Also as a follow should be add a might_sleep() to xe_exec_queue_kill to
> catch this type of bug immediately?

There is a might_sleep() in down_write().  If this is a real bug that
would have caught it.  The problem is that people don't generally test
with CONFIG_DEBUG_ATOMIC_SLEEP so the might_sleep() calls are turned off.

regards,
dan carpenter