[AMD Public Use] Reviewed-by: Amber Lin <Amber.Lin@xxxxxxx> -----Original Message----- From: Kuehling, Felix <Felix.Kuehling@xxxxxxx> Sent: Monday, May 25, 2020 10:47 PM To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx Cc: Lin, Amber <Amber.Lin@xxxxxxx> 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@xxxxxxx> Acked-by: Aaron Liu <aaron.liu@xxxxxxx> Tested-by: Aaron Liu <aaron.liu@xxxxxxx> --- 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 _______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx