Re: [PATCH 1/2] accel/ivpu: Fix deadlock in ivpu_ms_cleanup()

[Lizhi Hou <lizhi.hou@amd.com>]

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
>