[AMD Official Use Only - AMD Internal Distribution Only] With the update to commit message, some thing small and precise like Christian recommended sounds good. With that Reviewed-by: Sunil Khatri <[email protected]>
-----Original Message----- From: Koenig, Christian <[email protected]> Sent: Friday, May 2, 2025 1:35 PM To: Yadav, Arvind <[email protected]>; Deucher, Alexander <[email protected]>; Khatri, Sunil <[email protected]>; Paneer Selvam, Arunpravin <[email protected]> Cc: [email protected] Subject: Re: [PATCH v4 2/2] drm/amdgpu: only keep most recent fence for each context On 4/30/25 18:05, Arvind Yadav wrote: > Mesa passes shared bo, fence syncobj to userq_ioctl. > There can be duplicates here or some fences that are old. > This patch is remove duplicates fence and only keep the most recent > fence for each context. > > v2: - Export this code from dma-fence-unwrap.c(by Christian). > v3: - To split this in a dma_buf patch and amd userq patch(by Sunil). > - No need to add a new function just re-use existing(by Christian). > v4: Export dma_fence_dedub_array function and used it(by Christian). > > Cc: Alex Deucher <[email protected]> > Cc: Christian Koenig <[email protected]> > Cc: Sunil Khatri <[email protected]> > Cc: Arunpravin Paneer Selvam <[email protected]> > Signed-off-by: Arvind Yadav <[email protected]> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c > index 3288c2ff692e..e3e4aeee1356 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c > @@ -851,6 +851,12 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void > *data, > fences[num_fences++] = fence; > } > > + /* > + * Remove duplicates fence and only keep the most recent fence > for > + * each context. > + */ Drop or at least change the comment. You should never document what the code does, that should be obvious by reading the code. Only document why the code does something, e.g. in this case here something like "Keep only the latest fences to reduce the number of values given back to userspace" would be appropriate. With that done the patch is Reviewed-by: Christian König <[email protected]>. Regards, Christian. > + num_fences = dma_fence_dedup_array(fences, num_fences); > + > waitq = idr_find(&userq_mgr->userq_idr, wait_info->waitq_id); > if (!waitq) > goto free_fences;
