Messages in this thread |  | | Subject | Re: [PATCH] drm/panfrost: fix reference leak in panfrost_job_hw_submit | From | Qinglang Miao <> | Date | Mon, 30 Nov 2020 20:26:21 +0800 |
| |
在 2020/11/27 18:06, Steven Price 写道: > On 27/11/2020 09:44, Qinglang Miao wrote: >> pm_runtime_get_sync will increment pm usage counter even it >> failed. Forgetting to putting operation will result in a >> reference leak here. >> >> A new function pm_runtime_resume_and_get is introduced in >> [0] to keep usage counter balanced. So We fix the reference >> leak by replacing it with new funtion. >> >> [0] dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal >> with usage counter") >> >> Fixes: f3ba91228e8e ("drm/panfrost: Add initial panfrost driver") >> Reported-by: Hulk Robot <hulkci@huawei.com> >> Signed-off-by: Qinglang Miao <miaoqinglang@huawei.com> >> --- >> drivers/gpu/drm/panfrost/panfrost_job.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c >> b/drivers/gpu/drm/panfrost/panfrost_job.c >> index 30e7b7196..04cf3bb67 100644 >> --- a/drivers/gpu/drm/panfrost/panfrost_job.c >> +++ b/drivers/gpu/drm/panfrost/panfrost_job.c >> @@ -147,7 +147,7 @@ static void panfrost_job_hw_submit(struct >> panfrost_job *job, int js) >> panfrost_devfreq_record_busy(&pfdev->pfdevfreq); >> - ret = pm_runtime_get_sync(pfdev->dev); >> + ret = pm_runtime_resume_and_get(pfdev->dev); > > Sorry, but in this case this change isn't correct. > panfrost_job_hw_submit() is expected to be unbalanced (the PM reference > count is expected to be incremented on return). > > In the case where pm_runtime_get_sync() fails, the job will eventually > timeout, and there's a corresponding pm_runtime_put_noidle() in > panfrost_reset(). > > Potentially this could be handled better (e.g. without waiting for the > timeout to occur), but equally this isn't something we expect to happen > in normal operation). > > Steve Sorry, I didn't notice the pm_runtime_put_noidle() in panfrost_job_timedout() before.
Thanks for your reply. > >> if (ret < 0) >> return; >> > > .
|  |