[AMD Official Use Only - General] OK
----------------- Best Regards, Thomas -----Original Message----- From: Zhou1, Tao <tao.zh...@amd.com> Sent: Tuesday, April 9, 2024 10:52 AM To: Chai, Thomas <yipeng.c...@amd.com>; amd-gfx@lists.freedesktop.org Cc: Zhang, Hawking <hawking.zh...@amd.com>; Li, Candice <candice...@amd.com>; Wang, Yang(Kevin) <kevinyang.w...@amd.com>; Yang, Stanley <stanley.y...@amd.com> Subject: RE: [PATCH] drm/amdgpu: Fix incorrect return value [AMD Official Use Only - General] > -----Original Message----- > From: Chai, Thomas <yipeng.c...@amd.com> > Sent: Wednesday, April 3, 2024 3:07 PM > To: amd-gfx@lists.freedesktop.org > Cc: Chai, Thomas <yipeng.c...@amd.com>; Zhang, Hawking > <hawking.zh...@amd.com>; Zhou1, Tao <tao.zh...@amd.com>; Li, Candice > <candice...@amd.com>; Wang, Yang(Kevin) <kevinyang.w...@amd.com>; > Yang, Stanley <stanley.y...@amd.com>; Chai, Thomas > <yipeng.c...@amd.com> > Subject: [PATCH] drm/amdgpu: Fix incorrect return value > > [Why] > After calling amdgpu_vram_mgr_reserve_range multiple times with the > same address, calling amdgpu_vram_mgr_query_page_status will always > return - EBUSY. > From the second call to amdgpu_vram_mgr_reserve_range, the same > address will be added to the reservations_pending list again and is > never moved to the reserved_pages list because the address had been reserved. > > [How] > First add the address status check before calling > amdgpu_vram_mgr_do_reserve, if the address is already reserved, do > nothing; If the address is already in the reservations_pending list, > directly reserve memory; only add new nodes for the addresses that are > not in the reserved_pages list and reservations_pending list. > > Signed-off-by: YiPeng Chai <yipeng.c...@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 28 > +++++++++++++------- > 1 file changed, 19 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c > index 1e36c428d254..0bf3f4092900 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c > @@ -317,7 +317,6 @@ static void amdgpu_vram_mgr_do_reserve(struct > ttm_resource_manager *man) > > dev_dbg(adev->dev, "Reservation 0x%llx - %lld, Succeeded\n", > rsv->start, rsv->size); > - > vis_usage = amdgpu_vram_mgr_vis_size(adev, block); > atomic64_add(vis_usage, &mgr->vis_usage); > spin_lock(&man->bdev->lru_lock); @@ -340,19 +339,30 @@ > int amdgpu_vram_mgr_reserve_range(struct > amdgpu_vram_mgr *mgr, > uint64_t start, uint64_t size) { > struct amdgpu_vram_reservation *rsv; > + int ret = 0; > > - rsv = kzalloc(sizeof(*rsv), GFP_KERNEL); > - if (!rsv) > - return -ENOMEM; > + ret = amdgpu_vram_mgr_query_page_status(mgr, start); > + if (!ret) > + return 0; > + > + if (ret == -ENOENT) { > + rsv = kzalloc(sizeof(*rsv), GFP_KERNEL); > + if (!rsv) > + return -ENOMEM; > > - INIT_LIST_HEAD(&rsv->allocated); > - INIT_LIST_HEAD(&rsv->blocks); > + INIT_LIST_HEAD(&rsv->allocated); > + INIT_LIST_HEAD(&rsv->blocks); > > - rsv->start = start; > - rsv->size = size; > + rsv->start = start; > + rsv->size = size; > + > + mutex_lock(&mgr->lock); > + list_add_tail(&rsv->blocks, &mgr->reservations_pending); > + mutex_unlock(&mgr->lock); [Tao] we can drop the mutex_unlock and add if (ret != -ENOENT) for the second mutex_lock to avoid unlocking/locking repeatedly. > + > + } > > mutex_lock(&mgr->lock); > - list_add_tail(&rsv->blocks, &mgr->reservations_pending); > amdgpu_vram_mgr_do_reserve(&mgr->manager); > mutex_unlock(&mgr->lock); > > -- > 2.34.1