[PATCH 1/1] drm/amdkfd: Fix GCC 10 compiler warning

Lin, Amber Amber.Lin at amd.com
Thu May 28 02:25:51 UTC 2020


[AMD Public Use]

Reviewed-by:  Amber Lin <Amber.Lin at amd.com>


-----Original Message-----
From: Kuehling, Felix <Felix.Kuehling at amd.com> 
Sent: Monday, May 25, 2020 10:47 PM
To: amd-gfx at lists.freedesktop.org
Cc: Lin, Amber <Amber.Lin at amd.com>
Subject: [PATCH 1/1] drm/amdkfd: Fix GCC 10 compiler warning

GCC 10 was complaining about how we append data to a buffer using snprintf:

drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c: In function ‘perf_show’:
drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:214:3: warning: ‘snprintf’ argument 4 overlaps destination object ‘buf’ [-Wrestrict]
  214 |   snprintf(buffer, PAGE_SIZE, "%s"fmt, buffer, __VA_ARGS__)
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This patch fixes the warnings and makes the sysfs code more efficient by remembering the offset in the buffer between append operations.

Signed-off-by: Felix Kuehling <Felix.Kuehling at amd.com>
Acked-by: Aaron Liu <aaron.liu at amd.com>
Tested-by: Aaron Liu <aaron.liu at amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 274 +++++++++++-----------
 1 file changed, 139 insertions(+), 135 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
