linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/v3d: Use kvcalloc
@ 2022-03-12 15:26 Harshit Mogalapalli
  2022-03-12 22:27 ` Joe Perches
  2022-03-28 12:25 ` Melissa Wen
  0 siblings, 2 replies; 5+ messages in thread
From: Harshit Mogalapalli @ 2022-03-12 15:26 UTC (permalink / raw)
  Cc: harshit.m.mogalapalli, dan.carpenter, Emma Anholt, David Airlie,
	Daniel Vetter, dri-devel, linux-kernel

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);
 	if (!job->bo) {
 		DRM_DEBUG("Failed to allocate validated BO pointers\n");
 		return -ENOMEM;
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/v3d: Use kvcalloc
  2022-03-12 15:26 [PATCH] drm/v3d: Use kvcalloc Harshit Mogalapalli
@ 2022-03-12 22:27 ` Joe Perches
  2022-03-13  6:05   ` Harshit Mogalapalli
  2022-03-28 12:25 ` Melissa Wen
  1 sibling, 1 reply; 5+ messages in thread
From: Joe Perches @ 2022-03-12 22:27 UTC (permalink / raw)
  To: Harshit Mogalapalli
  Cc: dan.carpenter, Emma Anholt, David Airlie, Daniel Vetter,
	dri-devel, linux-kernel

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.



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/v3d: Use kvcalloc
  2022-03-12 22:27 ` Joe Perches
@ 2022-03-13  6:05   ` Harshit Mogalapalli
  0 siblings, 0 replies; 5+ messages in thread
From: Harshit Mogalapalli @ 2022-03-13  6:05 UTC (permalink / raw)
  To: Joe Perches
  Cc: dan.carpenter, Emma Anholt, David Airlie, Daniel Vetter,
	dri-devel, linux-kernel

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/v3d: Use kvcalloc
  2022-03-12 15:26 [PATCH] drm/v3d: Use kvcalloc Harshit Mogalapalli
  2022-03-12 22:27 ` Joe Perches
@ 2022-03-28 12:25 ` Melissa Wen
  2022-03-29 12:01   ` Harshit Mogalapalli
  1 sibling, 1 reply; 5+ messages in thread
From: Melissa Wen @ 2022-03-28 12:25 UTC (permalink / raw)
  To: Harshit Mogalapalli
  Cc: Emma Anholt, David Airlie, linux-kernel, dri-devel, dan.carpenter

[-- Attachment #1: Type: text/plain, Size: 1423 bytes --]

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
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/v3d: Use kvcalloc
  2022-03-28 12:25 ` Melissa Wen
@ 2022-03-29 12:01   ` Harshit Mogalapalli
  0 siblings, 0 replies; 5+ messages in thread
From: Harshit Mogalapalli @ 2022-03-29 12:01 UTC (permalink / raw)
  To: Melissa Wen
  Cc: Emma Anholt, David Airlie, linux-kernel, dri-devel,
	dan.carpenter, Melissa Wen

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
>>


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-03-29 12:02 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-12 15:26 [PATCH] drm/v3d: Use kvcalloc Harshit Mogalapalli
2022-03-12 22:27 ` Joe Perches
2022-03-13  6:05   ` Harshit Mogalapalli
2022-03-28 12:25 ` Melissa Wen
2022-03-29 12:01   ` Harshit Mogalapalli

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).