Re: [PATCH] drm/amdgpu: fix sysfs_emit/sysfs_emit_at warnings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 9/8/2021 1:14 PM, Yu, Lang wrote:

[AMD Official Use Only]




-----Original Message-----
From: Lazar, Lijo <Lijo.Lazar@xxxxxxx>
Sent: Wednesday, September 8, 2021 3:36 PM
To: Christian König <ckoenig.leichtzumerken@xxxxxxxxx>; Yu, Lang
<Lang.Yu@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Huang, Ray
<Ray.Huang@xxxxxxx>; Tian Tao <tiantao6@xxxxxxxxxxxxx>
Subject: Re: [PATCH] drm/amdgpu: fix sysfs_emit/sysfs_emit_at warnings



On 9/8/2021 12:07 PM, Christian König wrote:

Am 08.09.21 um 07:56 schrieb Lang Yu:

sysfs_emit and sysfs_emit_at requrie a page boundary aligned buf
address. Make them happy!

Warning Log:
[  492.545174] invalid sysfs_emit_at: buf:00000000f19bdfde at:0 [
492.546416] WARNING: CPU: 7 PID: 1304 at fs/sysfs/file.c:765
sysfs_emit_at+0x4a/0xa0
[  492.654805] Call Trace:
[  492.655353]  ? smu_cmn_get_metrics_table+0x40/0x50 [amdgpu] [
492.656780]  vangogh_print_clk_levels+0x369/0x410 [amdgpu] [
492.658245]  vangogh_common_print_clk_levels+0x77/0x80 [amdgpu] [
492.659733]  ? preempt_schedule_common+0x18/0x30 [  492.660713]
smu_print_ppclk_levels+0x65/0x90 [amdgpu] [  492.662107]
amdgpu_get_pp_od_clk_voltage+0x13d/0x190 [amdgpu] [  492.663620]
dev_attr_show+0x1d/0x40


Mhm, that at least partially doesn't looks like the right approach to me.

Why do we have string printing and sysfs code in the hardware version
specific backend in the first place?



This is a callback meant for printing ASIC specific information to sysfs node. The
buffer passed in sysfs read is passed as it is to the callback API.


That stuff needs to be implemented for each hardware generation and is
now cluttered with sysfs buffer offset calculations.



Looks like the warning happened because of this usage.

                 size = amdgpu_dpm_print_clock_levels(adev, OD_SCLK, buf);
                 size += amdgpu_dpm_print_clock_levels(adev, OD_MCLK,
buf+size);
                 size += amdgpu_dpm_print_clock_levels(adev,
OD_VDDC_CURVE, buf+size);
                 size += amdgpu_dpm_print_clock_levels(adev,
OD_VDDGFX_OFFSET, buf+size);
                 size += amdgpu_dpm_print_clock_levels(adev, OD_RANGE,
buf+size);
                 size += amdgpu_dpm_print_clock_levels(adev, OD_CCLK,
buf+size);



[Yu, Lang]
Yes. So it is fine we just fix the caller amdgpu_get_pp_od_clk_voltage like following:

static ssize_t amdgpu_get_pp_od_clk_voltage(struct device *dev,
		struct device_attribute *attr,
		char *buf)
{
	struct drm_device *ddev = dev_get_drvdata(dev);
	struct amdgpu_device *adev = drm_to_adev(ddev);
	ssize_t size, offset;
	int ret, i;
	char temp_buf[512];
	char clock_type[] = {OD_SCLK, OD_MCLK, OD_VDDC_CURVE,
	                     OD_VDDGFX_OFFSET, OD_RANGE, OD_CCLK};

	if (amdgpu_in_reset(adev))
		return -EPERM;
	if (adev->in_suspend && !adev->in_runpm)
		return -EPERM;

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

	offset = 0;

	if (adev->powerplay.pp_funcs->print_clock_levels) {	
		for (i = 0; i < ARRAY_SIZE(clock_type); i++) {
			size = amdgpu_dpm_print_clock_levels(adev, clock_type[i], buf);
			if (offset + size > PAGE_SIZE)
				break;
			memcpy(temp_buf + offset, buf, size);
			offset += size;
		}
		memcpy(buf, temp_buf, offset);
		size = offset;
	} else {
		size = sysfs_emit(buf, "\n");
	}
	pm_runtime_mark_last_busy(ddev->dev);
	pm_runtime_put_autosuspend(ddev->dev);

	return size;
}
Prefer to avoid any extra stack or heap usage for buffer. Maybe another arg to pass offset along with buf? 