index bb77f7af2b6d..d5e2585d6f34 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
@@ -210,39 +210,41 @@ struct kfd_topology_device *kfd_create_topology_device(  }
 
 
-#define sysfs_show_gen_prop(buffer, fmt, ...) \
-		snprintf(buffer, PAGE_SIZE, "%s"fmt, buffer, __VA_ARGS__)
-#define sysfs_show_32bit_prop(buffer, name, value) \
-		sysfs_show_gen_prop(buffer, "%s %u\n", name, value)
-#define sysfs_show_64bit_prop(buffer, name, value) \
-		sysfs_show_gen_prop(buffer, "%s %llu\n", name, value)
-#define sysfs_show_32bit_val(buffer, value) \
-		sysfs_show_gen_prop(buffer, "%u\n", value)
-#define sysfs_show_str_val(buffer, value) \
-		sysfs_show_gen_prop(buffer, "%s\n", value)
+#define sysfs_show_gen_prop(buffer, offs, fmt, ...)		\
+		(offs += snprintf(buffer+offs, PAGE_SIZE-offs,	\
+				  fmt, __VA_ARGS__))
+#define sysfs_show_32bit_prop(buffer, offs, name, value) \
+		sysfs_show_gen_prop(buffer, offs, "%s %u\n", name, value) #define 
+sysfs_show_64bit_prop(buffer, offs, name, value) \
+		sysfs_show_gen_prop(buffer, offs, "%s %llu\n", name, value) #define 
+sysfs_show_32bit_val(buffer, offs, value) \
+		sysfs_show_gen_prop(buffer, offs, "%u\n", value) #define 
+sysfs_show_str_val(buffer, offs, value) \
+		sysfs_show_gen_prop(buffer, offs, "%s\n", value)
 
 static ssize_t sysprops_show(struct kobject *kobj, struct attribute *attr,
 		char *buffer)
 {
-	ssize_t ret;
+	int offs = 0;
 
 	/* Making sure that the buffer is an empty string */
 	buffer[0] = 0;
 
 	if (attr == &sys_props.attr_genid) {
-		ret = sysfs_show_32bit_val(buffer, sys_props.generation_count);
+		sysfs_show_32bit_val(buffer, offs,
+				     sys_props.generation_count);
 	} else if (attr == &sys_props.attr_props) {
-		sysfs_show_64bit_prop(buffer, "platform_oem",
-				sys_props.platform_oem);
-		sysfs_show_64bit_prop(buffer, "platform_id",
-				sys_props.platform_id);
-		ret = sysfs_show_64bit_prop(buffer, "platform_rev",
-				sys_props.platform_rev);
+		sysfs_show_64bit_prop(buffer, offs, "platform_oem",
+				      sys_props.platform_oem);
+		sysfs_show_64bit_prop(buffer, offs, "platform_id",
+				      sys_props.platform_id);
+		sysfs_show_64bit_prop(buffer, offs, "platform_rev",
+				      sys_props.platform_rev);
 	} else {
-		ret = -EINVAL;
+		offs = -EINVAL;
 	}
 
-	return ret;
+	return offs;
 }
 
 static void kfd_topology_kobj_release(struct kobject *kobj) @@ -262,7 +264,7 @@ static struct kobj_type sysprops_type = {  static ssize_t iolink_show(struct kobject *kobj, struct attribute *attr,
 		char *buffer)
 {
-	ssize_t ret;
+	int offs = 0;
 	struct kfd_iolink_properties *iolink;
 
 	/* Making sure that the buffer is an empty string */ @@ -271,21 +273,23 @@ static ssize_t iolink_show(struct kobject *kobj, struct attribute *attr,
 	iolink = container_of(attr, struct kfd_iolink_properties, attr);
 	if (iolink->gpu && kfd_devcgroup_check_permission(iolink->gpu))
 		return -EPERM;
-	sysfs_show_32bit_prop(buffer, "type", iolink->iolink_type);
-	sysfs_show_32bit_prop(buffer, "version_major", iolink->ver_maj);
-	sysfs_show_32bit_prop(buffer, "version_minor", iolink->ver_min);
-	sysfs_show_32bit_prop(buffer, "node_from", iolink->node_from);
-	sysfs_show_32bit_prop(buffer, "node_to", iolink->node_to);
-	sysfs_show_32bit_prop(buffer, "weight", iolink->weight);
-	sysfs_show_32bit_prop(buffer, "min_latency", iolink->min_latency);
-	sysfs_show_32bit_prop(buffer, "max_latency", iolink->max_latency);
-	sysfs_show_32bit_prop(buffer, "min_bandwidth", iolink->min_bandwidth);
-	sysfs_show_32bit_prop(buffer, "max_bandwidth", iolink->max_bandwidth);
-	sysfs_show_32bit_prop(buffer, "recommended_transfer_size",
-			iolink->rec_transfer_size);
-	ret = sysfs_show_32bit_prop(buffer, "flags", iolink->flags);
-
-	return ret;
+	sysfs_show_32bit_prop(buffer, offs, "type", iolink->iolink_type);
+	sysfs_show_32bit_prop(buffer, offs, "version_major", iolink->ver_maj);
+	sysfs_show_32bit_prop(buffer, offs, "version_minor", iolink->ver_min);
+	sysfs_show_32bit_prop(buffer, offs, "node_from", iolink->node_from);
+	sysfs_show_32bit_prop(buffer, offs, "node_to", iolink->node_to);
+	sysfs_show_32bit_prop(buffer, offs, "weight", iolink->weight);
+	sysfs_show_32bit_prop(buffer, offs, "min_latency", iolink->min_latency);
+	sysfs_show_32bit_prop(buffer, offs, "max_latency", iolink->max_latency);
+	sysfs_show_32bit_prop(buffer, offs, "min_bandwidth",
+			      iolink->min_bandwidth);
+	sysfs_show_32bit_prop(buffer, offs, "max_bandwidth",
+			      iolink->max_bandwidth);
+	sysfs_show_32bit_prop(buffer, offs, "recommended_transfer_size",
+			      iolink->rec_transfer_size);
+	sysfs_show_32bit_prop(buffer, offs, "flags", iolink->flags);
+
+	return offs;
 }
 
 static const struct sysfs_ops iolink_ops = { @@ -300,7 +304,7 @@ static struct kobj_type iolink_type = {  static ssize_t mem_show(struct kobject *kobj, struct attribute *attr,
 		char *buffer)
 {
-	ssize_t ret;
+	int offs = 0;
 	struct kfd_mem_properties *mem;
 
 	/* Making sure that the buffer is an empty string */ @@ -309,13 +313,15 @@ static ssize_t mem_show(struct kobject *kobj, struct attribute *attr,
 	mem = container_of(attr, struct kfd_mem_properties, attr);
 	if (mem->gpu && kfd_devcgroup_check_permission(mem->gpu))
 		return -EPERM;
-	sysfs_show_32bit_prop(buffer, "heap_type", mem->heap_type);
-	sysfs_show_64bit_prop(buffer, "size_in_bytes", mem->size_in_bytes);
-	sysfs_show_32bit_prop(buffer, "flags", mem->flags);
-	sysfs_show_32bit_prop(buffer, "width", mem->width);
-	ret = sysfs_show_32bit_prop(buffer, "mem_clk_max", mem->mem_clk_max);
-
-	return ret;
+	sysfs_show_32bit_prop(buffer, offs, "heap_type", mem->heap_type);
+	sysfs_show_64bit_prop(buffer, offs, "size_in_bytes",
+			      mem->size_in_bytes);
+	sysfs_show_32bit_prop(buffer, offs, "flags", mem->flags);
+	sysfs_show_32bit_prop(buffer, offs, "width", mem->width);
+	sysfs_show_32bit_prop(buffer, offs, "mem_clk_max",
+			      mem->mem_clk_max);
+
+	return offs;
 }
 
 static const struct sysfs_ops mem_ops = { @@ -330,7 +336,7 @@ static struct kobj_type mem_type = {  static ssize_t kfd_cache_show(struct kobject *kobj, struct attribute *attr,
 		char *buffer)
 {
-	ssize_t ret;
+	int offs = 0;
 	uint32_t i, j;
 	struct kfd_cache_properties *cache;
 
@@ -340,30 +346,27 @@ static ssize_t kfd_cache_show(struct kobject *kobj, struct attribute *attr,
 	cache = container_of(attr, struct kfd_cache_properties, attr);
 	if (cache->gpu && kfd_devcgroup_check_permission(cache->gpu))
 		return -EPERM;
-	sysfs_show_32bit_prop(buffer, "processor_id_low",
+	sysfs_show_32bit_prop(buffer, offs, "processor_id_low",
 			cache->processor_id_low);
-	sysfs_show_32bit_prop(buffer, "level", cache->cache_level);
-	sysfs_show_32bit_prop(buffer, "size", cache->cache_size);
-	sysfs_show_32bit_prop(buffer, "cache_line_size", cache->cacheline_size);
-	sysfs_show_32bit_prop(buffer, "cache_lines_per_tag",
-			cache->cachelines_per_tag);
-	sysfs_show_32bit_prop(buffer, "association", cache->cache_assoc);
-	sysfs_show_32bit_prop(buffer, "latency", cache->cache_latency);
-	sysfs_show_32bit_prop(buffer, "type", cache->cache_type);
-	snprintf(buffer, PAGE_SIZE, "%ssibling_map ", buffer);
+	sysfs_show_32bit_prop(buffer, offs, "level", cache->cache_level);
+	sysfs_show_32bit_prop(buffer, offs, "size", cache->cache_size);
+	sysfs_show_32bit_prop(buffer, offs, "cache_line_size",
+			      cache->cacheline_size);
+	sysfs_show_32bit_prop(buffer, offs, "cache_lines_per_tag",
+			      cache->cachelines_per_tag);
+	sysfs_show_32bit_prop(buffer, offs, "association", cache->cache_assoc);
+	sysfs_show_32bit_prop(buffer, offs, "latency", cache->cache_latency);
+	sysfs_show_32bit_prop(buffer, offs, "type", cache->cache_type);
+	offs += snprintf(buffer+offs, PAGE_SIZE-offs, "sibling_map ");
 	for (i = 0; i < CRAT_SIBLINGMAP_SIZE; i++)
-		for (j = 0; j < sizeof(cache->sibling_map[0])*8; j++) {
+		for (j = 0; j < sizeof(cache->sibling_map[0])*8; j++)
 			/* Check each bit */
-			if (cache->sibling_map[i] & (1 << j))
-				ret = snprintf(buffer, PAGE_SIZE,
-					 "%s%d%s", buffer, 1, ",");
-			else
-				ret = snprintf(buffer, PAGE_SIZE,
-					 "%s%d%s", buffer, 0, ",");
-		}
+			offs += snprintf(buffer+offs, PAGE_SIZE-offs, "%d,",
+					 (cache->sibling_map[i] >> j) & 1);
+
 	/* Replace the last "," with end of line */
-	*(buffer + strlen(buffer) - 1) = 0xA;
-	return ret;
+	buffer[offs-1] = '\n';
+	return offs;
 }
 
 static const struct sysfs_ops cache_ops = { @@ -385,6 +388,7 @@ struct kfd_perf_attr {  static ssize_t perf_show(struct kobject *kobj, struct kobj_attribute *attrs,
 			char *buf)
 {
+	int offs = 0;
 	struct kfd_perf_attr *attr;
 
 	buf[0] = 0;
@@ -392,7 +396,7 @@ static ssize_t perf_show(struct kobject *kobj, struct kobj_attribute *attrs,
 	if (!attr->data) /* invalid data for PMC */
 		return 0;
 	else
-		return sysfs_show_32bit_val(buf, attr->data);
+		return sysfs_show_32bit_val(buf, offs, attr->data);
 }
 
 #define KFD_PERF_DESC(_name, _data)			\
@@ -411,6 +415,7 @@ static struct kfd_perf_attr perf_attr_iommu[] = {  static ssize_t node_show(struct kobject *kobj, struct attribute *attr,
 		char *buffer)
 {
+	int offs = 0;
 	struct kfd_topology_device *dev;
 	uint32_t log_max_watch_addr;
 
@@ -422,7 +427,7 @@ static ssize_t node_show(struct kobject *kobj, struct attribute *attr,
 				attr_gpuid);
 		if (dev->gpu && kfd_devcgroup_check_permission(dev->gpu))
 			return -EPERM;
-		return sysfs_show_32bit_val(buffer, dev->gpu_id);
+		return sysfs_show_32bit_val(buffer, offs, dev->gpu_id);
 	}
 
 	if (strcmp(attr->name, "name") == 0) { @@ -431,69 +436,69 @@ static ssize_t node_show(struct kobject *kobj, struct attribute *attr,
 
 		if (dev->gpu && kfd_devcgroup_check_permission(dev->gpu))
 			return -EPERM;
-		return sysfs_show_str_val(buffer, dev->node_props.name);
+		return sysfs_show_str_val(buffer, offs, dev->node_props.name);
 	}
 
 	dev = container_of(attr, struct kfd_topology_device,
 			attr_props);
 	if (dev->gpu && kfd_devcgroup_check_permission(dev->gpu))
 		return -EPERM;
-	sysfs_show_32bit_prop(buffer, "cpu_cores_count",
-			dev->node_props.cpu_cores_count);
-	sysfs_show_32bit_prop(buffer, "simd_count",
-			dev->node_props.simd_count);
-	sysfs_show_32bit_prop(buffer, "mem_banks_count",
-			dev->node_props.mem_banks_count);
-	sysfs_show_32bit_prop(buffer, "caches_count",
-			dev->node_props.caches_count);
-	sysfs_show_32bit_prop(buffer, "io_links_count",
-			dev->node_props.io_links_count);
-	sysfs_show_32bit_prop(buffer, "cpu_core_id_base",
-			dev->node_props.cpu_core_id_base);
-	sysfs_show_32bit_prop(buffer, "simd_id_base",
-			dev->node_props.simd_id_base);
-	sysfs_show_32bit_prop(buffer, "max_waves_per_simd",
-			dev->node_props.max_waves_per_simd);
-	sysfs_show_32bit_prop(buffer, "lds_size_in_kb",
-			dev->node_props.lds_size_in_kb);
-	sysfs_show_32bit_prop(buffer, "gds_size_in_kb",
-			dev->node_props.gds_size_in_kb);
-	sysfs_show_32bit_prop(buffer, "num_gws",
-			dev->node_props.num_gws);
-	sysfs_show_32bit_prop(buffer, "wave_front_size",
-			dev->node_props.wave_front_size);
-	sysfs_show_32bit_prop(buffer, "array_count",
-			dev->node_props.array_count);
-	sysfs_show_32bit_prop(buffer, "simd_arrays_per_engine",
-			dev->node_props.simd_arrays_per_engine);
-	sysfs_show_32bit_prop(buffer, "cu_per_simd_array",
-			dev->node_props.cu_per_simd_array);
-	sysfs_show_32bit_prop(buffer, "simd_per_cu",
-			dev->node_props.simd_per_cu);
-	sysfs_show_32bit_prop(buffer, "max_slots_scratch_cu",
-			dev->node_props.max_slots_scratch_cu);
-	sysfs_show_32bit_prop(buffer, "vendor_id",
-			dev->node_props.vendor_id);
-	sysfs_show_32bit_prop(buffer, "device_id",
-			dev->node_props.device_id);
-	sysfs_show_32bit_prop(buffer, "location_id",
-			dev->node_props.location_id);
-	sysfs_show_32bit_prop(buffer, "domain",
-			dev->node_props.domain);
-	sysfs_show_32bit_prop(buffer, "drm_render_minor",
-			dev->node_props.drm_render_minor);
-	sysfs_show_64bit_prop(buffer, "hive_id",
-			dev->node_props.hive_id);
-	sysfs_show_32bit_prop(buffer, "num_sdma_engines",
-			dev->node_props.num_sdma_engines);
-	sysfs_show_32bit_prop(buffer, "num_sdma_xgmi_engines",
-			dev->node_props.num_sdma_xgmi_engines);
-	sysfs_show_32bit_prop(buffer, "num_sdma_queues_per_engine",
-			dev->node_props.num_sdma_queues_per_engine);
-	sysfs_show_32bit_prop(buffer, "num_cp_queues",
-			dev->node_props.num_cp_queues);
-	sysfs_show_64bit_prop(buffer, "unique_id",
-			dev->node_props.unique_id);
+	sysfs_show_32bit_prop(buffer, offs, "cpu_cores_count",
+			      dev->node_props.cpu_cores_count);
+	sysfs_show_32bit_prop(buffer, offs, "simd_count",
+			      dev->node_props.simd_count);
+	sysfs_show_32bit_prop(buffer, offs, "mem_banks_count",
+			      dev->node_props.mem_banks_count);
+	sysfs_show_32bit_prop(buffer, offs, "caches_count",
+			      dev->node_props.caches_count);
+	sysfs_show_32bit_prop(buffer, offs, "io_links_count",
+			      dev->node_props.io_links_count);
+	sysfs_show_32bit_prop(buffer, offs, "cpu_core_id_base",
+			      dev->node_props.cpu_core_id_base);
+	sysfs_show_32bit_prop(buffer, offs, "simd_id_base",
+			      dev->node_props.simd_id_base);
+	sysfs_show_32bit_prop(buffer, offs, "max_waves_per_simd",
+			      dev->node_props.max_waves_per_simd);
+	sysfs_show_32bit_prop(buffer, offs, "lds_size_in_kb",
+			      dev->node_props.lds_size_in_kb);
+	sysfs_show_32bit_prop(buffer, offs, "gds_size_in_kb",
+			      dev->node_props.gds_size_in_kb);
+	sysfs_show_32bit_prop(buffer, offs, "num_gws",
+			      dev->node_props.num_gws);
+	sysfs_show_32bit_prop(buffer, offs, "wave_front_size",
+			      dev->node_props.wave_front_size);
+	sysfs_show_32bit_prop(buffer, offs, "array_count",
+			      dev->node_props.array_count);
+	sysfs_show_32bit_prop(buffer, offs, "simd_arrays_per_engine",
+			      dev->node_props.simd_arrays_per_engine);
+	sysfs_show_32bit_prop(buffer, offs, "cu_per_simd_array",
+			      dev->node_props.cu_per_simd_array);
+	sysfs_show_32bit_prop(buffer, offs, "simd_per_cu",
+			      dev->node_props.simd_per_cu);
+	sysfs_show_32bit_prop(buffer, offs, "max_slots_scratch_cu",
+			      dev->node_props.max_slots_scratch_cu);
+	sysfs_show_32bit_prop(buffer, offs, "vendor_id",
+			      dev->node_props.vendor_id);
+	sysfs_show_32bit_prop(buffer, offs, "device_id",
+			      dev->node_props.device_id);
+	sysfs_show_32bit_prop(buffer, offs, "location_id",
+			      dev->node_props.location_id);
+	sysfs_show_32bit_prop(buffer, offs, "domain",
+			      dev->node_props.domain);
+	sysfs_show_32bit_prop(buffer, offs, "drm_render_minor",
+			      dev->node_props.drm_render_minor);
+	sysfs_show_64bit_prop(buffer, offs, "hive_id",
+			      dev->node_props.hive_id);
+	sysfs_show_32bit_prop(buffer, offs, "num_sdma_engines",
+			      dev->node_props.num_sdma_engines);
+	sysfs_show_32bit_prop(buffer, offs, "num_sdma_xgmi_engines",
+			      dev->node_props.num_sdma_xgmi_engines);
+	sysfs_show_32bit_prop(buffer, offs, "num_sdma_queues_per_engine",
+			      dev->node_props.num_sdma_queues_per_engine);
+	sysfs_show_32bit_prop(buffer, offs, "num_cp_queues",
+			      dev->node_props.num_cp_queues);
+	sysfs_show_64bit_prop(buffer, offs, "unique_id",
+			      dev->node_props.unique_id);
 
 	if (dev->gpu) {
 		log_max_watch_addr =
@@ -513,22 +518,21 @@ static ssize_t node_show(struct kobject *kobj, struct attribute *attr,
 			dev->node_props.capability |=
 					HSA_CAP_AQL_QUEUE_DOUBLE_MAP;
 
-		sysfs_show_32bit_prop(buffer, "max_engine_clk_fcompute",
+		sysfs_show_32bit_prop(buffer, offs, "max_engine_clk_fcompute",
 			dev->node_props.max_engine_clk_fcompute);
 
-		sysfs_show_64bit_prop(buffer, "local_mem_size",
-				(unsigned long long int) 0);
+		sysfs_show_64bit_prop(buffer, offs, "local_mem_size", 0ULL);
 
-		sysfs_show_32bit_prop(buffer, "fw_version",
-				dev->gpu->mec_fw_version);
-		sysfs_show_32bit_prop(buffer, "capability",
-				dev->node_props.capability);
-		sysfs_show_32bit_prop(buffer, "sdma_fw_version",
-				dev->gpu->sdma_fw_version);
+		sysfs_show_32bit_prop(buffer, offs, "fw_version",
+				      dev->gpu->mec_fw_version);
+		sysfs_show_32bit_prop(buffer, offs, "capability",
+				      dev->node_props.capability);
+		sysfs_show_32bit_prop(buffer, offs, "sdma_fw_version",
+				      dev->gpu->sdma_fw_version);
 	}
 
-	return sysfs_show_32bit_prop(buffer, "max_engine_clk_ccompute",
-					cpufreq_quick_get_max(0)/1000);
+	return sysfs_show_32bit_prop(buffer, offs, "max_engine_clk_ccompute",
+				     cpufreq_quick_get_max(0)/1000);
 }
 
 static const struct sysfs_ops node_ops = {
--
2.17.1


More information about the amd-gfx mailing list