Hi Ramesh,
On 6/29/22 14:04, Ramesh Errabolu wrote:
> The patch fixes couple of warnings, as reported by Smatch
> a static analyzer.
>
> Fixes: 40d6aa758b13 ("drm/amdkfd: Extend KFD device topology to surface peer-to-peer links")>
> drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:1428 kfd_create_indirect_link_prop() warn: iterator used outside loop: 'cpu_link'
> drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:1462 kfd_create_indirect_link_prop() error: we previously assumed 'cpu_dev' could be null (see line 1420)
> drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:1516 kfd_add_peer_prop() warn: iterator used outside loop: 'iolink3'
>
Usually, the Fixes tag would go here, after the commit message.
> Signed-off-by: Ramesh Errabolu <Ramesh.Errabolu@xxxxxxx>
> Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> ---
As this is a v2 PATCH, it would be nice to have a small changelog here,
describing what has changed between the v1 and v2 versions of the patch.
Also, you can mark the patch as v2 with git send-email by adding the
flag -v2. More on the canonical patch format can be seen in [1].
[1]
https://docs.kernel.org/process/submitting-patches.html#the-canonical-patch-format
Best Regards,
- Maíra Canal
> drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 34 +++++++++++------------
> 1 file changed, 17 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..ca4825e555b7 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> @@ -1417,15 +1417,15 @@ 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;
> +
> + 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))
> return -ENOMEM;
>
> /* CPU <--> CPU <--> GPU, GPU node*/
> @@ -1510,16 +1510,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");
> }
