'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
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 <[email protected]>
---
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
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,
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,
>>
>
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,
>