| Message ID | 20220312152656.51625-1-harshit.m.mogalapalli@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/v3d: Use kvcalloc | expand |
Hi Joe, On 13/03/22 3:57 am, Joe Perches wrote: > On Sat, 2022-03-12 at 07:26 -0800, Harshit Mogalapalli wrote: >> kvcalloc is same as kvmalloc_array + __GFP_ZERO. > [] >> diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c > [] >> @@ -308,9 +308,8 @@ v3d_lookup_bos(struct drm_device *dev, >> return -EINVAL; >> } >> >> - job->bo = kvmalloc_array(job->bo_count, >> - sizeof(struct drm_gem_cma_object *), >> - GFP_KERNEL | __GFP_ZERO); >> + job->bo = kvcalloc(job->bo_count, sizeof(struct drm_gem_cma_object *), >> + GFP_KERNEL); >> if (!job->bo) { >> DRM_DEBUG("Failed to allocate validated BO pointers\n"); >> return -ENOMEM; > > trivia: > > The DRM_DEBUG could also be removed as the the alloc will do a > a dump_stack on failure. > > Thanks for sharing your comments. DRM_DEBUG statements are present in other parts of code as well. So didnot remove it. Example below: drivers/gpu/drm/v3d/v3d_gem.c at 322. 311 job->bo = kvmalloc_array(job->bo_count, 312 sizeof(struct drm_gem_cma_object *), 313 GFP_KERNEL | __GFP_ZERO); 314 if (!job->bo) { 315 DRM_DEBUG("Failed to allocate validated BO pointers\n"); 316 return -ENOMEM; 317 } 318 319 handles = kvmalloc_array(job->bo_count, sizeof(u32), GFP_KERNEL); 320 if (!handles) { 321 ret = -ENOMEM; 322 DRM_DEBUG("Failed to allocate incoming GEM handles\n"); 323 goto fail; 324 } Regards, Harshit
On 03/12, Harshit Mogalapalli wrote: > kvcalloc is same as kvmalloc_array + __GFP_ZERO. > > Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com> > --- > drivers/gpu/drm/v3d/v3d_gem.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c > index c7ed2e1cbab6..f7d37228461e 100644 > --- a/drivers/gpu/drm/v3d/v3d_gem.c > +++ b/drivers/gpu/drm/v3d/v3d_gem.c > @@ -308,9 +308,8 @@ v3d_lookup_bos(struct drm_device *dev, > return -EINVAL; > } > > - job->bo = kvmalloc_array(job->bo_count, > - sizeof(struct drm_gem_cma_object *), > - GFP_KERNEL | __GFP_ZERO); > + job->bo = kvcalloc(job->bo_count, sizeof(struct drm_gem_cma_object *), > + GFP_KERNEL); Hi Harshit, This change seems valid to me, but I believe, in this point, v3d should move to use the DRM function `drm_gem_objects_lookup()`, and then your change goes there, since drm_get_objects_lookup() has the same issue you're pointing. What do you think? I already sent a patchset to replace steps in v3d_lookup_bos() by drm_gem_objects_lookup(), as I mentioned. The patchset is here: https://patchwork.freedesktop.org/series/101610/ Willing to review it? ^ Thanks, Melissa > if (!job->bo) { > DRM_DEBUG("Failed to allocate validated BO pointers\n"); > return -ENOMEM; > -- > 2.31.1 >
On 28/03/22 5:55 pm, Melissa Wen wrote: > On 03/12, Harshit Mogalapalli wrote: >> kvcalloc is same as kvmalloc_array + __GFP_ZERO. >> >> Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com> >> --- >> drivers/gpu/drm/v3d/v3d_gem.c | 5 ++--- >> 1 file changed, 2 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c >> index c7ed2e1cbab6..f7d37228461e 100644 >> --- a/drivers/gpu/drm/v3d/v3d_gem.c >> +++ b/drivers/gpu/drm/v3d/v3d_gem.c >> @@ -308,9 +308,8 @@ v3d_lookup_bos(struct drm_device *dev, >> return -EINVAL; >> } >> >> - job->bo = kvmalloc_array(job->bo_count, >> - sizeof(struct drm_gem_cma_object *), >> - GFP_KERNEL | __GFP_ZERO); >> + job->bo = kvcalloc(job->bo_count, sizeof(struct drm_gem_cma_object *), >> + GFP_KERNEL); > > Hi Harshit, > Hi Melissa, > This change seems valid to me, but I believe, in this point, v3d should > move to use the DRM function `drm_gem_objects_lookup()`, and then your > change goes there, since drm_get_objects_lookup() has the same issue > you're pointing. What do you think? > Thanks for looking at the patch. Yes, you are right, the issue is same there as well. Few other similar instances in drm/ subsystem. drivers/gpu/drm/drm_gem.c:700 drm_gem_objects_lookup() warn: Please consider using kvcalloc instead drivers/gpu/drm/ttm/ttm_tt.c:99 ttm_tt_alloc_page_directory() warn: Please consider using kvcalloc instead drivers/gpu/drm/ttm/ttm_tt.c:108 ttm_dma_tt_alloc_page_directory() warn: Please consider using kvcalloc instead drivers/gpu/drm/ttm/ttm_tt.c:121 ttm_sg_tt_alloc_page_directory() warn: Please consider using kvcalloc instead drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c:541 amdgpu_cs_parser_bos() warn: Please consider using kvcalloc instead drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_svm.c:152 svm_range_dma_map_dev() warn: Please consider using kvcalloc instead drivers/gpu/drm/v3d/v3d_gem.c:311 v3d_lookup_bos() warn: Please consider using kvcalloc instead drivers/gpu/drm/vc4/vc4_gem.c:746 vc4_cl_lookup_bos() warn: Please consider using kvcalloc instead drivers/gpu/drm/lima/lima_gem.c:42 lima_heap_alloc() warn: Please consider using kvcalloc instead drivers/gpu/drm/panfrost/panfrost_drv.c:147 panfrost_lookup_bos() warn: Please consider using kvcalloc instead drivers/gpu/drm/panfrost/panfrost_mmu.c:452 panfrost_mmu_map_fault_addr() warn: Please consider using kvcalloc instead drivers/gpu/drm/panfrost/panfrost_mmu.c:460 panfrost_mmu_map_fault_addr() warn: Please consider using kvcalloc instead Tool Used: Smatch. > I already sent a patchset to replace steps in v3d_lookup_bos() by > drm_gem_objects_lookup(), as I mentioned. The patchset is here: > https://patchwork.freedesktop.org/series/101610/ > Willing to review it? ^ > Sorry Melissa, I am still a beginner, Can't review it. Regards, Harshit > Thanks, > > Melissa > >> if (!job->bo) { >> DRM_DEBUG("Failed to allocate validated BO pointers\n"); >> return -ENOMEM; >> -- >> 2.31.1 >>
diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c index c7ed2e1cbab6..f7d37228461e 100644 --- a/drivers/gpu/drm/v3d/v3d_gem.c +++ b/drivers/gpu/drm/v3d/v3d_gem.c @@ -308,9 +308,8 @@ v3d_lookup_bos(struct drm_device *dev, return -EINVAL; } - job->bo = kvmalloc_array(job->bo_count, - sizeof(struct drm_gem_cma_object *), - GFP_KERNEL | __GFP_ZERO); + job->bo = kvcalloc(job->bo_count, sizeof(struct drm_gem_cma_object *), + GFP_KERNEL); if (!job->bo) { DRM_DEBUG("Failed to allocate validated BO pointers\n"); return -ENOMEM;
kvcalloc is same as kvmalloc_array + __GFP_ZERO. Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com> --- drivers/gpu/drm/v3d/v3d_gem.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)