[AMD Official Use Only - General] Maira,
Thanks for taking time to review. I understand the request to tag the patch as version 2. However I don't follow your comments on "Fixes" block. Looking at the git log of the branch I see many "Fixes" block that precede the "Signed-off-by" statement. Could you provide an example. Regards, Ramesh -----Original Message----- From: Maíra Canal <mairaca...@riseup.net> Sent: Wednesday, June 29, 2022 6:25 PM To: Errabolu, Ramesh <ramesh.errab...@amd.com>; amd-gfx@lists.freedesktop.org; dan.carpen...@oracle.com Subject: Re: [PATCH] drm/amdkfd: Fix warnings from static analyzer Smatch [CAUTION: External Email] 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.errab...@amd.com> > Reported-by: Dan Carpenter <dan.carpen...@oracle.com> > --- 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://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdocs.kernel.org%2Fprocess%2Fsubmitting-patches.html%23the-canonical-patch-format&data=05%7C01%7CRamesh.Errabolu%40amd.com%7Cc54753a9471843cc9d1f08da5a26898d%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637921418813227961%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=PAc5A8z2EvJAOUiY378K9XyVBCKewQNsSNCr9pB3Ias%3D&reserved=0 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"); > }