| Message ID | 20201127094441.121094-1-miaoqinglang@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/panfrost: fix reference leak in panfrost_job_hw_submit | expand |
在 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; >> > > .
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); if (ret < 0) return;
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(-)