All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/4] drm/amdgpu/debugfs: fix ref count leak when pm_runtime_get_sync fails
@ 2020-06-17 19:02 Alex Deucher
  2020-06-17 19:02 ` [PATCH 2/4] drm/amdgpu/pm: " Alex Deucher
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Alex Deucher @ 2020-06-17 19:02 UTC (permalink / raw)
  To: amd-gfx; +Cc: Alex Deucher

The call to pm_runtime_get_sync increments the counter even in case of
failure, leading to incorrect ref count.
In case of failure, decrement the ref count before returning.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 95 +++++++++++++++------
 1 file changed, 70 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
index 0ac9aab69497..aeada7c9fbea 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
@@ -223,12 +223,16 @@ static int  amdgpu_debugfs_process_reg_op(bool read, struct file *f,
 	*pos &= (1UL << 22) - 1;
 
 	r = pm_runtime_get_sync(adev->ddev->dev);
-	if (r < 0)
+	if (r < 0) {
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 		return r;
+	}
 
 	r = amdgpu_virt_enable_access_debugfs(adev);
-	if (r < 0)
+	if (r < 0) {
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 		return r;
+	}
 
 	if (use_bank) {
 		if ((sh_bank != 0xFFFFFFFF && sh_bank >= adev->gfx.config.max_sh_per_se) ||
@@ -332,12 +336,16 @@ static ssize_t amdgpu_debugfs_regs_pcie_read(struct file *f, char __user *buf,
 		return -EINVAL;
 
 	r = pm_runtime_get_sync(adev->ddev->dev);
-	if (r < 0)
+	if (r < 0) {
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 		return r;
+	}
 
 	r = amdgpu_virt_enable_access_debugfs(adev);
-	if (r < 0)
+	if (r < 0) {
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 		return r;
+	}
 
 	while (size) {
 		uint32_t value;
@@ -387,12 +395,16 @@ static ssize_t amdgpu_debugfs_regs_pcie_write(struct file *f, const char __user
 		return -EINVAL;
 
 	r = pm_runtime_get_sync(adev->ddev->dev);
-	if (r < 0)
+	if (r < 0) {
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 		return r;
+	}
 
 	r = amdgpu_virt_enable_access_debugfs(adev);
-	if (r < 0)
+	if (r < 0) {
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 		return r;
+	}
 
 	while (size) {
 		uint32_t value;
@@ -443,12 +455,16 @@ static ssize_t amdgpu_debugfs_regs_didt_read(struct file *f, char __user *buf,
 		return -EINVAL;
 
 	r = pm_runtime_get_sync(adev->ddev->dev);
-	if (r < 0)
+	if (r < 0) {
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 		return r;
+	}
 
 	r = amdgpu_virt_enable_access_debugfs(adev);
-	if (r < 0)
+	if (r < 0) {
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 		return r;
+	}
 
 	while (size) {
 		uint32_t value;
@@ -498,12 +514,16 @@ static ssize_t amdgpu_debugfs_regs_didt_write(struct file *f, const char __user
 		return -EINVAL;
 
 	r = pm_runtime_get_sync(adev->ddev->dev);
-	if (r < 0)
+	if (r < 0) {
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 		return r;
+	}
 
 	r = amdgpu_virt_enable_access_debugfs(adev);
-	if (r < 0)
+	if (r < 0) {
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 		return r;
+	}
 
 	while (size) {
 		uint32_t value;
@@ -554,12 +574,16 @@ static ssize_t amdgpu_debugfs_regs_smc_read(struct file *f, char __user *buf,
 		return -EINVAL;
 
 	r = pm_runtime_get_sync(adev->ddev->dev);
-	if (r < 0)
+	if (r < 0) {
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 		return r;
+	}
 
 	r = amdgpu_virt_enable_access_debugfs(adev);
-	if (r < 0)
+	if (r < 0) {
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 		return r;
+	}
 
 	while (size) {
 		uint32_t value;
@@ -609,12 +633,16 @@ static ssize_t amdgpu_debugfs_regs_smc_write(struct file *f, const char __user *
 		return -EINVAL;
 
 	r = pm_runtime_get_sync(adev->ddev->dev);
-	if (r < 0)
+	if (r < 0) {
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 		return r;
+	}
 
 	r = amdgpu_virt_enable_access_debugfs(adev);
-	if (r < 0)
+	if (r < 0) {
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 		return r;
+	}
 
 	while (size) {
 		uint32_t value;
@@ -764,12 +792,16 @@ static ssize_t amdgpu_debugfs_sensor_read(struct file *f, char __user *buf,
 	valuesize = sizeof(values);
 
 	r = pm_runtime_get_sync(adev->ddev->dev);
-	if (r < 0)
+	if (r < 0) {
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 		return r;
+	}
 
 	r = amdgpu_virt_enable_access_debugfs(adev);
-	if (r < 0)
+	if (r < 0) {
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 		return r;
+	}
 
 	r = amdgpu_dpm_read_sensor(adev, idx, &values[0], &valuesize);
 
@@ -842,12 +874,16 @@ static ssize_t amdgpu_debugfs_wave_read(struct file *f, char __user *buf,
 	simd = (*pos & GENMASK_ULL(44, 37)) >> 37;
 
 	r = pm_runtime_get_sync(adev->ddev->dev);
-	if (r < 0)
+	if (r < 0) {
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 		return r;
+	}
 
 	r = amdgpu_virt_enable_access_debugfs(adev);
-	if (r < 0)
+	if (r < 0) {
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 		return r;
+	}
 
 	/* switch to the specific se/sh/cu */
 	mutex_lock(&adev->grbm_idx_mutex);
@@ -981,6 +1017,7 @@ static ssize_t amdgpu_debugfs_gpr_read(struct file *f, char __user *buf,
 	return result;
 
 err:
+	pm_runtime_put_autosuspend(adev->ddev->dev);
 	kfree(data);
 	return r;
 }
@@ -1006,8 +1043,10 @@ static ssize_t amdgpu_debugfs_gfxoff_write(struct file *f, const char __user *bu
 		return -EINVAL;
 
 	r = pm_runtime_get_sync(adev->ddev->dev);
-	if (r < 0)
+	if (r < 0) {
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 		return r;
+	}
 
 	while (size) {
 		uint32_t value;
@@ -1143,8 +1182,10 @@ static int amdgpu_debugfs_test_ib(struct seq_file *m, void *data)
 	int r = 0, i;
 
 	r = pm_runtime_get_sync(dev->dev);
-	if (r < 0)
+	if (r < 0) {
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 		return r;
+	}
 
 	/* Avoid accidently unparking the sched thread during GPU reset */
 	mutex_lock(&adev->lock_reset);
@@ -1200,8 +1241,10 @@ static int amdgpu_debugfs_evict_vram(struct seq_file *m, void *data)
 	int r;
 
 	r = pm_runtime_get_sync(dev->dev);
-	if (r < 0)
+	if (r < 0) {
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 		return r;
+	}
 
 	seq_printf(m, "(%d)\n", amdgpu_bo_evict_vram(adev));
 
@@ -1219,8 +1262,10 @@ static int amdgpu_debugfs_evict_gtt(struct seq_file *m, void *data)
 	int r;
 
 	r = pm_runtime_get_sync(dev->dev);
-	if (r < 0)
+	if (r < 0) {
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 		return r;
+	}
 
 	seq_printf(m, "(%d)\n", ttm_bo_evict_mm(&adev->mman.bdev, TTM_PL_TT));
 
@@ -1410,16 +1455,16 @@ static int amdgpu_debugfs_sclk_set(void *data, u64 val)
 		return -EINVAL;
 
 	ret = pm_runtime_get_sync(adev->ddev->dev);
-	if (ret < 0)
+	if (ret < 0) {
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 		return ret;
+	}
 
 	if (is_support_sw_smu(adev)) {
 		ret = smu_get_dpm_freq_range(&adev->smu, SMU_SCLK, &min_freq, &max_freq, true);
 		if (ret || val > max_freq || val < min_freq)
 			return -EINVAL;
 		ret = smu_set_soft_freq_range(&adev->smu, SMU_SCLK, (uint32_t)val, (uint32_t)val, true);
-	} else {
-		return 0;
 	}
 
 	pm_runtime_mark_last_busy(adev->ddev->dev);
-- 
2.25.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 9+ messages in thread
* [PATCH 2/4] drm/amdgpu/pm: fix ref count leak when pm_runtime_get_sync fails
  2020-06-17 19:02 [PATCH 1/4] drm/amdgpu/debugfs: fix ref count leak when pm_runtime_get_sync fails Alex Deucher
@ 2020-06-17 19:02 ` Alex Deucher
  2020-06-18  4:24   ` Quan, Evan
  2020-06-17 19:02 ` [PATCH 3/4] drm/amdgpu/fence: " Alex Deucher
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Alex Deucher @ 2020-06-17 19:02 UTC (permalink / raw)
  To: amd-gfx; +Cc: Alex Deucher

The call to pm_runtime_get_sync increments the counter even in case of
failure, leading to incorrect ref count.
In case of failure, decrement the ref count before returning.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 220 ++++++++++++++++++-------
 1 file changed, 165 insertions(+), 55 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
