[PATCH 1/2] drm/amdkfd: Use bitmap_zalloc() when applicable

[PATCH 1/2] drm/amdkfd: Use bitmap_zalloc() when applicable

[Christophe JAILLET]

'kfd->gtt_sa_bitmap' is a bitmap. So use 'bitmap_zalloc()' to simplify
code, improve the semantic and avoid some open-coded arithmetic in
allocator arguments.

Also change the corresponding 'kfree()' into 'bitmap_free()' to keep
consistency.
---
 drivers/gpu/drm/amd/amdkfd/kfd_device.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
index e1294fba0c26..c5a0ce44a295 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
@@ -1252,8 +1252,6 @@ int kgd2kfd_schedule_evict_and_restore_process(struct mm_struct *mm,
 static int kfd_gtt_sa_init(struct kfd_dev *kfd, unsigned int buf_size,
 				unsigned int chunk_size)
 {
-	unsigned int num_of_longs;
-
 	if (WARN_ON(buf_size < chunk_size))
 		return -EINVAL;
 	if (WARN_ON(buf_size == 0))
@@ -1264,11 +1262,8 @@ static int kfd_gtt_sa_init(struct kfd_dev *kfd, unsigned int buf_size,
 	kfd->gtt_sa_chunk_size = chunk_size;
 	kfd->gtt_sa_num_of_chunks = buf_size / chunk_size;
 
-	num_of_longs = (kfd->gtt_sa_num_of_chunks + BITS_PER_LONG - 1) /
-		BITS_PER_LONG;
-
-	kfd->gtt_sa_bitmap = kcalloc(num_of_longs, sizeof(long), GFP_KERNEL);
-
+	kfd->gtt_sa_bitmap = bitmap_zalloc(kfd->gtt_sa_num_of_chunks,
+					   GFP_KERNEL);
 	if (!kfd->gtt_sa_bitmap)
 		return -ENOMEM;
 
@@ -1278,13 +1273,12 @@ static int kfd_gtt_sa_init(struct kfd_dev *kfd, unsigned int buf_size,
 	mutex_init(&kfd->gtt_sa_lock);
 
 	return 0;
-
 }
 
 static void kfd_gtt_sa_fini(struct kfd_dev *kfd)
 {
 	mutex_destroy(&kfd->gtt_sa_lock);
-	kfree(kfd->gtt_sa_bitmap);
+	bitmap_free(kfd->gtt_sa_bitmap);
 }
 
 static inline uint64_t kfd_gtt_sa_calc_gpu_addr(uint64_t start_addr,
-- 
2.30.2

[PATCH 2/2] drm/amdkfd: Use non-atomic bitmap functions when possible

[Christophe JAILLET]

All uses of the 'kfd->gtt_sa_bitmap' bitmap are protected with the
'kfd->gtt_sa_lock' mutex.

So:
   - prefer the non-atomic '__set_bit()' function
   - use the non-atomic 'bitmap_[set|clear]()' functions instead of
     equivalent 'for' loops. These functions can work on several bits at a
     time

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
 drivers/gpu/drm/amd/amdkfd/kfd_device.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
index c5a0ce44a295..c4d868a5dd97 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
@@ -1346,7 +1346,7 @@ int kfd_gtt_sa_allocate(struct kfd_dev *kfd, unsigned int size,
 	/* If we need only one chunk, mark it as allocated and get out */
 	if (size <= kfd->gtt_sa_chunk_size) {
 		pr_debug("Single bit\n");
-		set_bit(found, kfd->gtt_sa_bitmap);
+		__set_bit(found, kfd->gtt_sa_bitmap);
 		goto kfd_gtt_out;
 	}
 
@@ -1384,10 +1384,8 @@ int kfd_gtt_sa_allocate(struct kfd_dev *kfd, unsigned int size,
 		(*mem_obj)->range_start, (*mem_obj)->range_end);
 
 	/* Mark the chunks as allocated */
-	for (found = (*mem_obj)->range_start;
-		found <= (*mem_obj)->range_end;
-		found++)
-		set_bit(found, kfd->gtt_sa_bitmap);
+	bitmap_set(kfd->gtt_sa_bitmap, (*mem_obj)->range_start,
+		   (*mem_obj)->range_end - (*mem_obj)->range_start + 1);
 
 kfd_gtt_out:
 	mutex_unlock(&kfd->gtt_sa_lock);
@@ -1402,8 +1400,6 @@ int kfd_gtt_sa_allocate(struct kfd_dev *kfd, unsigned int size,
 
 int kfd_gtt_sa_free(struct kfd_dev *kfd, struct kfd_mem_obj *mem_obj)
 {
-	unsigned int bit;
-
 	/* Act like kfree when trying to free a NULL object */
 	if (!mem_obj)
 		return 0;
@@ -1414,10 +1410,8 @@ int kfd_gtt_sa_free(struct kfd_dev *kfd, struct kfd_mem_obj *mem_obj)
 	mutex_lock(&kfd->gtt_sa_lock);
 
 	/* Mark the chunks as free */
-	for (bit = mem_obj->range_start;
-		bit <= mem_obj->range_end;
-		bit++)
-		clear_bit(bit, kfd->gtt_sa_bitmap);
+	bitmap_clear(kfd->gtt_sa_bitmap, mem_obj->range_start,
+		     mem_obj->range_end - mem_obj->range_start + 1);
 
 	mutex_unlock(&kfd->gtt_sa_lock);
 
-- 
2.30.2

Re: [PATCH 1/2] drm/amdkfd: Use bitmap_zalloc() when applicable

[Felix Kuehling]

Hi Christophe,

I just stumbled over this patch series while cleaning up my inbox. Sorry 
for dropping it back in November. I'm about to apply it but I noticed 
that patch 1 is missing a Signed-off-by. Is it OK to add that in your name?

Thanks,
   Felix


Am 2021-11-28 um 11:45 schrieb Christophe JAILLET:
> 'kfd->gtt_sa_bitmap' is a bitmap. So use 'bitmap_zalloc()' to simplify
> code, improve the semantic and avoid some open-coded arithmetic in
> allocator arguments.
>
> Also change the corresponding 'kfree()' into 'bitmap_free()' to keep
> consistency.
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_device.c | 12 +++---------
>   1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> index e1294fba0c26..c5a0ce44a295 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> @@ -1252,8 +1252,6 @@ int kgd2kfd_schedule_evict_and_restore_process(struct mm_struct *mm,
>   static int kfd_gtt_sa_init(struct kfd_dev *kfd, unsigned int buf_size,
>   				unsigned int chunk_size)
>   {
> -	unsigned int num_of_longs;
> -
>   	if (WARN_ON(buf_size < chunk_size))
>   		return -EINVAL;
>   	if (WARN_ON(buf_size == 0))
> @@ -1264,11 +1262,8 @@ static int kfd_gtt_sa_init(struct kfd_dev *kfd, unsigned int buf_size,
>   	kfd->gtt_sa_chunk_size = chunk_size;
>   	kfd->gtt_sa_num_of_chunks = buf_size / chunk_size;
>   
> -	num_of_longs = (kfd->gtt_sa_num_of_chunks + BITS_PER_LONG - 1) /
> -		BITS_PER_LONG;
> -
> -	kfd->gtt_sa_bitmap = kcalloc(num_of_longs, sizeof(long), GFP_KERNEL);
> -
> +	kfd->gtt_sa_bitmap = bitmap_zalloc(kfd->gtt_sa_num_of_chunks,
> +					   GFP_KERNEL);
>   	if (!kfd->gtt_sa_bitmap)
>   		return -ENOMEM;
>   
> @@ -1278,13 +1273,12 @@ static int kfd_gtt_sa_init(struct kfd_dev *kfd, unsigned int buf_size,
>   	mutex_init(&kfd->gtt_sa_lock);
>   
>   	return 0;
> -
>   }
>   
>   static void kfd_gtt_sa_fini(struct kfd_dev *kfd)
>   {
>   	mutex_destroy(&kfd->gtt_sa_lock);
> -	kfree(kfd->gtt_sa_bitmap);
> +	bitmap_free(kfd->gtt_sa_bitmap);
>   }
>   
>   static inline uint64_t kfd_gtt_sa_calc_gpu_addr(uint64_t start_addr,

Re: [PATCH 1/2] drm/amdkfd: Use bitmap_zalloc() when applicable

[Christophe JAILLET]

Le 26/04/2022 à 20:01, Felix Kuehling a écrit :
> Hi Christophe,
> 
> I just stumbled over this patch series while cleaning up my inbox. Sorry 
> for dropping it back in November. I'm about to apply it but I noticed 
> that patch 1 is missing a Signed-off-by. Is it OK to add that in your name?

Hi,

No problem for me if you can add it. Thanks.
But if you prefer a v2, it is also fine for me.

BTW sorry for missing the SoB tag. This definitively means that I forgot 
the checkpatch.pl step for this patch, which is bad.

CJ

> 
> Thanks,
>    Felix
> 
> 
> Am 2021-11-28 um 11:45 schrieb Christophe JAILLET:
>> 'kfd->gtt_sa_bitmap' is a bitmap. So use 'bitmap_zalloc()' to simplify
>> code, improve the semantic and avoid some open-coded arithmetic in
>> allocator arguments.
>>
>> Also change the corresponding 'kfree()' into 'bitmap_free()' to keep
>> consistency.
>> ---
>>   drivers/gpu/drm/amd/amdkfd/kfd_device.c | 12 +++---------
>>   1 file changed, 3 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c 
>> b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
>> index e1294fba0c26..c5a0ce44a295 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
>> @@ -1252,8 +1252,6 @@ int 
>> kgd2kfd_schedule_evict_and_restore_process(struct mm_struct *mm,
>>   static int kfd_gtt_sa_init(struct kfd_dev *kfd, unsigned int buf_size,
>>                   unsigned int chunk_size)
>>   {
>> -    unsigned int num_of_longs;
>> -
>>       if (WARN_ON(buf_size < chunk_size))
>>           return -EINVAL;
>>       if (WARN_ON(buf_size == 0))
>> @@ -1264,11 +1262,8 @@ static int kfd_gtt_sa_init(struct kfd_dev *kfd, 
>> unsigned int buf_size,
>>       kfd->gtt_sa_chunk_size = chunk_size;
>>       kfd->gtt_sa_num_of_chunks = buf_size / chunk_size;
>> -    num_of_longs = (kfd->gtt_sa_num_of_chunks + BITS_PER_LONG - 1) /
>> -        BITS_PER_LONG;
>> -
>> -    kfd->gtt_sa_bitmap = kcalloc(num_of_longs, sizeof(long), 
>> GFP_KERNEL);
>> -
>> +    kfd->gtt_sa_bitmap = bitmap_zalloc(kfd->gtt_sa_num_of_chunks,
>> +                       GFP_KERNEL);
>>       if (!kfd->gtt_sa_bitmap)
>>           return -ENOMEM;
>> @@ -1278,13 +1273,12 @@ static int kfd_gtt_sa_init(struct kfd_dev 
>> *kfd, unsigned int buf_size,
>>       mutex_init(&kfd->gtt_sa_lock);
>>       return 0;
>> -
>>   }
>>   static void kfd_gtt_sa_fini(struct kfd_dev *kfd)
>>   {
>>       mutex_destroy(&kfd->gtt_sa_lock);
>> -    kfree(kfd->gtt_sa_bitmap);
>> +    bitmap_free(kfd->gtt_sa_bitmap);
>>   }
>>   static inline uint64_t kfd_gtt_sa_calc_gpu_addr(uint64_t start_addr,
> 

Re: [PATCH 1/2] drm/amdkfd: Use bitmap_zalloc() when applicable

[Felix Kuehling]

Am 2022-04-26 um 14:47 schrieb Christophe JAILLET:
> Le 26/04/2022 à 20:01, Felix Kuehling a écrit :
>> Hi Christophe,
>>
>> I just stumbled over this patch series while cleaning up my inbox. 
>> Sorry for dropping it back in November. I'm about to apply it but I 
>> noticed that patch 1 is missing a Signed-off-by. Is it OK to add that 
>> in your name?
>
> Hi,
>
> No problem for me if you can add it. Thanks.
> But if you prefer a v2, it is also fine for me.

No need. I submitted the patches to amd-staging-drm-next.

Regards,
   Felix


>
> BTW sorry for missing the SoB tag. This definitively means that I 
> forgot the checkpatch.pl step for this patch, which is bad.
>
> CJ
>
>>
>> Thanks,
>>    Felix
>>
>>
>> Am 2021-11-28 um 11:45 schrieb Christophe JAILLET:
>>> 'kfd->gtt_sa_bitmap' is a bitmap. So use 'bitmap_zalloc()' to simplify
>>> code, improve the semantic and avoid some open-coded arithmetic in
>>> allocator arguments.
>>>
>>> Also change the corresponding 'kfree()' into 'bitmap_free()' to keep
>>> consistency.
>>> ---
>>>   drivers/gpu/drm/amd/amdkfd/kfd_device.c | 12 +++---------
>>>   1 file changed, 3 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c 
>>> b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
>>> index e1294fba0c26..c5a0ce44a295 100644
>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
>>> @@ -1252,8 +1252,6 @@ int 
>>> kgd2kfd_schedule_evict_and_restore_process(struct mm_struct *mm,
>>>   static int kfd_gtt_sa_init(struct kfd_dev *kfd, unsigned int 
>>> buf_size,
>>>                   unsigned int chunk_size)
>>>   {
>>> -    unsigned int num_of_longs;
>>> -
>>>       if (WARN_ON(buf_size < chunk_size))
>>>           return -EINVAL;
>>>       if (WARN_ON(buf_size == 0))
>>> @@ -1264,11 +1262,8 @@ static int kfd_gtt_sa_init(struct kfd_dev 
>>> *kfd, unsigned int buf_size,
>>>       kfd->gtt_sa_chunk_size = chunk_size;
>>>       kfd->gtt_sa_num_of_chunks = buf_size / chunk_size;
>>> -    num_of_longs = (kfd->gtt_sa_num_of_chunks + BITS_PER_LONG - 1) /
>>> -        BITS_PER_LONG;
>>> -
>>> -    kfd->gtt_sa_bitmap = kcalloc(num_of_longs, sizeof(long), 
>>> GFP_KERNEL);
>>> -
>>> +    kfd->gtt_sa_bitmap = bitmap_zalloc(kfd->gtt_sa_num_of_chunks,
>>> +                       GFP_KERNEL);
>>>       if (!kfd->gtt_sa_bitmap)
>>>           return -ENOMEM;
>>> @@ -1278,13 +1273,12 @@ static int kfd_gtt_sa_init(struct kfd_dev 
>>> *kfd, unsigned int buf_size,
>>>       mutex_init(&kfd->gtt_sa_lock);
>>>       return 0;
>>> -
>>>   }
>>>   static void kfd_gtt_sa_fini(struct kfd_dev *kfd)
>>>   {
>>>       mutex_destroy(&kfd->gtt_sa_lock);
>>> -    kfree(kfd->gtt_sa_bitmap);
>>> +    bitmap_free(kfd->gtt_sa_bitmap);
>>>   }
>>>   static inline uint64_t kfd_gtt_sa_calc_gpu_addr(uint64_t start_addr,
>>
>

[PATCH 1/2] drm/amdkfd: Use bitmap_zalloc() when applicable

[Christophe JAILLET]

'doorbell_bitmap' is a bitmap. So use 'bitmap_zalloc()' to simplify code,
improve the semantic and avoid some open-coded arithmetic in allocator
arguments.

Also change the corresponding 'kfree()' into 'bitmap_free()' to keep
consistency.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
 drivers/gpu/drm/amd/amdkfd/kfd_process.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index f29b3932e3dc..172ee8763523 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -1011,7 +1011,7 @@ static void kfd_process_destroy_pdds(struct kfd_process *p)
 			free_pages((unsigned long)pdd->qpd.cwsr_kaddr,
 				get_order(KFD_CWSR_TBA_TMA_SIZE));
 
-		kfree(pdd->qpd.doorbell_bitmap);
+		bitmap_free(pdd->qpd.doorbell_bitmap);
 		idr_destroy(&pdd->alloc_idr);
 
 		kfd_free_process_doorbells(pdd->dev, pdd->doorbell_index);
@@ -1434,9 +1434,8 @@ static int init_doorbell_bitmap(struct qcm_process_device *qpd,
 	if (!KFD_IS_SOC15(dev->device_info->asic_family))
 		return 0;
 
-	qpd->doorbell_bitmap =
-		kzalloc(DIV_ROUND_UP(KFD_MAX_NUM_OF_QUEUES_PER_PROCESS,
-				     BITS_PER_BYTE), GFP_KERNEL);
+	qpd->doorbell_bitmap = bitmap_zalloc(KFD_MAX_NUM_OF_QUEUES_PER_PROCESS
+					     GFP_KERNEL);
 	if (!qpd->doorbell_bitmap)
 		return -ENOMEM;
 
-- 
2.30.2