drm/amdkfd: Fix GCC 10 compiler warning
authorFelix Kuehling <Felix.Kuehling@amd.com>
Fri, 22 May 2020 19:57:51 +0000 (15:57 -0400)
committerAlex Deucher <alexander.deucher@amd.com>
Thu, 28 May 2020 21:46:20 +0000 (17:46 -0400)
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@amd.com>
Acked-by: Aaron Liu <aaron.liu@amd.com>
Tested-by: Aaron Liu <aaron.liu@amd.com>
Reviewed-by: Amber Lin <Amber.Lin@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
drivers/gpu/drm/amd/amdkfd/kfd_topology.c

index bb77f7af2b6d9e3f37ce33d56614952021dbd01c..d5e2585d6f346b983f02b6d60781da9f47621160 100644 (file)
@@ -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 = {