On 2018å¹´01æ??12æ?¥ 16:04, Christian König wrote:
> Am 12.01.2018 um 06:45 schrieb Chunming Zhou:
>> Change-Id: I7ca19a1f6404356a6c69ab4af27c8e13454f0279
>> Signed-off-by: Chunming Zhou <david1.zhou at amd.com>
>> ---
>> Â amdgpu/amdgpu_internal.h |Â Â 2 -
>>  amdgpu/amdgpu_vamgr.c   | 152
>> ++++++++++++++---------------------------------
>> Â 2 files changed, 46 insertions(+), 108 deletions(-)
>>
>> diff --git a/amdgpu/amdgpu_internal.h b/amdgpu/amdgpu_internal.h
>> index fd522f39..7484780b 100644
>> --- a/amdgpu/amdgpu_internal.h
>> +++ b/amdgpu/amdgpu_internal.h
>> @@ -53,8 +53,6 @@ struct amdgpu_bo_va_hole {
>> Â };
>> Â Â struct amdgpu_bo_va_mgr {
>> -Â Â Â /* the start virtual address */
>> -Â Â Â uint64_t va_offset;
>> Â Â Â Â Â uint64_t va_min;
>> Â Â Â Â Â uint64_t va_max;
>
> Is va_min and va_max actually still used after that series?
Yes, still used for svm manager.
> If not I would remove them as well.
>
> Apart from that I would indeed add the warning when the calloc during
> free fails.
>
> With that fixed the series is Reviewed-by: Christian König
> <christian.koenig at amd.com>.
Thanks for review.
Regards,
David Zhou
>
> Regards,
> Christian.
>
>> Â Â Â Â Â struct list_head va_holes;
>> diff --git a/amdgpu/amdgpu_vamgr.c b/amdgpu/amdgpu_vamgr.c
>> index 66bb8ecd..b826ac81 100644
>> --- a/amdgpu/amdgpu_vamgr.c
>> +++ b/amdgpu/amdgpu_vamgr.c
>> @@ -61,13 +61,20 @@ int amdgpu_va_range_query(amdgpu_device_handle dev,
>> Â drm_private void amdgpu_vamgr_init(struct amdgpu_bo_va_mgr *mgr,
>> uint64_t start,
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â uint64_t max, uint64_t alignment)
>> Â {
>> -Â Â Â mgr->va_offset = start;
>> +Â Â Â struct amdgpu_bo_va_hole *n;
>> +
>> Â Â Â Â Â mgr->va_min = start;
>> Â Â Â Â Â mgr->va_max = max;
>> Â Â Â Â Â mgr->va_alignment = alignment;
>> Â Â Â Â Â Â list_inithead(&mgr->va_holes);
>> Â Â Â Â Â pthread_mutex_init(&mgr->bo_va_mutex, NULL);
>> +Â Â Â pthread_mutex_lock(&mgr->bo_va_mutex);
>> +Â Â Â n = calloc(1, sizeof(struct amdgpu_bo_va_hole));
>> +Â Â Â n->size = mgr->va_max;
>> +Â Â Â n->offset = mgr->va_min;
>> +Â Â Â list_add(&n->list, &mgr->va_holes);
>> +Â Â Â pthread_mutex_unlock(&mgr->bo_va_mutex);
>> Â }
>> Â Â drm_private void amdgpu_vamgr_deinit(struct amdgpu_bo_va_mgr *mgr)
>> @@ -136,35 +143,8 @@ amdgpu_vamgr_find_va(struct amdgpu_bo_va_mgr
>> *mgr, uint64_t size,
>> Â Â Â Â Â Â Â Â Â }
>> Â Â Â Â Â }
>> Â -Â Â Â if (base_required) {
>> -Â Â Â Â Â Â Â if (base_required < mgr->va_offset) {
>> -Â Â Â Â Â Â Â Â Â Â Â pthread_mutex_unlock(&mgr->bo_va_mutex);
>> -Â Â Â Â Â Â Â Â Â Â Â return AMDGPU_INVALID_VA_ADDRESS;
>> -Â Â Â Â Â Â Â }
>> -Â Â Â Â Â Â Â offset = mgr->va_offset;
>> -Â Â Â Â Â Â Â waste = base_required - mgr->va_offset;
>> -Â Â Â } else {
>> -Â Â Â Â Â Â Â offset = mgr->va_offset;
>> -Â Â Â Â Â Â Â waste = offset % alignment;
>> -Â Â Â Â Â Â Â waste = waste ? alignment - waste : 0;
>> -Â Â Â }
>> -
>> -Â Â Â if (offset + waste + size > mgr->va_max) {
>> -Â Â Â Â Â Â Â pthread_mutex_unlock(&mgr->bo_va_mutex);
>> -Â Â Â Â Â Â Â return AMDGPU_INVALID_VA_ADDRESS;
>> -Â Â Â }
>> -
>> -Â Â Â if (waste) {
>> -Â Â Â Â Â Â Â n = calloc(1, sizeof(struct amdgpu_bo_va_hole));
>> -Â Â Â Â Â Â Â n->size = waste;
>> -Â Â Â Â Â Â Â n->offset = offset;
>> -Â Â Â Â Â Â Â list_add(&n->list, &mgr->va_holes);
>> -Â Â Â }
>> -
>> -Â Â Â offset += waste;
>> -Â Â Â mgr->va_offset += size + waste;
>> Â Â Â Â Â pthread_mutex_unlock(&mgr->bo_va_mutex);
>> -Â Â Â return offset;
>> +Â Â Â return AMDGPU_INVALID_VA_ADDRESS;
>> Â }
>> Â Â static uint64_t amdgpu_vamgr_find_va_in_range(struct
>> amdgpu_bo_va_mgr *mgr, uint64_t size,
>> @@ -232,41 +212,14 @@ static uint64_t
>> amdgpu_vamgr_find_va_in_range(struct amdgpu_bo_va_mgr *mgr, uint
>> Â Â Â Â Â Â Â Â Â }
>> Â Â Â Â Â }
>> Â -Â Â Â if (mgr->va_offset > range_max) {
>> -Â Â Â Â Â Â Â pthread_mutex_unlock(&mgr->bo_va_mutex);
>> -Â Â Â Â Â Â Â return AMDGPU_INVALID_VA_ADDRESS;
>> -Â Â Â } else if (mgr->va_offset > range_min) {
>> -Â Â Â Â Â Â Â offset = mgr->va_offset;
>> -Â Â Â Â Â Â Â waste = offset % alignment;
>> -Â Â Â Â Â Â Â waste = waste ? alignment - waste : 0;
>> -Â Â Â Â Â Â Â if (offset + waste + size > range_max) {
>> -Â Â Â Â Â Â Â Â Â Â Â pthread_mutex_unlock(&mgr->bo_va_mutex);
>> -Â Â Â Â Â Â Â Â Â Â Â return AMDGPU_INVALID_VA_ADDRESS;
>> -Â Â Â Â Â Â Â }
>> -Â Â Â } else {
>> -Â Â Â Â Â Â Â offset = mgr->va_offset;
>> -Â Â Â Â Â Â Â waste = range_min % alignment;
>> -Â Â Â Â Â Â Â waste = waste ? alignment - waste : 0;
>> -Â Â Â Â Â Â Â waste += range_min - offset ;
>> -Â Â Â }
>> -
>> -Â Â Â if (waste) {
>> -Â Â Â Â Â Â Â n = calloc(1, sizeof(struct amdgpu_bo_va_hole));
>> -Â Â Â Â Â Â Â n->size = waste;
>> -Â Â Â Â Â Â Â n->offset = offset;
>> -Â Â Â Â Â Â Â list_add(&n->list, &mgr->va_holes);
>> -Â Â Â }
>> -
>> -Â Â Â offset += waste;
>> -Â Â Â mgr->va_offset = size + offset;
>> Â Â Â Â Â pthread_mutex_unlock(&mgr->bo_va_mutex);
>> -Â Â Â return offset;
>> +Â Â Â return AMDGPU_INVALID_VA_ADDRESS;
>> Â }
>> Â Â drm_private void
>> Â amdgpu_vamgr_free_va(struct amdgpu_bo_va_mgr *mgr, uint64_t va,
>> uint64_t size)
>> Â {
>> -Â Â Â struct amdgpu_bo_va_hole *hole;
>> +Â Â Â struct amdgpu_bo_va_hole *hole, *next;
>> Â Â Â Â Â Â if (va == AMDGPU_INVALID_VA_ADDRESS)
>> Â Â Â Â Â Â Â Â Â return;
>> @@ -274,61 +227,48 @@ amdgpu_vamgr_free_va(struct amdgpu_bo_va_mgr
>> *mgr, uint64_t va, uint64_t size)
>> Â Â Â Â Â size = ALIGN(size, mgr->va_alignment);
>> Â Â Â Â Â Â pthread_mutex_lock(&mgr->bo_va_mutex);
>> -Â Â Â if ((va + size) == mgr->va_offset) {
>> -Â Â Â Â Â Â Â mgr->va_offset = va;
>> -Â Â Â Â Â Â Â /* Delete uppermost hole if it reaches the new top */
>> -Â Â Â Â Â Â Â if (!LIST_IS_EMPTY(&mgr->va_holes)) {
>> -Â Â Â Â Â Â Â Â Â Â Â hole = container_of(mgr->va_holes.next, hole, list);
>> -Â Â Â Â Â Â Â Â Â Â Â if ((hole->offset + hole->size) == va) {
>> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â mgr->va_offset = hole->offset;
>> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â list_del(&hole->list);
>> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â free(hole);
>> -Â Â Â Â Â Â Â Â Â Â Â }
>> -Â Â Â Â Â Â Â }
>> -Â Â Â } else {
>> -Â Â Â Â Â Â Â struct amdgpu_bo_va_hole *next;
>> Â -Â Â Â Â Â Â Â hole = container_of(&mgr->va_holes, hole, list);
>> -Â Â Â Â Â Â Â LIST_FOR_EACH_ENTRY(next, &mgr->va_holes, list) {
>> -Â Â Â Â Â Â Â Â Â Â Â if (next->offset < va)
>> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â break;
>> -Â Â Â Â Â Â Â Â Â Â Â hole = next;
>> -Â Â Â Â Â Â Â }
>> +Â Â Â hole = container_of(&mgr->va_holes, hole, list);
>> +Â Â Â LIST_FOR_EACH_ENTRY(next, &mgr->va_holes, list) {
>> +Â Â Â Â Â Â Â if (next->offset < va)
>> +Â Â Â Â Â Â Â Â Â Â Â break;
>> +Â Â Â Â Â Â Â hole = next;
>> +Â Â Â }
>> Â -Â Â Â Â Â Â Â if (&hole->list != &mgr->va_holes) {
>> -Â Â Â Â Â Â Â Â Â Â Â /* Grow upper hole if it's adjacent */
>> -Â Â Â Â Â Â Â Â Â Â Â if (hole->offset == (va + size)) {
>> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â hole->offset = va;
>> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â hole->size += size;
>> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â /* Merge lower hole if it's adjacent */
>> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (next != hole
>> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â && &next->list != &mgr->va_holes
>> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â && (next->offset + next->size) == va) {
>> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â next->size += hole->size;
>> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â list_del(&hole->list);
>> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â free(hole);
>> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â }
>> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â goto out;
>> +Â Â Â if (&hole->list != &mgr->va_holes) {
>> +Â Â Â Â Â Â Â /* Grow upper hole if it's adjacent */
>> +Â Â Â Â Â Â Â if (hole->offset == (va + size)) {
>> +Â Â Â Â Â Â Â Â Â Â Â hole->offset = va;
>> +Â Â Â Â Â Â Â Â Â Â Â hole->size += size;
>> +Â Â Â Â Â Â Â Â Â Â Â /* Merge lower hole if it's adjacent */
>> +Â Â Â Â Â Â Â Â Â Â Â if (next != hole &&
>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â &next->list != &mgr->va_holes &&
>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (next->offset + next->size) == va) {
>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â next->size += hole->size;
>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â list_del(&hole->list);
>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â free(hole);
>> Â Â Â Â Â Â Â Â Â Â Â Â Â }
>> -Â Â Â Â Â Â Â }
>> -
>> -Â Â Â Â Â Â Â /* Grow lower hole if it's adjacent */
>> -Â Â Â Â Â Â Â if (next != hole && &next->list != &mgr->va_holes &&
>> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (next->offset + next->size) == va) {
>> -Â Â Â Â Â Â Â Â Â Â Â next->size += size;
>> Â Â Â Â Â Â Â Â Â Â Â Â Â goto out;
>> Â Â Â Â Â Â Â Â Â }
>> +Â Â Â }
>> Â -Â Â Â Â Â Â Â /* FIXME on allocation failure we just lose virtual
>> address space
>> -Â Â Â Â Â Â Â Â * maybe print a warning
>> -Â Â Â Â Â Â Â Â */
>> -Â Â Â Â Â Â Â next = calloc(1, sizeof(struct amdgpu_bo_va_hole));
>> -Â Â Â Â Â Â Â if (next) {
>> -Â Â Â Â Â Â Â Â Â Â Â next->size = size;
>> -Â Â Â Â Â Â Â Â Â Â Â next->offset = va;
>> -Â Â Â Â Â Â Â Â Â Â Â list_add(&next->list, &hole->list);
>> -Â Â Â Â Â Â Â }
>> +Â Â Â /* Grow lower hole if it's adjacent */
>> +Â Â Â if (next != hole && &next->list != &mgr->va_holes &&
>> +Â Â Â Â Â Â Â (next->offset + next->size) == va) {
>> +Â Â Â Â Â Â Â next->size += size;
>> +Â Â Â Â Â Â Â goto out;
>> Â Â Â Â Â }
>> +
>> +Â Â Â /* FIXME on allocation failure we just lose virtual address space
>> +Â Â Â Â * maybe print a warning
>> +Â Â Â Â */
>> +Â Â Â next = calloc(1, sizeof(struct amdgpu_bo_va_hole));
>> +Â Â Â if (next) {
>> +Â Â Â Â Â Â Â next->size = size;
>> +Â Â Â Â Â Â Â next->offset = va;
>> +Â Â Â Â Â Â Â list_add(&next->list, &hole->list);
>> +Â Â Â }
>> +
>> Â out:
>> Â Â Â Â Â pthread_mutex_unlock(&mgr->bo_va_mutex);
>> Â }
>