index 42bbdf49458e..14cc6c546f65 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
@@ -167,8 +167,10 @@ static ssize_t amdgpu_get_power_dpm_state(struct device *dev,
 		return -EPERM;
 
 	ret = pm_runtime_get_sync(ddev->dev);
-	if (ret < 0)
+	if (ret < 0) {
+		pm_runtime_put_autosuspend(ddev->dev);
 		return ret;
+	}
 
 	if (is_support_sw_smu(adev)) {
 		if (adev->smu.ppt_funcs->get_current_power_state)
@@ -212,8 +214,10 @@ static ssize_t amdgpu_set_power_dpm_state(struct device *dev,
 		return -EINVAL;
 
 	ret = pm_runtime_get_sync(ddev->dev);
-	if (ret < 0)
+	if (ret < 0) {
+		pm_runtime_put_autosuspend(ddev->dev);
 		return ret;
+	}
 
 	if (is_support_sw_smu(adev)) {
 		mutex_lock(&adev->pm.mutex);
@@ -307,8 +311,10 @@ static ssize_t amdgpu_get_power_dpm_force_performance_level(struct device *dev,
 		return -EPERM;
 
 	ret = pm_runtime_get_sync(ddev->dev);
-	if (ret < 0)
+	if (ret < 0) {
+		pm_runtime_put_autosuspend(ddev->dev);
 		return ret;
+	}
 
 	if (is_support_sw_smu(adev))
 		level = smu_get_performance_level(&adev->smu);
@@ -369,8 +375,10 @@ static ssize_t amdgpu_set_power_dpm_force_performance_level(struct device *dev,
 	}
 
 	ret = pm_runtime_get_sync(ddev->dev);
-	if (ret < 0)
+	if (ret < 0) {
+		pm_runtime_put_autosuspend(ddev->dev);
 		return ret;
+	}
 
 	if (is_support_sw_smu(adev))
 		current_level = smu_get_performance_level(&adev->smu);
@@ -449,8 +457,10 @@ static ssize_t amdgpu_get_pp_num_states(struct device *dev,
 		return -EPERM;
 
 	ret = pm_runtime_get_sync(ddev->dev);
-	if (ret < 0)
+	if (ret < 0) {
+		pm_runtime_put_autosuspend(ddev->dev);
 		return ret;
+	}
 
 	if (is_support_sw_smu(adev)) {
 		ret = smu_get_power_num_states(&adev->smu, &data);
@@ -491,8 +501,10 @@ static ssize_t amdgpu_get_pp_cur_state(struct device *dev,
 		return -EPERM;
 
 	ret = pm_runtime_get_sync(ddev->dev);
-	if (ret < 0)
+	if (ret < 0) {
+		pm_runtime_put_autosuspend(ddev->dev);
 		return ret;
+	}
 
 	if (is_support_sw_smu(adev)) {
 		pm = smu_get_current_power_state(smu);
@@ -567,8 +579,10 @@ static ssize_t amdgpu_set_pp_force_state(struct device *dev,
 		state = data.states[idx];
 
 		ret = pm_runtime_get_sync(ddev->dev);
-		if (ret < 0)
+		if (ret < 0) {
+			pm_runtime_put_autosuspend(ddev->dev);
 			return ret;
+		}
 
 		/* only set user selected power states */
 		if (state != POWER_STATE_TYPE_INTERNAL_BOOT &&
@@ -608,8 +622,10 @@ static ssize_t amdgpu_get_pp_table(struct device *dev,
 		return -EPERM;
 
 	ret = pm_runtime_get_sync(ddev->dev);
-	if (ret < 0)
+	if (ret < 0) {
+		pm_runtime_put_autosuspend(ddev->dev);
 		return ret;
+	}
 
 	if (is_support_sw_smu(adev)) {
 		size = smu_sys_get_pp_table(&adev->smu, (void **)&table);
@@ -650,8 +666,10 @@ static ssize_t amdgpu_set_pp_table(struct device *dev,
 		return -EPERM;
 
 	ret = pm_runtime_get_sync(ddev->dev);
-	if (ret < 0)
+	if (ret < 0) {
+		pm_runtime_put_autosuspend(ddev->dev);
 		return ret;
+	}
 
 	if (is_support_sw_smu(adev)) {
 		ret = smu_sys_set_pp_table(&adev->smu, (void *)buf, count);
@@ -790,8 +808,10 @@ static ssize_t amdgpu_set_pp_od_clk_voltage(struct device *dev,
 	}
 
 	ret = pm_runtime_get_sync(ddev->dev);
-	if (ret < 0)
+	if (ret < 0) {
+		pm_runtime_put_autosuspend(ddev->dev);
 		return ret;
+	}
 
 	if (is_support_sw_smu(adev)) {
 		ret = smu_od_edit_dpm_table(&adev->smu, type,
@@ -847,8 +867,10 @@ static ssize_t amdgpu_get_pp_od_clk_voltage(struct device *dev,
 		return -EPERM;
 
 	ret = pm_runtime_get_sync(ddev->dev);
-	if (ret < 0)
+	if (ret < 0) {
+		pm_runtime_put_autosuspend(ddev->dev);
 		return ret;
+	}
 
 	if (is_support_sw_smu(adev)) {
 		size = smu_print_clk_levels(&adev->smu, SMU_OD_SCLK, buf);
@@ -905,8 +927,10 @@ static ssize_t amdgpu_set_pp_features(struct device *dev,
 	pr_debug("featuremask = 0x%llx\n", featuremask);
 
 	ret = pm_runtime_get_sync(ddev->dev);
-	if (ret < 0)
+	if (ret < 0) {
+		pm_runtime_put_autosuspend(ddev->dev);
 		return ret;
+	}
 
 	if (is_support_sw_smu(adev)) {
 		ret = smu_sys_set_pp_feature_mask(&adev->smu, featuremask);
@@ -942,8 +966,10 @@ static ssize_t amdgpu_get_pp_features(struct device *dev,
 		return -EPERM;
 
 	ret = pm_runtime_get_sync(ddev->dev);
-	if (ret < 0)
+	if (ret < 0) {
+		pm_runtime_put_autosuspend(ddev->dev);
 		return ret;
+	}
 
 	if (is_support_sw_smu(adev))
 		size = smu_sys_get_pp_feature_mask(&adev->smu, buf);
@@ -1001,8 +1027,10 @@ static ssize_t amdgpu_get_pp_dpm_sclk(struct device *dev,
 		return -EPERM;
 
 	ret = pm_runtime_get_sync(ddev->dev);
-	if (ret < 0)
+	if (ret < 0) {
+		pm_runtime_put_autosuspend(ddev->dev);
 		return ret;
+	}
 
 	if (is_support_sw_smu(adev))
 		size = smu_print_clk_levels(&adev->smu, SMU_SCLK, buf);
@@ -1071,8 +1099,10 @@ static ssize_t amdgpu_set_pp_dpm_sclk(struct device *dev,
 		return ret;
 
 	ret = pm_runtime_get_sync(ddev->dev);
-	if (ret < 0)
+	if (ret < 0) {
+		pm_runtime_put_autosuspend(ddev->dev);
 		return ret;
+	}
 
 	if (is_support_sw_smu(adev))
 		ret = smu_force_clk_levels(&adev->smu, SMU_SCLK, mask, true);
@@ -1101,8 +1131,10 @@ static ssize_t amdgpu_get_pp_dpm_mclk(struct device *dev,
 		return -EPERM;
 
 	ret = pm_runtime_get_sync(ddev->dev);
-	if (ret < 0)
+	if (ret < 0) {
+		pm_runtime_put_autosuspend(ddev->dev);
 		return ret;
+	}
 
 	if (is_support_sw_smu(adev))
 		size = smu_print_clk_levels(&adev->smu, SMU_MCLK, buf);
@@ -1135,8 +1167,10 @@ static ssize_t amdgpu_set_pp_dpm_mclk(struct device *dev,
 		return ret;
 
 	ret = pm_runtime_get_sync(ddev->dev);
-	if (ret < 0)
+	if (ret < 0) {
+		pm_runtime_put_autosuspend(ddev->dev);
 		return ret;
+	}
 
 	if (is_support_sw_smu(adev))
 		ret = smu_force_clk_levels(&adev->smu, SMU_MCLK, mask, true);
@@ -1165,8 +1199,10 @@ static ssize_t amdgpu_get_pp_dpm_socclk(struct device *dev,
 		return -EPERM;
 
 	ret = pm_runtime_get_sync(ddev->dev);
-	if (ret < 0)
+	if (ret < 0) {
+		pm_runtime_put_autosuspend(ddev->dev);
 		return ret;
+	}
 
 	if (is_support_sw_smu(adev))
 		size = smu_print_clk_levels(&adev->smu, SMU_SOCCLK, buf);
@@ -1199,8 +1235,10 @@ static ssize_t amdgpu_set_pp_dpm_socclk(struct device *dev,
 		return ret;
 
 	ret = pm_runtime_get_sync(ddev->dev);
-	if (ret < 0)
+	if (ret < 0) {
+		pm_runtime_put_autosuspend(ddev->dev);
 		return ret;
+	}
 
 	if (is_support_sw_smu(adev))
 		ret = smu_force_clk_levels(&adev->smu, SMU_SOCCLK, mask, true);
@@ -1231,8 +1269,10 @@ static ssize_t amdgpu_get_pp_dpm_fclk(struct device *dev,
 		return -EPERM;
 
 	ret = pm_runtime_get_sync(ddev->dev);
-	if (ret < 0)
+	if (ret < 0) {
+		pm_runtime_put_autosuspend(ddev->dev);
 		return ret;
+	}
 
 	if (is_support_sw_smu(adev))
 		size = smu_print_clk_levels(&adev->smu, SMU_FCLK, buf);
@@ -1265,8 +1305,10 @@ static ssize_t amdgpu_set_pp_dpm_fclk(struct device *dev,
 		return ret;
 
 	ret = pm_runtime_get_sync(ddev->dev);
-	if (ret < 0)
+	if (ret < 0) {
+		pm_runtime_put_autosuspend(ddev->dev);
 		return ret;
+	}
 
 	if (is_support_sw_smu(adev))
 		ret = smu_force_clk_levels(&adev->smu, SMU_FCLK, mask, true);
@@ -1297,8 +1339,10 @@ static ssize_t amdgpu_get_pp_dpm_dcefclk(struct device *dev,
 		return -EPERM;
 
 	ret = pm_runtime_get_sync(ddev->dev);
-	if (ret < 0)
+	if (ret < 0) {
+		pm_runtime_put_autosuspend(ddev->dev);
 		return ret;
+	}
 
 	if (is_support_sw_smu(adev))
 		size = smu_print_clk_levels(&adev->smu, SMU_DCEFCLK, buf);
@@ -1331,8 +1375,10 @@ static ssize_t amdgpu_set_pp_dpm_dcefclk(struct device *dev,
 		return ret;
 
 	ret = pm_runtime_get_sync(ddev->dev);
-	if (ret < 0)
+	if (ret < 0) {
+		pm_runtime_put_autosuspend(ddev->dev);
 		return ret;
+	}
 
 	if (is_support_sw_smu(adev))
 		ret = smu_force_clk_levels(&adev->smu, SMU_DCEFCLK, mask, true);
@@ -1363,8 +1409,10 @@ static ssize_t amdgpu_get_pp_dpm_pcie(struct device *dev,
 		return -EPERM;
 
 	ret = pm_runtime_get_sync(ddev->dev);
-	if (ret < 0)
+	if (ret < 0) {
+		pm_runtime_put_autosuspend(ddev->dev);
 		return ret;
+	}
 
 	if (is_support_sw_smu(adev))
 		size = smu_print_clk_levels(&adev->smu, SMU_PCIE, buf);
@@ -1397,8 +1445,10 @@ static ssize_t amdgpu_set_pp_dpm_pcie(struct device *dev,
 		return ret;
 
 	ret = pm_runtime_get_sync(ddev->dev);
-	if (ret < 0)
+	if (ret < 0) {
+		pm_runtime_put_autosuspend(ddev->dev);
 		return ret;
+	}
 
 	if (is_support_sw_smu(adev))
 		ret = smu_force_clk_levels(&adev->smu, SMU_PCIE, mask, true);
@@ -1429,8 +1479,10 @@ static ssize_t amdgpu_get_pp_sclk_od(struct device *dev,
 		return -EPERM;
 
 	ret = pm_runtime_get_sync(ddev->dev);
-	if (ret < 0)
+	if (ret < 0) {
+		pm_runtime_put_autosuspend(ddev->dev);
 		return ret;
+	}
 
 	if (is_support_sw_smu(adev))
 		value = smu_get_od_percentage(&(adev->smu), SMU_OD_SCLK);
@@ -1462,8 +1514,10 @@ static ssize_t amdgpu_set_pp_sclk_od(struct device *dev,
 		return -EINVAL;
 
 	ret = pm_runtime_get_sync(ddev->dev);
-	if (ret < 0)
+	if (ret < 0) {
+		pm_runtime_put_autosuspend(ddev->dev);
 		return ret;
+	}
 
 	if (is_support_sw_smu(adev)) {
 		value = smu_set_od_percentage(&(adev->smu), SMU_OD_SCLK, (uint32_t)value);
@@ -1498,8 +1552,10 @@ static ssize_t amdgpu_get_pp_mclk_od(struct device *dev,
 		return -EPERM;
 
 	ret = pm_runtime_get_sync(ddev->dev);
-	if (ret < 0)
+	if (ret < 0) {
+		pm_runtime_put_autosuspend(ddev->dev);
 		return ret;
+	}
 
 	if (is_support_sw_smu(adev))
 		value = smu_get_od_percentage(&(adev->smu), SMU_OD_MCLK);
@@ -1531,8 +1587,10 @@ static ssize_t amdgpu_set_pp_mclk_od(struct device *dev,
 		return -EINVAL;
 
 	ret = pm_runtime_get_sync(ddev->dev);
-	if (ret < 0)
+	if (ret < 0) {
+		pm_runtime_put_autosuspend(ddev->dev);
 		return ret;
+	}
 
 	if (is_support_sw_smu(adev)) {
 		value = smu_set_od_percentage(&(adev->smu), SMU_OD_MCLK, (uint32_t)value);
@@ -1587,8 +1645,10 @@ static ssize_t amdgpu_get_pp_power_profile_mode(struct device *dev,
 		return -EPERM;
 
 	ret = pm_runtime_get_sync(ddev->dev);
-	if (ret < 0)
+	if (ret < 0) {
+		pm_runtime_put_autosuspend(ddev->dev);
 		return ret;
+	}
 
 	if (is_support_sw_smu(adev))
 		size = smu_get_power_profile_mode(&adev->smu, buf);
@@ -1650,8 +1710,10 @@ static ssize_t amdgpu_set_pp_power_profile_mode(struct device *dev,
 	parameter[parameter_size] = profile_mode;
 
 	ret = pm_runtime_get_sync(ddev->dev);
-	if (ret < 0)
+	if (ret < 0) {
+		pm_runtime_put_autosuspend(ddev->dev);
 		return ret;
+	}
 
 	if (is_support_sw_smu(adev))
 		ret = smu_set_power_profile_mode(&adev->smu, parameter, parameter_size, true);
@@ -1687,8 +1749,10 @@ static ssize_t amdgpu_get_gpu_busy_percent(struct device *dev,
 		return -EPERM;
 
 	r = pm_runtime_get_sync(ddev->dev);
-	if (r < 0)
+	if (r < 0) {
+		pm_runtime_put_autosuspend(ddev->dev);
 		return r;
+	}
 
 	/* read the IP busy sensor */
 	r = amdgpu_dpm_read_sensor(adev, AMDGPU_PP_SENSOR_GPU_LOAD,
@@ -1723,8 +1787,10 @@ static ssize_t amdgpu_get_mem_busy_percent(struct device *dev,
 		return -EPERM;
 
 	r = pm_runtime_get_sync(ddev->dev);
-	if (r < 0)
+	if (r < 0) {
+		pm_runtime_put_autosuspend(ddev->dev);
 		return r;
+	}
 
 	/* read the IP busy sensor */
 	r = amdgpu_dpm_read_sensor(adev, AMDGPU_PP_SENSOR_MEM_LOAD,
@@ -1770,8 +1836,10 @@ static ssize_t amdgpu_get_pcie_bw(struct device *dev,
 		return -ENODATA;
 
 	ret = pm_runtime_get_sync(ddev->dev);
-	if (ret < 0)
+	if (ret < 0) {
+		pm_runtime_put_autosuspend(ddev->dev);
 		return ret;
+	}
 
 	amdgpu_asic_get_pcie_usage(adev, &count0, &count1);
 
@@ -2073,8 +2141,10 @@ static ssize_t amdgpu_hwmon_show_temp(struct device *dev,
 		return -EINVAL;
 
 	r = pm_runtime_get_sync(adev->ddev->dev);
-	if (r < 0)
+	if (r < 0) {
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 		return r;
+	}
 
 	switch (channel) {
 	case PP_TEMP_JUNCTION:
@@ -2204,8 +2274,10 @@ static ssize_t amdgpu_hwmon_get_pwm1_enable(struct device *dev,
 		return -EPERM;
 
 	ret = pm_runtime_get_sync(adev->ddev->dev);
-	if (ret < 0)
+	if (ret < 0) {
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 		return ret;
+	}
 
 	if (is_support_sw_smu(adev)) {
 		pwm_mode = smu_get_fan_control_mode(&adev->smu);
@@ -2242,8 +2314,10 @@ static ssize_t amdgpu_hwmon_set_pwm1_enable(struct device *dev,
 		return err;
 
 	ret = pm_runtime_get_sync(adev->ddev->dev);
-	if (ret < 0)
+	if (ret < 0) {
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 		return ret;
+	}
 
 	if (is_support_sw_smu(adev)) {
 		smu_set_fan_control_mode(&adev->smu, value);
@@ -2290,8 +2364,10 @@ static ssize_t amdgpu_hwmon_set_pwm1(struct device *dev,
 		return -EPERM;
 
 	err = pm_runtime_get_sync(adev->ddev->dev);
-	if (err < 0)
+	if (err < 0) {
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 		return err;
+	}
 
 	if (is_support_sw_smu(adev))
 		pwm_mode = smu_get_fan_control_mode(&adev->smu);
@@ -2342,8 +2418,10 @@ static ssize_t amdgpu_hwmon_get_pwm1(struct device *dev,
 		return -EPERM;
 
 	err = pm_runtime_get_sync(adev->ddev->dev);
-	if (err < 0)
+	if (err < 0) {
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 		return err;
+	}
 
 	if (is_support_sw_smu(adev))
 		err = smu_get_fan_speed_percent(&adev->smu, &speed);
@@ -2375,8 +2453,10 @@ static ssize_t amdgpu_hwmon_get_fan1_input(struct device *dev,
 		return -EPERM;
 
 	err = pm_runtime_get_sync(adev->ddev->dev);
-	if (err < 0)
+	if (err < 0) {
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 		return err;
+	}
 
 	if (is_support_sw_smu(adev))
 		err = smu_get_fan_speed_rpm(&adev->smu, &speed);
@@ -2407,8 +2487,10 @@ static ssize_t amdgpu_hwmon_get_fan1_min(struct device *dev,
 		return -EPERM;
 
 	r = pm_runtime_get_sync(adev->ddev->dev);
-	if (r < 0)
+	if (r < 0) {
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 		return r;
+	}
 
 	r = amdgpu_dpm_read_sensor(adev, AMDGPU_PP_SENSOR_MIN_FAN_RPM,
 				   (void *)&min_rpm, &size);
@@ -2435,8 +2517,10 @@ static ssize_t amdgpu_hwmon_get_fan1_max(struct device *dev,
 		return -EPERM;
 
 	r = pm_runtime_get_sync(adev->ddev->dev);
-	if (r < 0)
+	if (r < 0) {
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 		return r;
+	}
 
 	r = amdgpu_dpm_read_sensor(adev, AMDGPU_PP_SENSOR_MAX_FAN_RPM,
 				   (void *)&max_rpm, &size);
@@ -2462,8 +2546,10 @@ static ssize_t amdgpu_hwmon_get_fan1_target(struct device *dev,
 		return -EPERM;
 
 	err = pm_runtime_get_sync(adev->ddev->dev);
-	if (err < 0)
+	if (err < 0) {
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 		return err;
+	}
 
 	if (is_support_sw_smu(adev))
 		err = smu_get_fan_speed_rpm(&adev->smu, &rpm);
@@ -2494,8 +2580,10 @@ static ssize_t amdgpu_hwmon_set_fan1_target(struct device *dev,
 		return -EPERM;
 
 	err = pm_runtime_get_sync(adev->ddev->dev);
-	if (err < 0)
+	if (err < 0) {
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 		return err;
+	}
 
 	if (is_support_sw_smu(adev))
 		pwm_mode = smu_get_fan_control_mode(&adev->smu);
@@ -2543,8 +2631,10 @@ static ssize_t amdgpu_hwmon_get_fan1_enable(struct device *dev,
 		return -EPERM;
 
 	ret = pm_runtime_get_sync(adev->ddev->dev);
-	if (ret < 0)
+	if (ret < 0) {
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 		return ret;
+	}
 
 	if (is_support_sw_smu(adev)) {
 		pwm_mode = smu_get_fan_control_mode(&adev->smu);
@@ -2589,8 +2679,10 @@ static ssize_t amdgpu_hwmon_set_fan1_enable(struct device *dev,
 		return -EINVAL;
 
 	err = pm_runtime_get_sync(adev->ddev->dev);
-	if (err < 0)
+	if (err < 0) {
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 		return err;
+	}
 
 	if (is_support_sw_smu(adev)) {
 		smu_set_fan_control_mode(&adev->smu, pwm_mode);
@@ -2621,8 +2713,10 @@ static ssize_t amdgpu_hwmon_show_vddgfx(struct device *dev,
 		return -EPERM;
 
 	r = pm_runtime_get_sync(adev->ddev->dev);
-	if (r < 0)
+	if (r < 0) {
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 		return r;
+	}
 
 	/* get the voltage */
 	r = amdgpu_dpm_read_sensor(adev, AMDGPU_PP_SENSOR_VDDGFX,
@@ -2660,8 +2754,10 @@ static ssize_t amdgpu_hwmon_show_vddnb(struct device *dev,
 		return -EINVAL;
 
 	r = pm_runtime_get_sync(adev->ddev->dev);
-	if (r < 0)
+	if (r < 0) {
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 		return r;
+	}
 
 	/* get the voltage */
 	r = amdgpu_dpm_read_sensor(adev, AMDGPU_PP_SENSOR_VDDNB,
@@ -2696,8 +2792,10 @@ static ssize_t amdgpu_hwmon_show_power_avg(struct device *dev,
 		return -EPERM;
 
 	r = pm_runtime_get_sync(adev->ddev->dev);
-	if (r < 0)
+	if (r < 0) {
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 		return r;
+	}
 
 	/* get the voltage */
 	r = amdgpu_dpm_read_sensor(adev, AMDGPU_PP_SENSOR_GPU_POWER,
@@ -2735,8 +2833,10 @@ static ssize_t amdgpu_hwmon_show_power_cap_max(struct device *dev,
 		return -EPERM;
 
 	r = pm_runtime_get_sync(adev->ddev->dev);
-	if (r < 0)
+	if (r < 0) {
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 		return r;
+	}
 
 	if (is_support_sw_smu(adev)) {
 		smu_get_power_limit(&adev->smu, &limit, true);
@@ -2767,8 +2867,10 @@ static ssize_t amdgpu_hwmon_show_power_cap(struct device *dev,
 		return -EPERM;
 
 	r = pm_runtime_get_sync(adev->ddev->dev);
-	if (r < 0)
+	if (r < 0) {
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 		return r;
+	}
 
 	if (is_support_sw_smu(adev)) {
 		smu_get_power_limit(&adev->smu, &limit, false);
@@ -2810,8 +2912,10 @@ static ssize_t amdgpu_hwmon_set_power_cap(struct device *dev,
 
 
 	err = pm_runtime_get_sync(adev->ddev->dev);
-	if (err < 0)
+	if (err < 0) {
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 		return err;
+	}
 
 	if (is_support_sw_smu(adev))
 		err = smu_set_power_limit(&adev->smu, value);
@@ -2841,8 +2945,10 @@ static ssize_t amdgpu_hwmon_show_sclk(struct device *dev,
 		return -EPERM;
 
 	r = pm_runtime_get_sync(adev->ddev->dev);
-	if (r < 0)
+	if (r < 0) {
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 		return r;
+	}
 
 	/* get the sclk */
 	r = amdgpu_dpm_read_sensor(adev, AMDGPU_PP_SENSOR_GFX_SCLK,
@@ -2876,8 +2982,10 @@ static ssize_t amdgpu_hwmon_show_mclk(struct device *dev,
 		return -EPERM;
 
 	r = pm_runtime_get_sync(adev->ddev->dev);
-	if (r < 0)
+	if (r < 0) {
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 		return r;
+	}
 
 	/* get the sclk */
 	r = amdgpu_dpm_read_sensor(adev, AMDGPU_PP_SENSOR_GFX_MCLK,
@@ -3739,8 +3847,10 @@ static int amdgpu_debugfs_pm_info(struct seq_file *m, void *data)
 		return -EPERM;
 
 	r = pm_runtime_get_sync(dev->dev);
-	if (r < 0)
+	if (r < 0) {
+		pm_runtime_put_autosuspend(dev->dev);
 		return r;
+	}
 
 	amdgpu_device_ip_get_clockgating_state(adev, &flags);
 	seq_printf(m, "Clock Gating Flags Mask: 0x%x\n", flags);
-- 
2.25.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 9+ messages in thread
* [PATCH 3/4] drm/amdgpu/fence: fix ref count leak when pm_runtime_get_sync fails
  2020-06-17 19:02 [PATCH 1/4] drm/amdgpu/debugfs: fix ref count leak when pm_runtime_get_sync fails Alex Deucher
  2020-06-17 19:02 ` [PATCH 2/4] drm/amdgpu/pm: " Alex Deucher
@ 2020-06-17 19:02 ` Alex Deucher
  2020-06-17 20:27   ` Bhardwaj, Rajneesh
  2020-06-17 19:02 ` [PATCH 4/4] drm/amdkfd: " Alex Deucher
  2020-06-18  4:25 ` [PATCH 1/4] drm/amdgpu/debugfs: " Quan, Evan
  3 siblings, 1 reply; 9+ messages in thread
From: Alex Deucher @ 2020-06-17 19:02 UTC (permalink / raw)
  To: amd-gfx; +Cc: Alex Deucher

The call to pm_runtime_get_sync increments the counter even in case of
failure, leading to incorrect ref count.
In case of failure, decrement the ref count before returning.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
index 8537f4704348..60b323d7caf2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
@@ -755,8 +755,10 @@ static int amdgpu_debugfs_gpu_recover(struct seq_file *m, void *data)
 	int r;
 
 	r = pm_runtime_get_sync(dev->dev);
-	if (r < 0)
+	if (r < 0) {
+		pm_runtime_put_autosuspend(dev->dev);
 		return 0;
+	}
 
 	seq_printf(m, "gpu recover\n");
 	amdgpu_device_gpu_recover(adev, NULL);
-- 
2.25.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 9+ messages in thread
* [PATCH 4/4] drm/amdkfd: fix ref count leak when pm_runtime_get_sync fails
  2020-06-17 19:02 [PATCH 1/4] drm/amdgpu/debugfs: fix ref count leak when pm_runtime_get_sync fails Alex Deucher
  2020-06-17 19:02 ` [PATCH 2/4] drm/amdgpu/pm: " Alex Deucher
  2020-06-17 19:02 ` [PATCH 3/4] drm/amdgpu/fence: " Alex Deucher
@ 2020-06-17 19:02 ` Alex Deucher
  2020-06-17 20:16   ` Felix Kuehling
  2020-06-18  4:25 ` [PATCH 1/4] drm/amdgpu/debugfs: " Quan, Evan
  3 siblings, 1 reply; 9+ messages in thread
From: Alex Deucher @ 2020-06-17 19:02 UTC (permalink / raw)
  To: amd-gfx; +Cc: Alex Deucher

The call to pm_runtime_get_sync increments the counter even in case of
failure, leading to incorrect ref count.
In case of failure, decrement the ref count before returning.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_process.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index a9a7f5aa2710..0b4f40905f55 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -1116,8 +1116,10 @@ struct kfd_process_device *kfd_bind_process_to_device(struct kfd_dev *dev,
 	 */
 	if (!pdd->runtime_inuse) {
 		err = pm_runtime_get_sync(dev->ddev->dev);
-		if (err < 0)
+		if (err < 0) {
+			pm_runtime_put_autosuspend(dev->ddev->dev);
 			return ERR_PTR(err);
+		}
 	}
 
 	err = kfd_iommu_bind_process_to_device(pdd);
-- 
2.25.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 9+ messages in thread
* Re: [PATCH 4/4] drm/amdkfd: fix ref count leak when pm_runtime_get_sync fails
  2020-06-17 19:02 ` [PATCH 4/4] drm/amdkfd: " Alex Deucher
@ 2020-06-17 20:16   ` Felix Kuehling
  2020-06-17 20:26     ` Bhardwaj, Rajneesh
  0 siblings, 1 reply; 9+ messages in thread
From: Felix Kuehling @ 2020-06-17 20:16 UTC (permalink / raw)
  To: Alex Deucher, amd-gfx, Bhardwaj, Rajneesh; +Cc: Alex Deucher

[+Rajneesh]

Am 2020-06-17 um 3:02 p.m. schrieb Alex Deucher:
> The call to pm_runtime_get_sync increments the counter even in case of
> failure, leading to incorrect ref count.
> In case of failure, decrement the ref count before returning.
>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>


> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_process.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> index a9a7f5aa2710..0b4f40905f55 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> @@ -1116,8 +1116,10 @@ struct kfd_process_device *kfd_bind_process_to_device(struct kfd_dev *dev,
>  	 */
>  	if (!pdd->runtime_inuse) {
>  		err = pm_runtime_get_sync(dev->ddev->dev);
> -		if (err < 0)
> +		if (err < 0) {
> +			pm_runtime_put_autosuspend(dev->ddev->dev);
>  			return ERR_PTR(err);
> +		}
>  	}
>  
>  	err = kfd_iommu_bind_process_to_device(pdd);
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread
* RE: [PATCH 4/4] drm/amdkfd: fix ref count leak when pm_runtime_get_sync fails
  2020-06-17 20:16   ` Felix Kuehling
@ 2020-06-17 20:26     ` Bhardwaj, Rajneesh
  0 siblings, 0 replies; 9+ messages in thread
From: Bhardwaj, Rajneesh @ 2020-06-17 20:26 UTC (permalink / raw)
  To: Kuehling, Felix, Alex Deucher, amd-gfx; +Cc: Deucher, Alexander

[AMD Official Use Only - Internal Distribution Only]

Acked-by: Rajneesh Bhardwaj <rajneesh.bhardwaj@amd.com>

-----Original Message-----
From: Kuehling, Felix <Felix.Kuehling@amd.com> 
Sent: Wednesday, June 17, 2020 4:17 PM
To: Alex Deucher <alexdeucher@gmail.com>; amd-gfx@lists.freedesktop.org; Bhardwaj, Rajneesh <Rajneesh.Bhardwaj@amd.com>
Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
Subject: Re: [PATCH 4/4] drm/amdkfd: fix ref count leak when pm_runtime_get_sync fails

[+Rajneesh]

Am 2020-06-17 um 3:02 p.m. schrieb Alex Deucher:
> The call to pm_runtime_get_sync increments the counter even in case of 
> failure, leading to incorrect ref count.
> In case of failure, decrement the ref count before returning.
>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>


> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_process.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c 
> b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> index a9a7f5aa2710..0b4f40905f55 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> @@ -1116,8 +1116,10 @@ struct kfd_process_device *kfd_bind_process_to_device(struct kfd_dev *dev,
>  	 */
>  	if (!pdd->runtime_inuse) {
>  		err = pm_runtime_get_sync(dev->ddev->dev);
> -		if (err < 0)
> +		if (err < 0) {
> +			pm_runtime_put_autosuspend(dev->ddev->dev);
>  			return ERR_PTR(err);
> +		}
>  	}
>  
>  	err = kfd_iommu_bind_process_to_device(pdd);
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread
* RE: [PATCH 3/4] drm/amdgpu/fence: fix ref count leak when pm_runtime_get_sync fails
  2020-06-17 19:02 ` [PATCH 3/4] drm/amdgpu/fence: " Alex Deucher
@ 2020-06-17 20:27   ` Bhardwaj, Rajneesh
  0 siblings, 0 replies; 9+ messages in thread
From: Bhardwaj, Rajneesh @ 2020-06-17 20:27 UTC (permalink / raw)
  To: Alex Deucher, amd-gfx; +Cc: Deucher, Alexander

[AMD Official Use Only - Internal Distribution Only]

Reviewed-by: Rajneesh Bhardwaj <rajneesh.bhardwaj@amd.com>

-----Original Message-----
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Alex Deucher
Sent: Wednesday, June 17, 2020 3:02 PM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
Subject: [PATCH 3/4] drm/amdgpu/fence: fix ref count leak when pm_runtime_get_sync fails

[CAUTION: External Email]

The call to pm_runtime_get_sync increments the counter even in case of failure, leading to incorrect ref count.
In case of failure, decrement the ref count before returning.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
index 8537f4704348..60b323d7caf2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
@@ -755,8 +755,10 @@ static int amdgpu_debugfs_gpu_recover(struct seq_file *m, void *data)
        int r;

        r = pm_runtime_get_sync(dev->dev);
-       if (r < 0)
+       if (r < 0) {
+               pm_runtime_put_autosuspend(dev->dev);
                return 0;
+       }

        seq_printf(m, "gpu recover\n");
        amdgpu_device_gpu_recover(adev, NULL);
--
2.25.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7Crajneesh.bhardwaj%40amd.com%7Cdcc3842e55284da6689f08d812f0fb07%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637280173535571918&amp;sdata=NU1AzDtdUzmvXItL05quhP35rT1plj4%2B3vOJE9Rr2lA%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 9+ messages in thread
* RE: [PATCH 2/4] drm/amdgpu/pm: fix ref count leak when pm_runtime_get_sync fails
  2020-06-17 19:02 ` [PATCH 2/4] drm/amdgpu/pm: " Alex Deucher
@ 2020-06-18  4:24   ` Quan, Evan
  0 siblings, 0 replies; 9+ messages in thread
From: Quan, Evan @ 2020-06-18  4:24 UTC (permalink / raw)
  To: Alex Deucher, amd-gfx; +Cc: Deucher, Alexander

[AMD Official Use Only - Internal Distribution Only]

Reviewed-by: Evan Quan <evan.quan@amd.com>

-----Original Message-----
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Alex Deucher
Sent: Thursday, June 18, 2020 3:02 AM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
Subject: [PATCH 2/4] drm/amdgpu/pm: fix ref count leak when pm_runtime_get_sync fails

The call to pm_runtime_get_sync increments the counter even in case of failure, leading to incorrect ref count.
In case of failure, decrement the ref count before returning.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 220 ++++++++++++++++++-------
 1 file changed, 165 insertions(+), 55 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
index 42bbdf49458e..14cc6c546f65 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
@@ -167,8 +167,10 @@ static ssize_t amdgpu_get_power_dpm_state(struct device *dev,
 return -EPERM;

 ret = pm_runtime_get_sync(ddev->dev);
-if (ret < 0)
+if (ret < 0) {
+pm_runtime_put_autosuspend(ddev->dev);
 return ret;
+}

 if (is_support_sw_smu(adev)) {
 if (adev->smu.ppt_funcs->get_current_power_state)
@@ -212,8 +214,10 @@ static ssize_t amdgpu_set_power_dpm_state(struct device *dev,
 return -EINVAL;

 ret = pm_runtime_get_sync(ddev->dev);
-if (ret < 0)
+if (ret < 0) {
+pm_runtime_put_autosuspend(ddev->dev);
 return ret;
+}

 if (is_support_sw_smu(adev)) {
 mutex_lock(&adev->pm.mutex);
@@ -307,8 +311,10 @@ static ssize_t amdgpu_get_power_dpm_force_performance_level(struct device *dev,
 return -EPERM;

 ret = pm_runtime_get_sync(ddev->dev);
-if (ret < 0)
+if (ret < 0) {
+pm_runtime_put_autosuspend(ddev->dev);
 return ret;
+}

 if (is_support_sw_smu(adev))
 level = smu_get_performance_level(&adev->smu);
@@ -369,8 +375,10 @@ static ssize_t amdgpu_set_power_dpm_force_performance_level(struct device *dev,
 }

 ret = pm_runtime_get_sync(ddev->dev);
-if (ret < 0)
+if (ret < 0) {
+pm_runtime_put_autosuspend(ddev->dev);
 return ret;
+}

 if (is_support_sw_smu(adev))
 current_level = smu_get_performance_level(&adev->smu);
@@ -449,8 +457,10 @@ static ssize_t amdgpu_get_pp_num_states(struct device *dev,
 return -EPERM;

 ret = pm_runtime_get_sync(ddev->dev);
-if (ret < 0)
+if (ret < 0) {
+pm_runtime_put_autosuspend(ddev->dev);
 return ret;
+}

 if (is_support_sw_smu(adev)) {
 ret = smu_get_power_num_states(&adev->smu, &data); @@ -491,8 +501,10 @@ static ssize_t amdgpu_get_pp_cur_state(struct device *dev,
 return -EPERM;

 ret = pm_runtime_get_sync(ddev->dev);
-if (ret < 0)
+if (ret < 0) {
+pm_runtime_put_autosuspend(ddev->dev);
 return ret;
+}

 if (is_support_sw_smu(adev)) {
 pm = smu_get_current_power_state(smu); @@ -567,8 +579,10 @@ static ssize_t amdgpu_set_pp_force_state(struct device *dev,
 state = data.states[idx];

 ret = pm_runtime_get_sync(ddev->dev);
-if (ret < 0)
+if (ret < 0) {
+pm_runtime_put_autosuspend(ddev->dev);
 return ret;
+}

 /* only set user selected power states */
 if (state != POWER_STATE_TYPE_INTERNAL_BOOT && @@ -608,8 +622,10 @@ static ssize_t amdgpu_get_pp_table(struct device *dev,
 return -EPERM;

 ret = pm_runtime_get_sync(ddev->dev);
-if (ret < 0)
+if (ret < 0) {
+pm_runtime_put_autosuspend(ddev->dev);
 return ret;
+}

 if (is_support_sw_smu(adev)) {
 size = smu_sys_get_pp_table(&adev->smu, (void **)&table); @@ -650,8 +666,10 @@ static ssize_t amdgpu_set_pp_table(struct device *dev,
 return -EPERM;

 ret = pm_runtime_get_sync(ddev->dev);
-if (ret < 0)
+if (ret < 0) {
+pm_runtime_put_autosuspend(ddev->dev);
 return ret;
+}

 if (is_support_sw_smu(adev)) {
 ret = smu_sys_set_pp_table(&adev->smu, (void *)buf, count); @@ -790,8 +808,10 @@ static ssize_t amdgpu_set_pp_od_clk_voltage(struct device *dev,
 }

 ret = pm_runtime_get_sync(ddev->dev);
-if (ret < 0)
+if (ret < 0) {
+pm_runtime_put_autosuspend(ddev->dev);
 return ret;
+}

 if (is_support_sw_smu(adev)) {
 ret = smu_od_edit_dpm_table(&adev->smu, type, @@ -847,8 +867,10 @@ static ssize_t amdgpu_get_pp_od_clk_voltage(struct device *dev,
 return -EPERM;

 ret = pm_runtime_get_sync(ddev->dev);
-if (ret < 0)
+if (ret < 0) {
+pm_runtime_put_autosuspend(ddev->dev);
 return ret;
+}

 if (is_support_sw_smu(adev)) {
 size = smu_print_clk_levels(&adev->smu, SMU_OD_SCLK, buf); @@ -905,8 +927,10 @@ static ssize_t amdgpu_set_pp_features(struct device *dev,
 pr_debug("featuremask = 0x%llx\n", featuremask);

 ret = pm_runtime_get_sync(ddev->dev);
-if (ret < 0)
+if (ret < 0) {
+pm_runtime_put_autosuspend(ddev->dev);
 return ret;
+}

 if (is_support_sw_smu(adev)) {
 ret = smu_sys_set_pp_feature_mask(&adev->smu, featuremask); @@ -942,8 +966,10 @@ static ssize_t amdgpu_get_pp_features(struct device *dev,
 return -EPERM;

 ret = pm_runtime_get_sync(ddev->dev);
-if (ret < 0)
+if (ret < 0) {
+pm_runtime_put_autosuspend(ddev->dev);
 return ret;
+}

 if (is_support_sw_smu(adev))
 size = smu_sys_get_pp_feature_mask(&adev->smu, buf); @@ -1001,8 +1027,10 @@ static ssize_t amdgpu_get_pp_dpm_sclk(struct device *dev,
 return -EPERM;

 ret = pm_runtime_get_sync(ddev->dev);
-if (ret < 0)
+if (ret < 0) {
+pm_runtime_put_autosuspend(ddev->dev);
 return ret;
+}

 if (is_support_sw_smu(adev))
 size = smu_print_clk_levels(&adev->smu, SMU_SCLK, buf); @@ -1071,8 +1099,10 @@ static ssize_t amdgpu_set_pp_dpm_sclk(struct device *dev,
 return ret;

 ret = pm_runtime_get_sync(ddev->dev);
-if (ret < 0)
+if (ret < 0) {
+pm_runtime_put_autosuspend(ddev->dev);
 return ret;
+}

 if (is_support_sw_smu(adev))
 ret = smu_force_clk_levels(&adev->smu, SMU_SCLK, mask, true); @@ -1101,8 +1131,10 @@ static ssize_t amdgpu_get_pp_dpm_mclk(struct device *dev,
 return -EPERM;

 ret = pm_runtime_get_sync(ddev->dev);
-if (ret < 0)
+if (ret < 0) {
+pm_runtime_put_autosuspend(ddev->dev);
 return ret;
+}

 if (is_support_sw_smu(adev))
 size = smu_print_clk_levels(&adev->smu, SMU_MCLK, buf); @@ -1135,8 +1167,10 @@ static ssize_t amdgpu_set_pp_dpm_mclk(struct device *dev,
 return ret;

 ret = pm_runtime_get_sync(ddev->dev);
-if (ret < 0)
+if (ret < 0) {
+pm_runtime_put_autosuspend(ddev->dev);
 return ret;
+}

 if (is_support_sw_smu(adev))
 ret = smu_force_clk_levels(&adev->smu, SMU_MCLK, mask, true); @@ -1165,8 +1199,10 @@ static ssize_t amdgpu_get_pp_dpm_socclk(struct device *dev,
 return -EPERM;

 ret = pm_runtime_get_sync(ddev->dev);
-if (ret < 0)
+if (ret < 0) {
+pm_runtime_put_autosuspend(ddev->dev);
 return ret;
+}

 if (is_support_sw_smu(adev))
 size = smu_print_clk_levels(&adev->smu, SMU_SOCCLK, buf); @@ -1199,8 +1235,10 @@ static ssize_t amdgpu_set_pp_dpm_socclk(struct device *dev,
 return ret;

 ret = pm_runtime_get_sync(ddev->dev);
-if (ret < 0)
+if (ret < 0) {
+pm_runtime_put_autosuspend(ddev->dev);
 return ret;
+}

 if (is_support_sw_smu(adev))
 ret = smu_force_clk_levels(&adev->smu, SMU_SOCCLK, mask, true); @@ -1231,8 +1269,10 @@ static ssize_t amdgpu_get_pp_dpm_fclk(struct device *dev,
 return -EPERM;

 ret = pm_runtime_get_sync(ddev->dev);
-if (ret < 0)
+if (ret < 0) {
+pm_runtime_put_autosuspend(ddev->dev);
 return ret;
+}

 if (is_support_sw_smu(adev))
 size = smu_print_clk_levels(&adev->smu, SMU_FCLK, buf); @@ -1265,8 +1305,10 @@ static ssize_t amdgpu_set_pp_dpm_fclk(struct device *dev,
 return ret;

 ret = pm_runtime_get_sync(ddev->dev);
-if (ret < 0)
+if (ret < 0) {
+pm_runtime_put_autosuspend(ddev->dev);
 return ret;
+}

 if (is_support_sw_smu(adev))
 ret = smu_force_clk_levels(&adev->smu, SMU_FCLK, mask, true); @@ -1297,8 +1339,10 @@ static ssize_t amdgpu_get_pp_dpm_dcefclk(struct device *dev,
 return -EPERM;

 ret = pm_runtime_get_sync(ddev->dev);
-if (ret < 0)
+if (ret < 0) {
+pm_runtime_put_autosuspend(ddev->dev);
 return ret;
+}

 if (is_support_sw_smu(adev))
 size = smu_print_clk_levels(&adev->smu, SMU_DCEFCLK, buf); @@ -1331,8 +1375,10 @@ static ssize_t amdgpu_set_pp_dpm_dcefclk(struct device *dev,
 return ret;

 ret = pm_runtime_get_sync(ddev->dev);
-if (ret < 0)
+if (ret < 0) {
+pm_runtime_put_autosuspend(ddev->dev);
 return ret;
+}

 if (is_support_sw_smu(adev))
 ret = smu_force_clk_levels(&adev->smu, SMU_DCEFCLK, mask, true); @@ -1363,8 +1409,10 @@ static ssize_t amdgpu_get_pp_dpm_pcie(struct device *dev,
 return -EPERM;

 ret = pm_runtime_get_sync(ddev->dev);
-if (ret < 0)
+if (ret < 0) {
+pm_runtime_put_autosuspend(ddev->dev);
 return ret;
+}

 if (is_support_sw_smu(adev))
 size = smu_print_clk_levels(&adev->smu, SMU_PCIE, buf); @@ -1397,8 +1445,10 @@ static ssize_t amdgpu_set_pp_dpm_pcie(struct device *dev,
 return ret;

 ret = pm_runtime_get_sync(ddev->dev);
-if (ret < 0)
+if (ret < 0) {
+pm_runtime_put_autosuspend(ddev->dev);
 return ret;
+}

 if (is_support_sw_smu(adev))
 ret = smu_force_clk_levels(&adev->smu, SMU_PCIE, mask, true); @@ -1429,8 +1479,10 @@ static ssize_t amdgpu_get_pp_sclk_od(struct device *dev,
 return -EPERM;

 ret = pm_runtime_get_sync(ddev->dev);
-if (ret < 0)
+if (ret < 0) {
+pm_runtime_put_autosuspend(ddev->dev);
 return ret;
+}

 if (is_support_sw_smu(adev))
 value = smu_get_od_percentage(&(adev->smu), SMU_OD_SCLK); @@ -1462,8 +1514,10 @@ static ssize_t amdgpu_set_pp_sclk_od(struct device *dev,
 return -EINVAL;

 ret = pm_runtime_get_sync(ddev->dev);
-if (ret < 0)
+if (ret < 0) {
+pm_runtime_put_autosuspend(ddev->dev);
 return ret;
+}

 if (is_support_sw_smu(adev)) {
 value = smu_set_od_percentage(&(adev->smu), SMU_OD_SCLK, (uint32_t)value); @@ -1498,8 +1552,10 @@ static ssize_t amdgpu_get_pp_mclk_od(struct device *dev,
 return -EPERM;

 ret = pm_runtime_get_sync(ddev->dev);
-if (ret < 0)
+if (ret < 0) {
+pm_runtime_put_autosuspend(ddev->dev);
 return ret;
+}

 if (is_support_sw_smu(adev))
 value = smu_get_od_percentage(&(adev->smu), SMU_OD_MCLK); @@ -1531,8 +1587,10 @@ static ssize_t amdgpu_set_pp_mclk_od(struct device *dev,
 return -EINVAL;

 ret = pm_runtime_get_sync(ddev->dev);
-if (ret < 0)
+if (ret < 0) {
+pm_runtime_put_autosuspend(ddev->dev);
 return ret;
+}

 if (is_support_sw_smu(adev)) {
 value = smu_set_od_percentage(&(adev->smu), SMU_OD_MCLK, (uint32_t)value); @@ -1587,8 +1645,10 @@ static ssize_t amdgpu_get_pp_power_profile_mode(struct device *dev,
 return -EPERM;

 ret = pm_runtime_get_sync(ddev->dev);
-if (ret < 0)
+if (ret < 0) {
+pm_runtime_put_autosuspend(ddev->dev);
 return ret;
+}

 if (is_support_sw_smu(adev))
 size = smu_get_power_profile_mode(&adev->smu, buf); @@ -1650,8 +1710,10 @@ static ssize_t amdgpu_set_pp_power_profile_mode(struct device *dev,
 parameter[parameter_size] = profile_mode;

 ret = pm_runtime_get_sync(ddev->dev);
-if (ret < 0)
+if (ret < 0) {
+pm_runtime_put_autosuspend(ddev->dev);
 return ret;
+}

 if (is_support_sw_smu(adev))
 ret = smu_set_power_profile_mode(&adev->smu, parameter, parameter_size, true); @@ -1687,8 +1749,10 @@ static ssize_t amdgpu_get_gpu_busy_percent(struct device *dev,
 return -EPERM;

 r = pm_runtime_get_sync(ddev->dev);
-if (r < 0)
+if (r < 0) {
+pm_runtime_put_autosuspend(ddev->dev);
 return r;
+}

 /* read the IP busy sensor */
 r = amdgpu_dpm_read_sensor(adev, AMDGPU_PP_SENSOR_GPU_LOAD, @@ -1723,8 +1787,10 @@ static ssize_t amdgpu_get_mem_busy_percent(struct device *dev,
 return -EPERM;

 r = pm_runtime_get_sync(ddev->dev);
-if (r < 0)
+if (r < 0) {
+pm_runtime_put_autosuspend(ddev->dev);
 return r;
+}

 /* read the IP busy sensor */
 r = amdgpu_dpm_read_sensor(adev, AMDGPU_PP_SENSOR_MEM_LOAD, @@ -1770,8 +1836,10 @@ static ssize_t amdgpu_get_pcie_bw(struct device *dev,
 return -ENODATA;

 ret = pm_runtime_get_sync(ddev->dev);
-if (ret < 0)
+if (ret < 0) {
+pm_runtime_put_autosuspend(ddev->dev);
 return ret;
+}

 amdgpu_asic_get_pcie_usage(adev, &count0, &count1);

@@ -2073,8 +2141,10 @@ static ssize_t amdgpu_hwmon_show_temp(struct device *dev,
 return -EINVAL;

 r = pm_runtime_get_sync(adev->ddev->dev);
-if (r < 0)
+if (r < 0) {
+pm_runtime_put_autosuspend(adev->ddev->dev);
 return r;
+}

 switch (channel) {
 case PP_TEMP_JUNCTION:
@@ -2204,8 +2274,10 @@ static ssize_t amdgpu_hwmon_get_pwm1_enable(struct device *dev,
 return -EPERM;

 ret = pm_runtime_get_sync(adev->ddev->dev);
-if (ret < 0)
+if (ret < 0) {
+pm_runtime_put_autosuspend(adev->ddev->dev);
 return ret;
+}

 if (is_support_sw_smu(adev)) {
 pwm_mode = smu_get_fan_control_mode(&adev->smu);
@@ -2242,8 +2314,10 @@ static ssize_t amdgpu_hwmon_set_pwm1_enable(struct device *dev,
 return err;

 ret = pm_runtime_get_sync(adev->ddev->dev);
-if (ret < 0)
+if (ret < 0) {
+pm_runtime_put_autosuspend(adev->ddev->dev);
 return ret;
+}

 if (is_support_sw_smu(adev)) {
 smu_set_fan_control_mode(&adev->smu, value); @@ -2290,8 +2364,10 @@ static ssize_t amdgpu_hwmon_set_pwm1(struct device *dev,
 return -EPERM;

 err = pm_runtime_get_sync(adev->ddev->dev);
-if (err < 0)
+if (err < 0) {
+pm_runtime_put_autosuspend(adev->ddev->dev);
 return err;
+}

 if (is_support_sw_smu(adev))
 pwm_mode = smu_get_fan_control_mode(&adev->smu);
@@ -2342,8 +2418,10 @@ static ssize_t amdgpu_hwmon_get_pwm1(struct device *dev,
 return -EPERM;

 err = pm_runtime_get_sync(adev->ddev->dev);
-if (err < 0)
+if (err < 0) {
+pm_runtime_put_autosuspend(adev->ddev->dev);
 return err;
+}

 if (is_support_sw_smu(adev))
 err = smu_get_fan_speed_percent(&adev->smu, &speed); @@ -2375,8 +2453,10 @@ static ssize_t amdgpu_hwmon_get_fan1_input(struct device *dev,
 return -EPERM;

 err = pm_runtime_get_sync(adev->ddev->dev);
-if (err < 0)
+if (err < 0) {
+pm_runtime_put_autosuspend(adev->ddev->dev);
 return err;
+}

 if (is_support_sw_smu(adev))
 err = smu_get_fan_speed_rpm(&adev->smu, &speed); @@ -2407,8 +2487,10 @@ static ssize_t amdgpu_hwmon_get_fan1_min(struct device *dev,
 return -EPERM;

 r = pm_runtime_get_sync(adev->ddev->dev);
-if (r < 0)
+if (r < 0) {
+pm_runtime_put_autosuspend(adev->ddev->dev);
 return r;
+}

 r = amdgpu_dpm_read_sensor(adev, AMDGPU_PP_SENSOR_MIN_FAN_RPM,
    (void *)&min_rpm, &size);
@@ -2435,8 +2517,10 @@ static ssize_t amdgpu_hwmon_get_fan1_max(struct device *dev,
 return -EPERM;

 r = pm_runtime_get_sync(adev->ddev->dev);
-if (r < 0)
+if (r < 0) {
+pm_runtime_put_autosuspend(adev->ddev->dev);
 return r;
+}

 r = amdgpu_dpm_read_sensor(adev, AMDGPU_PP_SENSOR_MAX_FAN_RPM,
    (void *)&max_rpm, &size);
@@ -2462,8 +2546,10 @@ static ssize_t amdgpu_hwmon_get_fan1_target(struct device *dev,
 return -EPERM;

 err = pm_runtime_get_sync(adev->ddev->dev);
-if (err < 0)
+if (err < 0) {
+pm_runtime_put_autosuspend(adev->ddev->dev);
 return err;
+}

 if (is_support_sw_smu(adev))
 err = smu_get_fan_speed_rpm(&adev->smu, &rpm); @@ -2494,8 +2580,10 @@ static ssize_t amdgpu_hwmon_set_fan1_target(struct device *dev,
 return -EPERM;

 err = pm_runtime_get_sync(adev->ddev->dev);
-if (err < 0)
+if (err < 0) {
+pm_runtime_put_autosuspend(adev->ddev->dev);
 return err;
+}

 if (is_support_sw_smu(adev))
 pwm_mode = smu_get_fan_control_mode(&adev->smu);
@@ -2543,8 +2631,10 @@ static ssize_t amdgpu_hwmon_get_fan1_enable(struct device *dev,
 return -EPERM;

 ret = pm_runtime_get_sync(adev->ddev->dev);
-if (ret < 0)
+if (ret < 0) {
+pm_runtime_put_autosuspend(adev->ddev->dev);
 return ret;
+}

 if (is_support_sw_smu(adev)) {
 pwm_mode = smu_get_fan_control_mode(&adev->smu);
@@ -2589,8 +2679,10 @@ static ssize_t amdgpu_hwmon_set_fan1_enable(struct device *dev,
 return -EINVAL;

 err = pm_runtime_get_sync(adev->ddev->dev);
-if (err < 0)
+if (err < 0) {
+pm_runtime_put_autosuspend(adev->ddev->dev);
 return err;
+}

 if (is_support_sw_smu(adev)) {
 smu_set_fan_control_mode(&adev->smu, pwm_mode); @@ -2621,8 +2713,10 @@ static ssize_t amdgpu_hwmon_show_vddgfx(struct device *dev,
 return -EPERM;

 r = pm_runtime_get_sync(adev->ddev->dev);
-if (r < 0)
+if (r < 0) {
+pm_runtime_put_autosuspend(adev->ddev->dev);
 return r;
+}

 /* get the voltage */
 r = amdgpu_dpm_read_sensor(adev, AMDGPU_PP_SENSOR_VDDGFX, @@ -2660,8 +2754,10 @@ static ssize_t amdgpu_hwmon_show_vddnb(struct device *dev,
 return -EINVAL;

 r = pm_runtime_get_sync(adev->ddev->dev);
-if (r < 0)
+if (r < 0) {
+pm_runtime_put_autosuspend(adev->ddev->dev);
 return r;
+}

 /* get the voltage */
 r = amdgpu_dpm_read_sensor(adev, AMDGPU_PP_SENSOR_VDDNB, @@ -2696,8 +2792,10 @@ static ssize_t amdgpu_hwmon_show_power_avg(struct device *dev,
 return -EPERM;

 r = pm_runtime_get_sync(adev->ddev->dev);
-if (r < 0)
+if (r < 0) {
+pm_runtime_put_autosuspend(adev->ddev->dev);
 return r;
+}

 /* get the voltage */
 r = amdgpu_dpm_read_sensor(adev, AMDGPU_PP_SENSOR_GPU_POWER, @@ -2735,8 +2833,10 @@ static ssize_t amdgpu_hwmon_show_power_cap_max(struct device *dev,
 return -EPERM;

 r = pm_runtime_get_sync(adev->ddev->dev);
-if (r < 0)
+if (r < 0) {
+pm_runtime_put_autosuspend(adev->ddev->dev);
 return r;
+}

 if (is_support_sw_smu(adev)) {
 smu_get_power_limit(&adev->smu, &limit, true); @@ -2767,8 +2867,10 @@ static ssize_t amdgpu_hwmon_show_power_cap(struct device *dev,
 return -EPERM;

 r = pm_runtime_get_sync(adev->ddev->dev);
-if (r < 0)
+if (r < 0) {
+pm_runtime_put_autosuspend(adev->ddev->dev);
 return r;
+}

 if (is_support_sw_smu(adev)) {
 smu_get_power_limit(&adev->smu, &limit, false); @@ -2810,8 +2912,10 @@ static ssize_t amdgpu_hwmon_set_power_cap(struct device *dev,


 err = pm_runtime_get_sync(adev->ddev->dev);
-if (err < 0)
+if (err < 0) {
+pm_runtime_put_autosuspend(adev->ddev->dev);
 return err;
+}

 if (is_support_sw_smu(adev))
 err = smu_set_power_limit(&adev->smu, value); @@ -2841,8 +2945,10 @@ static ssize_t amdgpu_hwmon_show_sclk(struct device *dev,
 return -EPERM;

 r = pm_runtime_get_sync(adev->ddev->dev);
-if (r < 0)
+if (r < 0) {
+pm_runtime_put_autosuspend(adev->ddev->dev);
 return r;
+}

 /* get the sclk */
 r = amdgpu_dpm_read_sensor(adev, AMDGPU_PP_SENSOR_GFX_SCLK, @@ -2876,8 +2982,10 @@ static ssize_t amdgpu_hwmon_show_mclk(struct device *dev,
 return -EPERM;

 r = pm_runtime_get_sync(adev->ddev->dev);
-if (r < 0)
+if (r < 0) {
+pm_runtime_put_autosuspend(adev->ddev->dev);
 return r;
+}

 /* get the sclk */
 r = amdgpu_dpm_read_sensor(adev, AMDGPU_PP_SENSOR_GFX_MCLK, @@ -3739,8 +3847,10 @@ static int amdgpu_debugfs_pm_info(struct seq_file *m, void *data)
 return -EPERM;

 r = pm_runtime_get_sync(dev->dev);
-if (r < 0)
+if (r < 0) {
+pm_runtime_put_autosuspend(dev->dev);
 return r;
+}

 amdgpu_device_ip_get_clockgating_state(adev, &flags);
 seq_printf(m, "Clock Gating Flags Mask: 0x%x\n", flags);
--
2.25.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7Cevan.quan%40amd.com%7Cad0b80e4afa3473948d608d812f0fb82%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637280173604141377&amp;sdata=7q6QSHp4%2Bidxm%2B4M5rFAVryi3zcpbdXoqoLw3ouw%2Fos%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 9+ messages in thread
* RE: [PATCH 1/4] drm/amdgpu/debugfs: fix ref count leak when pm_runtime_get_sync fails
  2020-06-17 19:02 [PATCH 1/4] drm/amdgpu/debugfs: fix ref count leak when pm_runtime_get_sync fails Alex Deucher
                   ` (2 preceding siblings ...)
  2020-06-17 19:02 ` [PATCH 4/4] drm/amdkfd: " Alex Deucher
@ 2020-06-18  4:25 ` Quan, Evan
  3 siblings, 0 replies; 9+ messages in thread
From: Quan, Evan @ 2020-06-18  4:25 UTC (permalink / raw)
  To: Alex Deucher, amd-gfx; +Cc: Deucher, Alexander

[AMD Official Use Only - Internal Distribution Only]

Acked-by: Evan Quan <evan.quan@amd.com>

-----Original Message-----
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Alex Deucher
Sent: Thursday, June 18, 2020 3:02 AM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
Subject: [PATCH 1/4] drm/amdgpu/debugfs: fix ref count leak when pm_runtime_get_sync fails

The call to pm_runtime_get_sync increments the counter even in case of failure, leading to incorrect ref count.
In case of failure, decrement the ref count before returning.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 95 +++++++++++++++------
 1 file changed, 70 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
index 0ac9aab69497..aeada7c9fbea 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
@@ -223,12 +223,16 @@ static int  amdgpu_debugfs_process_reg_op(bool read, struct file *f,
 *pos &= (1UL << 22) - 1;

 r = pm_runtime_get_sync(adev->ddev->dev);
-if (r < 0)
+if (r < 0) {
+pm_runtime_put_autosuspend(adev->ddev->dev);
 return r;
+}

 r = amdgpu_virt_enable_access_debugfs(adev);
-if (r < 0)
+if (r < 0) {
+pm_runtime_put_autosuspend(adev->ddev->dev);
 return r;
+}

 if (use_bank) {
 if ((sh_bank != 0xFFFFFFFF && sh_bank >= adev->gfx.config.max_sh_per_se) || @@ -332,12 +336,16 @@ static ssize_t amdgpu_debugfs_regs_pcie_read(struct file *f, char __user *buf,
 return -EINVAL;

 r = pm_runtime_get_sync(adev->ddev->dev);
-if (r < 0)
+if (r < 0) {
+pm_runtime_put_autosuspend(adev->ddev->dev);
 return r;
+}

 r = amdgpu_virt_enable_access_debugfs(adev);
-if (r < 0)
+if (r < 0) {
+pm_runtime_put_autosuspend(adev->ddev->dev);
 return r;
+}

 while (size) {
 uint32_t value;
@@ -387,12 +395,16 @@ static ssize_t amdgpu_debugfs_regs_pcie_write(struct file *f, const char __user
 return -EINVAL;

 r = pm_runtime_get_sync(adev->ddev->dev);
-if (r < 0)
+if (r < 0) {
+pm_runtime_put_autosuspend(adev->ddev->dev);
 return r;
+}

 r = amdgpu_virt_enable_access_debugfs(adev);
-if (r < 0)
+if (r < 0) {
+pm_runtime_put_autosuspend(adev->ddev->dev);
 return r;
+}

 while (size) {
 uint32_t value;
@@ -443,12 +455,16 @@ static ssize_t amdgpu_debugfs_regs_didt_read(struct file *f, char __user *buf,
 return -EINVAL;

 r = pm_runtime_get_sync(adev->ddev->dev);
-if (r < 0)
+if (r < 0) {
+pm_runtime_put_autosuspend(adev->ddev->dev);
 return r;
+}

 r = amdgpu_virt_enable_access_debugfs(adev);
-if (r < 0)
+if (r < 0) {
+pm_runtime_put_autosuspend(adev->ddev->dev);
 return r;
+}

 while (size) {
 uint32_t value;
@@ -498,12 +514,16 @@ static ssize_t amdgpu_debugfs_regs_didt_write(struct file *f, const char __user
 return -EINVAL;

 r = pm_runtime_get_sync(adev->ddev->dev);
-if (r < 0)
+if (r < 0) {
+pm_runtime_put_autosuspend(adev->ddev->dev);
 return r;
+}

 r = amdgpu_virt_enable_access_debugfs(adev);
-if (r < 0)
+if (r < 0) {
+pm_runtime_put_autosuspend(adev->ddev->dev);
 return r;
+}

 while (size) {
 uint32_t value;
@@ -554,12 +574,16 @@ static ssize_t amdgpu_debugfs_regs_smc_read(struct file *f, char __user *buf,
 return -EINVAL;

 r = pm_runtime_get_sync(adev->ddev->dev);
-if (r < 0)
+if (r < 0) {
+pm_runtime_put_autosuspend(adev->ddev->dev);
 return r;
+}

 r = amdgpu_virt_enable_access_debugfs(adev);
-if (r < 0)
+if (r < 0) {
+pm_runtime_put_autosuspend(adev->ddev->dev);
 return r;
+}

 while (size) {
 uint32_t value;
@@ -609,12 +633,16 @@ static ssize_t amdgpu_debugfs_regs_smc_write(struct file *f, const char __user *
 return -EINVAL;

 r = pm_runtime_get_sync(adev->ddev->dev);
-if (r < 0)
+if (r < 0) {
+pm_runtime_put_autosuspend(adev->ddev->dev);
 return r;
+}

 r = amdgpu_virt_enable_access_debugfs(adev);
-if (r < 0)
+if (r < 0) {
+pm_runtime_put_autosuspend(adev->ddev->dev);
 return r;
+}

 while (size) {
 uint32_t value;
@@ -764,12 +792,16 @@ static ssize_t amdgpu_debugfs_sensor_read(struct file *f, char __user *buf,
 valuesize = sizeof(values);

 r = pm_runtime_get_sync(adev->ddev->dev);
-if (r < 0)
+if (r < 0) {
+pm_runtime_put_autosuspend(adev->ddev->dev);
 return r;
+}

 r = amdgpu_virt_enable_access_debugfs(adev);
-if (r < 0)
+if (r < 0) {
+pm_runtime_put_autosuspend(adev->ddev->dev);
 return r;
+}

 r = amdgpu_dpm_read_sensor(adev, idx, &values[0], &valuesize);

@@ -842,12 +874,16 @@ static ssize_t amdgpu_debugfs_wave_read(struct file *f, char __user *buf,
 simd = (*pos & GENMASK_ULL(44, 37)) >> 37;

 r = pm_runtime_get_sync(adev->ddev->dev);
-if (r < 0)
+if (r < 0) {
+pm_runtime_put_autosuspend(adev->ddev->dev);
 return r;
+}

 r = amdgpu_virt_enable_access_debugfs(adev);
-if (r < 0)
+if (r < 0) {
+pm_runtime_put_autosuspend(adev->ddev->dev);
 return r;
+}

 /* switch to the specific se/sh/cu */
 mutex_lock(&adev->grbm_idx_mutex);
@@ -981,6 +1017,7 @@ static ssize_t amdgpu_debugfs_gpr_read(struct file *f, char __user *buf,
 return result;

 err:
+pm_runtime_put_autosuspend(adev->ddev->dev);
 kfree(data);
 return r;
 }
@@ -1006,8 +1043,10 @@ static ssize_t amdgpu_debugfs_gfxoff_write(struct file *f, const char __user *bu
 return -EINVAL;

 r = pm_runtime_get_sync(adev->ddev->dev);
-if (r < 0)
+if (r < 0) {
+pm_runtime_put_autosuspend(adev->ddev->dev);
 return r;
+}

 while (size) {
 uint32_t value;
@@ -1143,8 +1182,10 @@ static int amdgpu_debugfs_test_ib(struct seq_file *m, void *data)
 int r = 0, i;

 r = pm_runtime_get_sync(dev->dev);
-if (r < 0)
+if (r < 0) {
+pm_runtime_put_autosuspend(adev->ddev->dev);
 return r;
+}

 /* Avoid accidently unparking the sched thread during GPU reset */
 mutex_lock(&adev->lock_reset);
@@ -1200,8 +1241,10 @@ static int amdgpu_debugfs_evict_vram(struct seq_file *m, void *data)
 int r;

 r = pm_runtime_get_sync(dev->dev);
-if (r < 0)
+if (r < 0) {
+pm_runtime_put_autosuspend(adev->ddev->dev);
 return r;
+}

 seq_printf(m, "(%d)\n", amdgpu_bo_evict_vram(adev));

@@ -1219,8 +1262,10 @@ static int amdgpu_debugfs_evict_gtt(struct seq_file *m, void *data)
 int r;

 r = pm_runtime_get_sync(dev->dev);
-if (r < 0)
+if (r < 0) {
+pm_runtime_put_autosuspend(adev->ddev->dev);
 return r;
+}

 seq_printf(m, "(%d)\n", ttm_bo_evict_mm(&adev->mman.bdev, TTM_PL_TT));

@@ -1410,16 +1455,16 @@ static int amdgpu_debugfs_sclk_set(void *data, u64 val)
 return -EINVAL;

 ret = pm_runtime_get_sync(adev->ddev->dev);
-if (ret < 0)
+if (ret < 0) {
+pm_runtime_put_autosuspend(adev->ddev->dev);
 return ret;
+}

 if (is_support_sw_smu(adev)) {
 ret = smu_get_dpm_freq_range(&adev->smu, SMU_SCLK, &min_freq, &max_freq, true);
 if (ret || val > max_freq || val < min_freq)
 return -EINVAL;
 ret = smu_set_soft_freq_range(&adev->smu, SMU_SCLK, (uint32_t)val, (uint32_t)val, true);
-} else {
-return 0;
 }

 pm_runtime_mark_last_busy(adev->ddev->dev);
--
2.25.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7Cevan.quan%40amd.com%7C1860e6d2bd8647d39ec108d812f0f8ef%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637280173541390159&amp;sdata=MVz3X2G09LrL6rLSgLDG5IKbOkLHOlkXKvrtMTR1Ics%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-06-18  4:25 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-17 19:02 [PATCH 1/4] drm/amdgpu/debugfs: fix ref count leak when pm_runtime_get_sync fails Alex Deucher
2020-06-17 19:02 ` [PATCH 2/4] drm/amdgpu/pm: " Alex Deucher
2020-06-18  4:24   ` Quan, Evan
2020-06-17 19:02 ` [PATCH 3/4] drm/amdgpu/fence: " Alex Deucher
2020-06-17 20:27   ` Bhardwaj, Rajneesh
2020-06-17 19:02 ` [PATCH 4/4] drm/amdkfd: " Alex Deucher
2020-06-17 20:16   ` Felix Kuehling
2020-06-17 20:26     ` Bhardwaj, Rajneesh
2020-06-18  4:25 ` [PATCH 1/4] drm/amdgpu/debugfs: " Quan, Evan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.