Am 2022-06-28 um 19:25 schrieb Ramesh Errabolu:
The patch fixes couple of warnings, as reported by Smatch
a static analyzer
Signed-off-by: Ramesh Errabolu <ramesh.errab...@amd.com>
Reported-by: Dan Carpenter <dan.carpen...@oracle.com>
---
drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 36 ++++++++++++-----------
1 file changed, 19 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
index 25990bec600d..9d7b9ad70bc8 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
@@ -1417,15 +1417,17 @@ static int kfd_create_indirect_link_prop(struct
kfd_topology_device *kdev, int g
/* find CPU <--> CPU links */
cpu_dev = kfd_topology_device_by_proximity_domain(i);
- if (cpu_dev) {
- list_for_each_entry(cpu_link,
- &cpu_dev->io_link_props, list) {
- if (cpu_link->node_to == gpu_link->node_to)
- break;
- }
- }
+ if (!cpu_dev)
+ continue;
+
+ cpu_link = NULL;
This initialization is unnecessary. list_for_each_entry will always
initialize it.
+ list_for_each_entry(cpu_link, &cpu_dev->io_link_props, list)
+ if (cpu_link->node_to == gpu_link->node_to)
+ break;
- if (cpu_link->node_to != gpu_link->node_to)
+ /* Ensures we didn't exit from list search with no hits */
+ if (list_entry_is_head(cpu_link, &cpu_dev->io_link_props, list)
||
+ (cpu_link->node_to != gpu_link->node_to))
The second condition is redundant. If the list entry is not the head,
the node_to must have already matched in the loop.
But I'm no sure this solution is going to satisfy the static checker. It
objects to using the iterator (cpu_link) outside the loop. I think a
proper solution, that doesn't make any assumptions about how
list_for_each_entry is implemented, would be to declare a separate
variable as the iterator, and assign cpu_link in the loop only if there
is a match.
Regards,
Felix
return -ENOMEM;
/* CPU <--> CPU <--> GPU, GPU node*/
@@ -1510,16 +1512,16 @@ static int kfd_add_peer_prop(struct kfd_topology_device
*kdev,
cpu_dev =
kfd_topology_device_by_proximity_domain(iolink1->node_to);
if (cpu_dev) {
list_for_each_entry(iolink3, &cpu_dev->io_link_props,
list)
- if (iolink3->node_to == iolink2->node_to)
+ if (iolink3->node_to == iolink2->node_to) {
+ props->weight += iolink3->weight;
+ props->min_latency +=
iolink3->min_latency;
+ props->max_latency +=
iolink3->max_latency;
+ props->min_bandwidth =
min(props->min_bandwidth,
+
iolink3->min_bandwidth);
+ props->max_bandwidth =
min(props->max_bandwidth,
+
iolink3->max_bandwidth);
break;
-
- props->weight += iolink3->weight;
- props->min_latency += iolink3->min_latency;
- props->max_latency += iolink3->max_latency;
- props->min_bandwidth = min(props->min_bandwidth,
- iolink3->min_bandwidth);
- props->max_bandwidth = min(props->max_bandwidth,
- iolink3->max_bandwidth);
+ }
} else {
WARN(1, "CPU node not found");
}