Thanks,
Lijo


Regards,
Lang




Regards,
Christian.



Signed-off-by: Lang Yu <lang.yu@xxxxxxx>
---
   drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c |  9 +++++++--
   drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c   |  5 ++++-
   .../drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c   |  5 ++++-
   drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c  | 15
+++++++++------
   drivers/gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c   |  3 +++
   .../gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c    | 13
+++++++++----
   .../gpu/drm/amd/pm/swsmu/smu13/yellow_carp_ppt.c  |  7 +++++--
   7 files changed, 41 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
index e343cc218990..53185fe96d83 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
@@ -771,8 +771,13 @@ static int arcturus_print_clk_levels(struct
smu_context *smu,
       struct smu_11_0_dpm_context *dpm_context = NULL;
       uint32_t gen_speed, lane_width;
-    if (amdgpu_ras_intr_triggered())
-        return sysfs_emit(buf, "unavailable\n");
+    size = offset_in_page(buf);
+    buf = buf - size;
+
+    if (amdgpu_ras_intr_triggered()) {
+        size += sysfs_emit_at(buf, size, "unavailable\n");
+        return size;
+    }
       dpm_context = smu_dpm->dpm_context; diff --git
a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
index 4c81989b8162..5490e8e66e14 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
@@ -1279,6 +1279,9 @@ static int navi10_print_clk_levels(struct
smu_context *smu,
       struct smu_11_0_overdrive_table *od_settings =
smu->od_settings;
       uint32_t min_value, max_value;
+    size = offset_in_page(buf);
+    buf = buf - size;
+
       switch (clk_type) {
       case SMU_GFXCLK:
       case SMU_SCLK:
@@ -1392,7 +1395,7 @@ static int navi10_print_clk_levels(struct
smu_context *smu,
       case SMU_OD_RANGE:
           if (!smu->od_enabled || !od_table || !od_settings)
               break;
-        size = sysfs_emit(buf, "%s:\n", "OD_RANGE");
+        size += sysfs_emit_at(buf, size, "%s:\n", "OD_RANGE");
           if (navi10_od_feature_is_supported(od_settings,
SMU_11_0_ODCAP_GFXCLK_LIMITS)) {
               navi10_od_setting_get_range(od_settings,
SMU_11_0_ODSETTING_GFXCLKFMIN,
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
index 5e292c3f5050..817ad6de3854 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
@@ -1058,6 +1058,9 @@ static int
sienna_cichlid_print_clk_levels(struct smu_context *smu,
       uint32_t min_value, max_value;
       uint32_t smu_version;
+    size = offset_in_page(buf);
+    buf = buf - size;
+
       switch (clk_type) {
       case SMU_GFXCLK:
       case SMU_SCLK:
@@ -1180,7 +1183,7 @@ static int
sienna_cichlid_print_clk_levels(struct smu_context *smu,
           if (!smu->od_enabled || !od_table || !od_settings)
               break;
-        size = sysfs_emit(buf, "%s:\n", "OD_RANGE");
+        size += sysfs_emit_at(buf, size, "%s:\n", "OD_RANGE");
           if (sienna_cichlid_is_od_feature_supported(od_settings,
SMU_11_0_7_ODCAP_GFXCLK_LIMITS)) {
               sienna_cichlid_get_od_setting_range(od_settings,
SMU_11_0_7_ODSETTING_GFXCLKFMIN,
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
b/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
index 3a3421452e57..c7842c69b570 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
@@ -592,7 +592,7 @@ static int vangogh_print_legacy_clk_levels(struct
smu_context *smu,
       switch (clk_type) {
       case SMU_OD_SCLK:
           if (smu_dpm_ctx->dpm_level == AMD_DPM_FORCED_LEVEL_MANUAL)
{
-            size = sysfs_emit(buf, "%s:\n", "OD_SCLK");
+            size += sysfs_emit_at(buf, size, "%s:\n", "OD_SCLK");
               size += sysfs_emit_at(buf, size, "0: %10uMhz\n",
               (smu->gfx_actual_hard_min_freq > 0) ?
smu->gfx_actual_hard_min_freq : smu->gfx_default_hard_min_freq);
               size += sysfs_emit_at(buf, size, "1: %10uMhz\n", @@
-601,7 +601,7 @@ static int vangogh_print_legacy_clk_levels(struct
smu_context *smu,
           break;
       case SMU_OD_CCLK:
           if (smu_dpm_ctx->dpm_level == AMD_DPM_FORCED_LEVEL_MANUAL)
{
-            size = sysfs_emit(buf, "CCLK_RANGE in Core%d:\n",
smu->cpu_core_id_select);
+            size += sysfs_emit_at(buf, size, "CCLK_RANGE in
Core%d:\n",  smu->cpu_core_id_select);
               size += sysfs_emit_at(buf, size, "0: %10uMhz\n",
               (smu->cpu_actual_soft_min_freq > 0) ?
smu->cpu_actual_soft_min_freq : smu->cpu_default_soft_min_freq);
               size += sysfs_emit_at(buf, size, "1: %10uMhz\n", @@
-610,7 +610,7 @@ static int vangogh_print_legacy_clk_levels(struct
smu_context *smu,
           break;
       case SMU_OD_RANGE:
           if (smu_dpm_ctx->dpm_level == AMD_DPM_FORCED_LEVEL_MANUAL)
{
-            size = sysfs_emit(buf, "%s:\n", "OD_RANGE");
+            size += sysfs_emit_at(buf, size, "%s:\n", "OD_RANGE");
               size += sysfs_emit_at(buf, size, "SCLK: %7uMhz
%10uMhz\n",
                   smu->gfx_default_hard_min_freq,
smu->gfx_default_soft_max_freq);
               size += sysfs_emit_at(buf, size, "CCLK: %7uMhz
%10uMhz\n", @@ -682,6 +682,9 @@ static int
vangogh_print_clk_levels(struct smu_context *smu,
       uint32_t cur_value = 0, value = 0, count = 0;
       bool cur_value_match_level = false;
+    size = offset_in_page(buf);
+    buf = buf - size;
+
       memset(&metrics, 0, sizeof(metrics));
       ret = smu_cmn_get_metrics_table(smu, &metrics, false); @@
-691,7 +694,7 @@ static int vangogh_print_clk_levels(struct
smu_context *smu,
       switch (clk_type) {
       case SMU_OD_SCLK:
           if (smu_dpm_ctx->dpm_level == AMD_DPM_FORCED_LEVEL_MANUAL)
{
-            size = sysfs_emit(buf, "%s:\n", "OD_SCLK");
+            size += sysfs_emit_at(buf, size, "%s:\n", "OD_SCLK");
               size += sysfs_emit_at(buf, size, "0: %10uMhz\n",
               (smu->gfx_actual_hard_min_freq > 0) ?
smu->gfx_actual_hard_min_freq : smu->gfx_default_hard_min_freq);
               size += sysfs_emit_at(buf, size, "1: %10uMhz\n", @@
-700,7 +703,7 @@ static int vangogh_print_clk_levels(struct
smu_context *smu,
           break;
       case SMU_OD_CCLK:
           if (smu_dpm_ctx->dpm_level == AMD_DPM_FORCED_LEVEL_MANUAL)
{
-            size = sysfs_emit(buf, "CCLK_RANGE in Core%d:\n",
smu->cpu_core_id_select);
+            size += sysfs_emit_at(buf, size, "CCLK_RANGE in
Core%d:\n",  smu->cpu_core_id_select);
               size += sysfs_emit_at(buf, size, "0: %10uMhz\n",
               (smu->cpu_actual_soft_min_freq > 0) ?
smu->cpu_actual_soft_min_freq : smu->cpu_default_soft_min_freq);
               size += sysfs_emit_at(buf, size, "1: %10uMhz\n", @@
-709,7 +712,7 @@ static int vangogh_print_clk_levels(struct
smu_context *smu,
           break;
       case SMU_OD_RANGE:
           if (smu_dpm_ctx->dpm_level == AMD_DPM_FORCED_LEVEL_MANUAL)
{
-            size = sysfs_emit(buf, "%s:\n", "OD_RANGE");
+            size += sysfs_emit_at(buf, size, "%s:\n", "OD_RANGE");
               size += sysfs_emit_at(buf, size, "SCLK: %7uMhz
%10uMhz\n",
                   smu->gfx_default_hard_min_freq,
smu->gfx_default_soft_max_freq);
               size += sysfs_emit_at(buf, size, "CCLK: %7uMhz
%10uMhz\n", diff --git
a/drivers/gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c
b/drivers/gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c
index 5aa175e12a78..86e7978b6d63 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c
@@ -491,6 +491,9 @@ static int renoir_print_clk_levels(struct
smu_context *smu,
       struct smu_dpm_context *smu_dpm_ctx = &(smu->smu_dpm);
       bool cur_value_match_level = false;
+    size = offset_in_page(buf);
+    buf = buf - size;
+
       memset(&metrics, 0, sizeof(metrics));
       ret = smu_cmn_get_metrics_table(smu, &metrics, false); diff
--git a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
index ab652028e003..6349f27e9efc 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
@@ -733,15 +733,20 @@ static int aldebaran_print_clk_levels(struct
smu_context *smu,
       uint32_t freq_values[3] = {0};
       uint32_t min_clk, max_clk;
-    if (amdgpu_ras_intr_triggered())
-        return sysfs_emit(buf, "unavailable\n");
+    size = offset_in_page(buf);
+    buf = buf - size;
+
+    if (amdgpu_ras_intr_triggered()) {
+        size += sysfs_emit_at(buf, size, "unavailable\n");
+        return size;
+    }
       dpm_context = smu_dpm->dpm_context;
       switch (type) {
       case SMU_OD_SCLK:
-        size = sysfs_emit(buf, "%s:\n", "GFXCLK");
+        size += sysfs_emit_at(buf, size, "%s:\n", "GFXCLK");
           fallthrough;
       case SMU_SCLK:
           ret = aldebaran_get_current_clk_freq_by_table(smu,
SMU_GFXCLK, &now);
@@ -795,7 +800,7 @@ static int aldebaran_print_clk_levels(struct
smu_context *smu,
           break;
       case SMU_OD_MCLK:
-        size = sysfs_emit(buf, "%s:\n", "MCLK");
+        size += sysfs_emit_at(buf, size, "%s:\n", "MCLK");
           fallthrough;
       case SMU_MCLK:
           ret = aldebaran_get_current_clk_freq_by_table(smu,
SMU_UCLK, &now); diff --git
a/drivers/gpu/drm/amd/pm/swsmu/smu13/yellow_carp_ppt.c
b/drivers/gpu/drm/amd/pm/swsmu/smu13/yellow_carp_ppt.c
index 627ba2eec7fd..3b21d9143b96 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/yellow_carp_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/yellow_carp_ppt.c
@@ -1052,16 +1052,19 @@ static int
yellow_carp_print_clk_levels(struct
smu_context *smu,
       int i, size = 0, ret = 0;
       uint32_t cur_value = 0, value = 0, count = 0;
+    size = offset_in_page(buf);
+    buf = buf - size;
+
       switch (clk_type) {
       case SMU_OD_SCLK:
-        size = sysfs_emit(buf, "%s:\n", "OD_SCLK");
+        size += sysfs_emit_at(buf, size, "%s:\n", "OD_SCLK");
           size += sysfs_emit_at(buf, size, "0: %10uMhz\n",
           (smu->gfx_actual_hard_min_freq > 0) ?
smu->gfx_actual_hard_min_freq : smu->gfx_default_hard_min_freq);
           size += sysfs_emit_at(buf, size, "1: %10uMhz\n",
           (smu->gfx_actual_soft_max_freq > 0) ?
smu->gfx_actual_soft_max_freq : smu->gfx_default_soft_max_freq);
           break;
       case SMU_OD_RANGE:
-        size = sysfs_emit(buf, "%s:\n", "OD_RANGE");
+        size += sysfs_emit_at(buf, size, "%s:\n", "OD_RANGE");
           size += sysfs_emit_at(buf, size, "SCLK: %7uMhz %10uMhz\n",
                           smu->gfx_default_hard_min_freq,
smu->gfx_default_soft_max_freq);
           break;
[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux