From: Jacek Lawrynowicz jacek.lawrynowicz@linux.intel.com
Fix deadlock in ivpu_ms_cleanup() by preventing runtime resume after file_priv->ms_lock is acquired.
During a failure in runtime resume, a cold boot is executed, which calls ivpu_ms_cleanup_all(). This function calls ivpu_ms_cleanup() that acquires file_priv->ms_lock and causes the deadlock.
Fixes: cdfad4db7756 ("accel/ivpu: Add NPU profiling support") Cc: stable@vger.kernel.org # v6.11+ Signed-off-by: Jacek Lawrynowicz jacek.lawrynowicz@linux.intel.com Signed-off-by: Maciej Falkowski maciej.falkowski@linux.intel.com --- drivers/accel/ivpu/ivpu_ms.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/drivers/accel/ivpu/ivpu_ms.c b/drivers/accel/ivpu/ivpu_ms.c index ffe7b10f8a76..eb485cf15ad6 100644 --- a/drivers/accel/ivpu/ivpu_ms.c +++ b/drivers/accel/ivpu/ivpu_ms.c @@ -4,6 +4,7 @@ */
#include <drm/drm_file.h> +#include <linux/pm_runtime.h>
#include "ivpu_drv.h" #include "ivpu_gem.h" @@ -281,6 +282,9 @@ int ivpu_ms_get_info_ioctl(struct drm_device *dev, void *data, struct drm_file * void ivpu_ms_cleanup(struct ivpu_file_priv *file_priv) { struct ivpu_ms_instance *ms, *tmp; + struct ivpu_device *vdev = file_priv->vdev; + + pm_runtime_get_sync(vdev->drm.dev);
mutex_lock(&file_priv->ms_lock);
@@ -293,6 +297,8 @@ void ivpu_ms_cleanup(struct ivpu_file_priv *file_priv) free_instance(file_priv, ms);
mutex_unlock(&file_priv->ms_lock); + + pm_runtime_put_autosuspend(vdev->drm.dev); }
void ivpu_ms_cleanup_all(struct ivpu_device *vdev)
On 3/25/25 04:43, Maciej Falkowski wrote:
From: Jacek Lawrynowicz jacek.lawrynowicz@linux.intel.com
Fix deadlock in ivpu_ms_cleanup() by preventing runtime resume after file_priv->ms_lock is acquired.
During a failure in runtime resume, a cold boot is executed, which calls ivpu_ms_cleanup_all(). This function calls ivpu_ms_cleanup() that acquires file_priv->ms_lock and causes the deadlock.
Fixes: cdfad4db7756 ("accel/ivpu: Add NPU profiling support") Cc: stable@vger.kernel.org # v6.11+ Signed-off-by: Jacek Lawrynowicz jacek.lawrynowicz@linux.intel.com Signed-off-by: Maciej Falkowski maciej.falkowski@linux.intel.com
drivers/accel/ivpu/ivpu_ms.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/drivers/accel/ivpu/ivpu_ms.c b/drivers/accel/ivpu/ivpu_ms.c index ffe7b10f8a76..eb485cf15ad6 100644 --- a/drivers/accel/ivpu/ivpu_ms.c +++ b/drivers/accel/ivpu/ivpu_ms.c @@ -4,6 +4,7 @@ */ #include <drm/drm_file.h> +#include <linux/pm_runtime.h> #include "ivpu_drv.h" #include "ivpu_gem.h" @@ -281,6 +282,9 @@ int ivpu_ms_get_info_ioctl(struct drm_device *dev, void *data, struct drm_file * void ivpu_ms_cleanup(struct ivpu_file_priv *file_priv) { struct ivpu_ms_instance *ms, *tmp;
- struct ivpu_device *vdev = file_priv->vdev;
- pm_runtime_get_sync(vdev->drm.dev);
Could get_sync() be failed here? Maybe it is better to add warning for failure?
Lizhi
mutex_lock(&file_priv->ms_lock); @@ -293,6 +297,8 @@ void ivpu_ms_cleanup(struct ivpu_file_priv *file_priv) free_instance(file_priv, ms); mutex_unlock(&file_priv->ms_lock);
- pm_runtime_put_autosuspend(vdev->drm.dev); }
void ivpu_ms_cleanup_all(struct ivpu_device *vdev)
Hi,
On 3/25/2025 9:50 PM, Lizhi Hou wrote:
On 3/25/25 04:43, Maciej Falkowski wrote:
From: Jacek Lawrynowicz jacek.lawrynowicz@linux.intel.com
Fix deadlock in ivpu_ms_cleanup() by preventing runtime resume after file_priv->ms_lock is acquired.
During a failure in runtime resume, a cold boot is executed, which calls ivpu_ms_cleanup_all(). This function calls ivpu_ms_cleanup() that acquires file_priv->ms_lock and causes the deadlock.
Fixes: cdfad4db7756 ("accel/ivpu: Add NPU profiling support") Cc: stable@vger.kernel.org # v6.11+ Signed-off-by: Jacek Lawrynowicz jacek.lawrynowicz@linux.intel.com Signed-off-by: Maciej Falkowski maciej.falkowski@linux.intel.com
drivers/accel/ivpu/ivpu_ms.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/drivers/accel/ivpu/ivpu_ms.c b/drivers/accel/ivpu/ivpu_ms.c index ffe7b10f8a76..eb485cf15ad6 100644 --- a/drivers/accel/ivpu/ivpu_ms.c +++ b/drivers/accel/ivpu/ivpu_ms.c @@ -4,6 +4,7 @@ */ #include <drm/drm_file.h> +#include <linux/pm_runtime.h> #include "ivpu_drv.h" #include "ivpu_gem.h" @@ -281,6 +282,9 @@ int ivpu_ms_get_info_ioctl(struct drm_device *dev, void *data, struct drm_file * void ivpu_ms_cleanup(struct ivpu_file_priv *file_priv) { struct ivpu_ms_instance *ms, *tmp; + struct ivpu_device *vdev = file_priv->vdev;
+ pm_runtime_get_sync(vdev->drm.dev);
Could get_sync() be failed here? Maybe it is better to add warning for failure?
Yes, this could fail but we already have detailed warnings in runtime resume callback (ivpu_pm_runtime_resume_cb()).
mutex_lock(&file_priv->ms_lock); @@ -293,6 +297,8 @@ void ivpu_ms_cleanup(struct ivpu_file_priv *file_priv) free_instance(file_priv, ms); mutex_unlock(&file_priv->ms_lock);
+ pm_runtime_put_autosuspend(vdev->drm.dev); } void ivpu_ms_cleanup_all(struct ivpu_device *vdev)
Regards, Jacek
On 3/26/25 01:06, Jacek Lawrynowicz wrote:
Hi,
On 3/25/2025 9:50 PM, Lizhi Hou wrote:
On 3/25/25 04:43, Maciej Falkowski wrote:
From: Jacek Lawrynowicz jacek.lawrynowicz@linux.intel.com
Fix deadlock in ivpu_ms_cleanup() by preventing runtime resume after file_priv->ms_lock is acquired.
During a failure in runtime resume, a cold boot is executed, which calls ivpu_ms_cleanup_all(). This function calls ivpu_ms_cleanup() that acquires file_priv->ms_lock and causes the deadlock.
Fixes: cdfad4db7756 ("accel/ivpu: Add NPU profiling support") Cc: stable@vger.kernel.org # v6.11+ Signed-off-by: Jacek Lawrynowicz jacek.lawrynowicz@linux.intel.com Signed-off-by: Maciej Falkowski maciej.falkowski@linux.intel.com
drivers/accel/ivpu/ivpu_ms.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/drivers/accel/ivpu/ivpu_ms.c b/drivers/accel/ivpu/ivpu_ms.c index ffe7b10f8a76..eb485cf15ad6 100644 --- a/drivers/accel/ivpu/ivpu_ms.c +++ b/drivers/accel/ivpu/ivpu_ms.c @@ -4,6 +4,7 @@ */ #include <drm/drm_file.h> +#include <linux/pm_runtime.h> #include "ivpu_drv.h" #include "ivpu_gem.h" @@ -281,6 +282,9 @@ int ivpu_ms_get_info_ioctl(struct drm_device *dev, void *data, struct drm_file * void ivpu_ms_cleanup(struct ivpu_file_priv *file_priv) { struct ivpu_ms_instance *ms, *tmp; + struct ivpu_device *vdev = file_priv->vdev;
+ pm_runtime_get_sync(vdev->drm.dev);
Could get_sync() be failed here? Maybe it is better to add warning for failure?
Yes, this could fail but we already have detailed warnings in runtime resume callback (ivpu_pm_runtime_resume_cb()).
Will the deadlock still happens if this function fails?
Lizhi
mutex_lock(&file_priv->ms_lock); @@ -293,6 +297,8 @@ void ivpu_ms_cleanup(struct ivpu_file_priv *file_priv) free_instance(file_priv, ms); mutex_unlock(&file_priv->ms_lock);
+ pm_runtime_put_autosuspend(vdev->drm.dev); } void ivpu_ms_cleanup_all(struct ivpu_device *vdev)
Regards, Jacek
On 3/27/2025 6:38 PM, Lizhi Hou wrote:
On 3/26/25 01:06, Jacek Lawrynowicz wrote:
Hi,
On 3/25/2025 9:50 PM, Lizhi Hou wrote:
On 3/25/25 04:43, Maciej Falkowski wrote:
From: Jacek Lawrynowicz jacek.lawrynowicz@linux.intel.com
Fix deadlock in ivpu_ms_cleanup() by preventing runtime resume after file_priv->ms_lock is acquired.
During a failure in runtime resume, a cold boot is executed, which calls ivpu_ms_cleanup_all(). This function calls ivpu_ms_cleanup() that acquires file_priv->ms_lock and causes the deadlock.
Fixes: cdfad4db7756 ("accel/ivpu: Add NPU profiling support") Cc: stable@vger.kernel.org # v6.11+ Signed-off-by: Jacek Lawrynowicz jacek.lawrynowicz@linux.intel.com Signed-off-by: Maciej Falkowski maciej.falkowski@linux.intel.com
drivers/accel/ivpu/ivpu_ms.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/drivers/accel/ivpu/ivpu_ms.c b/drivers/accel/ivpu/ivpu_ms.c index ffe7b10f8a76..eb485cf15ad6 100644 --- a/drivers/accel/ivpu/ivpu_ms.c +++ b/drivers/accel/ivpu/ivpu_ms.c @@ -4,6 +4,7 @@ */ #include <drm/drm_file.h> +#include <linux/pm_runtime.h> #include "ivpu_drv.h" #include "ivpu_gem.h" @@ -281,6 +282,9 @@ int ivpu_ms_get_info_ioctl(struct drm_device *dev, void *data, struct drm_file * void ivpu_ms_cleanup(struct ivpu_file_priv *file_priv) { struct ivpu_ms_instance *ms, *tmp; + struct ivpu_device *vdev = file_priv->vdev;
+ pm_runtime_get_sync(vdev->drm.dev);
Could get_sync() be failed here? Maybe it is better to add warning for failure?
Yes, this could fail but we already have detailed warnings in runtime resume callback (ivpu_pm_runtime_resume_cb()).
Will the deadlock still happens if this function fails?
No. The deadlock was caused by runtime resume in free_instance(). pm_runtime_get_sync() will always bump PM usage counter, so there will be no resume regardless if it fails or not.
mutex_lock(&file_priv->ms_lock); @@ -293,6 +297,8 @@ void ivpu_ms_cleanup(struct ivpu_file_priv *file_priv) free_instance(file_priv, ms); mutex_unlock(&file_priv->ms_lock);
+ pm_runtime_put_autosuspend(vdev->drm.dev); } void ivpu_ms_cleanup_all(struct ivpu_device *vdev)
Regards, Jacek
On 3/28/25 01:23, Jacek Lawrynowicz wrote:
On 3/27/2025 6:38 PM, Lizhi Hou wrote:
On 3/26/25 01:06, Jacek Lawrynowicz wrote:
Hi,
On 3/25/2025 9:50 PM, Lizhi Hou wrote:
On 3/25/25 04:43, Maciej Falkowski wrote:
From: Jacek Lawrynowicz jacek.lawrynowicz@linux.intel.com
Fix deadlock in ivpu_ms_cleanup() by preventing runtime resume after file_priv->ms_lock is acquired.
During a failure in runtime resume, a cold boot is executed, which calls ivpu_ms_cleanup_all(). This function calls ivpu_ms_cleanup() that acquires file_priv->ms_lock and causes the deadlock.
Fixes: cdfad4db7756 ("accel/ivpu: Add NPU profiling support") Cc: stable@vger.kernel.org # v6.11+ Signed-off-by: Jacek Lawrynowicz jacek.lawrynowicz@linux.intel.com Signed-off-by: Maciej Falkowski maciej.falkowski@linux.intel.com
drivers/accel/ivpu/ivpu_ms.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/drivers/accel/ivpu/ivpu_ms.c b/drivers/accel/ivpu/ivpu_ms.c index ffe7b10f8a76..eb485cf15ad6 100644 --- a/drivers/accel/ivpu/ivpu_ms.c +++ b/drivers/accel/ivpu/ivpu_ms.c @@ -4,6 +4,7 @@ */ #include <drm/drm_file.h> +#include <linux/pm_runtime.h> #include "ivpu_drv.h" #include "ivpu_gem.h" @@ -281,6 +282,9 @@ int ivpu_ms_get_info_ioctl(struct drm_device *dev, void *data, struct drm_file * void ivpu_ms_cleanup(struct ivpu_file_priv *file_priv) { struct ivpu_ms_instance *ms, *tmp; + struct ivpu_device *vdev = file_priv->vdev;
+ pm_runtime_get_sync(vdev->drm.dev);
Could get_sync() be failed here? Maybe it is better to add warning for failure?
Yes, this could fail but we already have detailed warnings in runtime resume callback (ivpu_pm_runtime_resume_cb()).
Will the deadlock still happens if this function fails?
No. The deadlock was caused by runtime resume in free_instance(). pm_runtime_get_sync() will always bump PM usage counter, so there will be no resume regardless if it fails or not.
Reviewed-by: Lizhi Hou lizhi.hou@amd.com
mutex_lock(&file_priv->ms_lock); @@ -293,6 +297,8 @@ void ivpu_ms_cleanup(struct ivpu_file_priv *file_priv) free_instance(file_priv, ms); mutex_unlock(&file_priv->ms_lock);
+ pm_runtime_put_autosuspend(vdev->drm.dev); } void ivpu_ms_cleanup_all(struct ivpu_device *vdev)
Regards, Jacek
linux-stable-mirror@lists.linaro.org
-
Jacek Lawrynowicz -
Lizhi Hou -
Maciej Falkowski