On 7/5/22 15:43, Errabolu, Ramesh wrote: > [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. >
Hi Ramesh, The canonical patch format is basically, as described by [1]: <commit message> ... Signed-off-by: Author <author@mail> --- V2 -> V3: Removed redundant helper function V1 -> V2: Cleaned up coding style and addressed review comments path/to/file | 5+++-- ... So, on your case, we have the commit message describing the warning reported by Smatch and the error log, then we got the tags. The tags should be in chronological order, so your tags should be: Fixes: 40d6aa758b13 ("drm/amdkfd: Extend KFD device topology to surface peer-to-peer links") Reported-by: Dan Carpenter <dan.carpen...@oracle.com> Signed-off-by: Ramesh Errabolu <ramesh.errab...@amd.com> See that this canonical format reflects the chronological history of the patch insofar as possible. After the ---, you describe the changes between v1 and v2 in a small changelog. The documentation linked in [1] explains this in details. So, some examples are exposed in [2], [3] and [4]. [1] https://docs.kernel.org/process/submitting-patches.html [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=1db88c5343712e411a2dd45375f27c477e33dc07 [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=34ad61514c4c3657df21a058f9961c3bb2f84ff2 [4] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d3f2a14b8906df913cb04a706367b012db94a6e8 Best Regards, - Maíra Canal > 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"); >> }