* Re: [PATCH v6 2/2] drm/amdkfd: correct vram_available calculation
2026-09-24 3:30 ` [PATCH v6 2/2] drm/amdkfd: correct vram_available calculation Han, Zhen
@ 2026-09-24 3:40 ` Lazar, Lijo
0 siblings, 0 replies; 2+ messages in thread
From: Lazar, Lijo @ 2026-09-24 3:40 UTC (permalink / raw)
To: Han, Zhen, amd-gfx@lists.freedesktop.org
Cc: Hou, Junhua (Richard), Yang, Philip, Joshi, Mukul, Zhou, Tony,
Li, Chuan (Richard), Russell, Kent, Tudor, Alexandru, Wu, Cindy
On 24-Sep-26 9:00 AM, Han, Zhen wrote:
> AMD General
>
> reserve_mem_limit() computes available VRAM as
> vram_size - reserved - vram_pin_size - kfd.vram_used However, kfd.vram_used already includes BOs that are later pinned.
> Pinning does not consume extra VRAM. Subtracting vram_pin_size counts those bytes twice and causes fake-OOM while VRAM is still free. A KFD BO reaches the VRAM pin path through the P2P dma-buf export, through the dma-buf vmap and through map_bo_to_kernel().
>
> Reuse get_available_memory() in reserve_mem_limit(). Move ALIGN_DOWN to the ioctl so the userspace ABI is unchanged. The VRAM counter is a parameter: the ioctl passes vram_used_aligned[], the gate passes vram_used[] so a 4K BO is not charged 2MB.
>
> kfd.vram_pinned is only updated in gpuvm_pin_bo(). dma-buf pin calls amdgpu_bo_pin() only, so the counter stays zero. Update it from amdgpu_bo_pin() through a KFD helper, on first pin and last unpin, matching vram_pin_size.
>
> Track per-XCP pins in vram_pin_size_xcp[] and kfd.vram_pinned[].
> Pins that are not owned by a single partition go to vram_pin_size_common, which every partition subtracts.
>
> A KFD VRAM BO always carries a partition, so the KFD credit is only correct for xcp >= 0. Call the helper for both branches and let it warn once and skip if that ever stops holding: without the credit the BO would be subtracted through vram_pin_size_common and never added back, which is the double counting this patch removes, and kfd.vram_pinned[] would be indexed out of bounds.
>
> Signed-off-by: Zhen Han <Zhen.Han@amd.com>
Reviewed-by: Lijo Lazar <lijo.lazar@amd.com>
Thanks,
Lijo
> ---
> v6:
> - rebase v5 on patch 1/2, which adds kfd.vram_pinned (Mukul Joshi)
>
> v5:
> - add the kfd.vram_pinned[] counter. v4 read it in get_available_memory()
> but it does not exist in the tree (Mukul Joshi)
> - track pins per XCP in vram_pin_size_xcp[]; pins with no partition go to
> vram_pin_size_common (Philip Yang, Lijo Lazar)
> - pass the VRAM counter into the shared helper: the gate uses vram_used[],
> the ioctl vram_used_aligned[]
> - WARN_ONCE if a KFD BO reaches the VRAM pin path with xcp < 0 (Tony Zhou)
>
> v4:
> - use get_available_memory() in reserve_mem_limit()
> - move ALIGN_DOWN from get_available_memory() to the ioctl
> - drop v3 extra call in amdgpu_dma_buf.c
> - update kfd.vram_pinned from amdgpu_bo_pin() via a KFD helper
>
> v3:
> - update kfd.vram_pinned from dma-buf pin/vmap and gpuvm_pin_bo
> on first pin / last unpin in amdgpu_dma_buf.c
>
> v2:
> - keep vram_pin_size; add kfd.vram_pinned in reserve_mem_limit (Mukul Joshi)
> - leave get_available_memory unchanged
> - drop former 2/2 pin/unpin changes in amdgpu_object.c
>
> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 11 ++-
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h | 20 +++-
> .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 94 +++++++++++--------
> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 33 +++++++
> drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 12 ++-
> 5 files changed, 127 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 79b69d74eb2e..04151eab3b3f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -839,8 +839,17 @@ struct amdgpu_device {
> struct mutex mn_lock;
> DECLARE_HASHTABLE(mn_hash, 7);
>
> - /* tracking pinned memory */
> + /* tracking pinned memory
> + *
> + * vram_pin_size = sum(vram_pin_size_xcp[]) + vram_pin_size_common
> + *
> + * vram_pin_size total pin size in device
> + * vram_pin_size_xcp[i] pins owned by partition i (bo->xcp_id = i, i >= 0)
> + * vram_pin_size_common pins not owned by any partition, bo->xcp_id = -1
> + */
> atomic64_t vram_pin_size;
> + atomic64_t vram_pin_size_xcp[MAX_XCP];
> + atomic64_t vram_pin_size_common;
> atomic64_t visible_pin_size;
> atomic64_t gart_pin_size;
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> index 056dd7375d24..f462b40220eb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> @@ -42,6 +42,12 @@
>
> extern uint64_t amdgpu_amdkfd_total_mem_size;
>
> +/*
> + * Align VRAM availability to 2MB to avoid fragmentation caused by 4K
> + * allocations in the tail 2MB BO chunk.
> + */
> +#define VRAM_AVAILABLITY_ALIGN (1 << 21)
> +
> enum TLB_FLUSH_TYPE {
> TLB_FLUSH_LEGACY = 0,
> TLB_FLUSH_LIGHTWEIGHT,
> @@ -106,7 +112,7 @@ struct amdgpu_kfd_dev {
> struct kfd_dev *dev;
> int64_t vram_used[MAX_XCP];
> uint64_t vram_used_aligned[MAX_XCP];
> - atomic64_t vram_pinned;
> + atomic64_t vram_pinned[MAX_XCP];
> bool init_complete;
> struct work_struct reset_work;
>
> @@ -391,6 +397,8 @@ void amdgpu_amdkfd_gpuvm_destroy_cb(struct amdgpu_device *adev,
> * Allows KFD to release its resources associated with the GEM object.
> */
> void amdgpu_amdkfd_release_notify(struct amdgpu_bo *bo);
> +void amdgpu_amdkfd_account_vram_pin(struct amdgpu_bo *bo, int xcp_id);
> +void amdgpu_amdkfd_account_vram_unpin(struct amdgpu_bo *bo, int
> +xcp_id);
> void amdgpu_amdkfd_reserve_system_mem(uint64_t size); #else static inline @@ -408,6 +416,16 @@ static inline void amdgpu_amdkfd_release_notify(struct amdgpu_bo *bo) { }
> +
> +static inline
> +void amdgpu_amdkfd_account_vram_pin(struct amdgpu_bo *bo, int xcp_id) {
> +}
> +
> +static inline
> +void amdgpu_amdkfd_account_vram_unpin(struct amdgpu_bo *bo, int xcp_id)
> +{ }
> #endif
>
> #if IS_ENABLED(CONFIG_HSA_AMD_SVM)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index 5a4072792a57..34cd791d826e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -46,12 +46,6 @@
> #define AMDGPU_USERPTR_RESTORE_DELAY_MS 1
> #define AMDGPU_RESERVE_MEM_LIMIT (3UL << 29)
>
> -/*
> - * Align VRAM availability to 2MB to avoid fragmentation caused by 4K allocations in the tail 2MB
> - * BO chunk
> - */
> -#define VRAM_AVAILABLITY_ALIGN (1 << 21)
> -
> /* Impose limit on how much memory KFD can use */ static struct {
> uint64_t max_system_mem_limit;
> @@ -73,6 +67,8 @@ static const char * const domain_bit_to_string[] = { #define domain_string(domain) domain_bit_to_string[ffs(domain)-1]
>
> static void amdgpu_amdkfd_restore_userptr_worker(struct work_struct *work);
> +static size_t __get_available_memory(struct amdgpu_device *adev,
> + uint8_t xcp_id, int64_t vram_used);
>
> static bool kfd_mem_is_attached(struct amdgpu_vm *avm,
> struct kgd_mem *mem)
> @@ -169,13 +165,8 @@ void amdgpu_amdkfd_reserve_system_mem(uint64_t size) int amdgpu_amdkfd_reserve_mem_limit(struct amdgpu_device *adev,
> uint64_t size, u32 alloc_flag, int8_t xcp_id) {
> - uint64_t reserved_for_pt =
> - ESTIMATE_PT_SIZE(amdgpu_amdkfd_total_mem_size);
> - struct amdgpu_ras *con = amdgpu_ras_get_context(adev);
> - uint64_t reserved_for_ras = (con ? con->reserved_pages_in_bytes : 0);
> size_t system_mem_needed, ttm_mem_needed, vram_needed;
> int ret = 0;
> - uint64_t vram_size = 0;
>
> system_mem_needed = 0;
> ttm_mem_needed = 0;
> @@ -196,7 +187,6 @@ int amdgpu_amdkfd_reserve_mem_limit(struct amdgpu_device *adev,
> if (WARN_ONCE(xcp_id < 0, "invalid XCP ID %d", xcp_id))
> return -EINVAL;
>
> - vram_size = KFD_XCP_MEMORY_SIZE(adev, xcp_id);
> if (adev->apu_prefer_gtt) {
> system_mem_needed = size;
> ttm_mem_needed = size;
> @@ -232,14 +222,11 @@ int amdgpu_amdkfd_reserve_mem_limit(struct amdgpu_device *adev,
> * VRAM check since ttm_mem_limit check already cover this allocation
> */
>
> - if (adev && xcp_id >= 0 && (!adev->apu_prefer_gtt || adev->gmc.is_app_apu)) {
> - uint64_t vram_available =
> - vram_size - reserved_for_pt - reserved_for_ras -
> - atomic64_read(&adev->vram_pin_size);
> - if (adev->kfd.vram_used[xcp_id] + vram_needed > vram_available) {
> - ret = -ENOMEM;
> - goto release;
> - }
> + if (adev && xcp_id >= 0 && (!adev->apu_prefer_gtt || adev->gmc.is_app_apu) &&
> + vram_needed > __get_available_memory(adev, xcp_id,
> + adev->kfd.vram_used[xcp_id])) {
> + ret = -ENOMEM;
> + goto release;
> }
>
> /* Update memory accounting by decreasing available system @@ -318,6 +305,30 @@ void amdgpu_amdkfd_release_notify(struct amdgpu_bo *bo)
> kfree(bo->kfd_bo);
> }
>
> +void amdgpu_amdkfd_account_vram_pin(struct amdgpu_bo *bo, int xcp_id) {
> + if (!bo->kfd_bo)
> + return;
> +
> + if (WARN_ONCE(xcp_id < 0, "KFD BO pinned in VRAM with no partition"))
> + return;
> +
> + atomic64_add(amdgpu_bo_size(bo),
> + &amdgpu_ttm_adev(bo->tbo.bdev)->kfd.vram_pinned[xcp_id]);
> +}
> +
> +void amdgpu_amdkfd_account_vram_unpin(struct amdgpu_bo *bo, int xcp_id)
> +{
> + if (!bo->kfd_bo)
> + return;
> +
> + if (WARN_ONCE(xcp_id < 0, "KFD BO unpinned in VRAM with no partition"))
> + return;
> +
> + atomic64_sub(amdgpu_bo_size(bo),
> + &amdgpu_ttm_adev(bo->tbo.bdev)->kfd.vram_pinned[xcp_id]);
> +}
> +
> /**
> * create_dmamap_sg_bo() - Creates a amdgpu_bo object to reflect information
> * about USERPTR or DOOREBELL or MMIO BO.
> @@ -1537,10 +1548,6 @@ static int amdgpu_amdkfd_gpuvm_pin_bo(struct amdgpu_bo *bo, u32 domain)
>
> amdgpu_bo_sync_wait(bo, AMDGPU_FENCE_OWNER_KFD, false);
>
> - if (!ret && bo->tbo.resource->mem_type == TTM_PL_VRAM)
> - atomic64_add(amdgpu_bo_size(bo),
> - &amdgpu_ttm_adev(bo->tbo.bdev)->kfd.vram_pinned);
> -
> out:
> amdgpu_bo_unreserve(bo);
> return ret;
> @@ -1564,10 +1571,6 @@ static void amdgpu_amdkfd_gpuvm_unpin_bo(struct amdgpu_bo *bo)
>
> amdgpu_bo_unpin(bo);
>
> - if (bo->tbo.resource->mem_type == TTM_PL_VRAM)
> - atomic64_sub(amdgpu_bo_size(bo),
> - &amdgpu_ttm_adev(bo->tbo.bdev)->kfd.vram_pinned);
> -
> amdgpu_bo_unreserve(bo);
> }
>
> @@ -1671,8 +1674,9 @@ int amdgpu_amdkfd_criu_resume(void *p)
> return ret;
> }
>
> -size_t amdgpu_amdkfd_get_available_memory(struct amdgpu_device *adev,
> - uint8_t xcp_id)
> +/* Caller must hold kfd_mem_limit.mem_limit_lock. */ static size_t
> +__get_available_memory(struct amdgpu_device *adev,
> + uint8_t xcp_id, int64_t vram_used)
> {
> uint64_t reserved_for_pt =
> ESTIMATE_PT_SIZE(amdgpu_amdkfd_total_mem_size);
> @@ -1681,15 +1685,17 @@ size_t amdgpu_amdkfd_get_available_memory(struct amdgpu_device *adev,
> ssize_t available;
> uint64_t vram_available, system_mem_available, ttm_mem_available;
>
> - spin_lock(&kfd_mem_limit.mem_limit_lock);
> + lockdep_assert_held(&kfd_mem_limit.mem_limit_lock);
> +
> if (adev->apu_prefer_gtt && !adev->gmc.is_app_apu)
> vram_available = KFD_XCP_MEMORY_SIZE(adev, xcp_id)
> - - adev->kfd.vram_used_aligned[xcp_id];
> + - vram_used;
> else
> vram_available = KFD_XCP_MEMORY_SIZE(adev, xcp_id)
> - - adev->kfd.vram_used_aligned[xcp_id]
> - - atomic64_read(&adev->vram_pin_size)
> - + atomic64_read(&adev->kfd.vram_pinned)
> + - vram_used
> + - atomic64_read(&adev->vram_pin_size_xcp[xcp_id])
> + - atomic64_read(&adev->vram_pin_size_common)
> + + atomic64_read(&adev->kfd.vram_pinned[xcp_id])
> - reserved_for_pt
> - reserved_for_ras;
>
> @@ -1704,19 +1710,29 @@ size_t amdgpu_amdkfd_get_available_memory(struct amdgpu_device *adev,
>
> available = min3(system_mem_available, ttm_mem_available,
> vram_available);
> - available = ALIGN_DOWN(available, PAGE_SIZE);
> } else {
> - available = ALIGN_DOWN(vram_available, VRAM_AVAILABLITY_ALIGN);
> + available = vram_available;
> }
>
> - spin_unlock(&kfd_mem_limit.mem_limit_lock);
> -
> if (available < 0)
> available = 0;
>
> return available;
> }
>
> +size_t amdgpu_amdkfd_get_available_memory(struct amdgpu_device *adev,
> + uint8_t xcp_id)
> +{
> + size_t available;
> +
> + spin_lock(&kfd_mem_limit.mem_limit_lock);
> + available = __get_available_memory(adev, xcp_id,
> + adev->kfd.vram_used_aligned[xcp_id]);
> + spin_unlock(&kfd_mem_limit.mem_limit_lock);
> +
> + return available;
> +}
> +
> int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
> struct amdgpu_device *adev, uint64_t va, uint64_t size,
> void *drm_priv, struct kgd_mem **mem, diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index fc6d3fa62d79..dc0ab5bc467f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -923,6 +923,19 @@ void amdgpu_bo_unref(struct amdgpu_bo **bo)
> *bo = NULL;
> }
>
> +/* Map a BO onto vram_pin_size_xcp[]. bo->xcp_id < 0 means "any partition":
> + * such pins are not owned by a single partition and go to
> + * vram_pin_size_common, which every partition subtracts.
> + */
> +static int amdgpu_bo_get_xcp(struct amdgpu_bo *bo) {
> + s8 xcp = bo->xcp_id;
> +
> + if (xcp >= 0 && xcp < MAX_XCP)
> + return xcp;
> + return -1;
> +}
> +
> /**
> * amdgpu_bo_pin - pin an &amdgpu_bo buffer object
> * @bo: &amdgpu_bo buffer object to be pinned @@ -1003,7 +1016,17 @@ int amdgpu_bo_pin(struct amdgpu_bo *bo, u32 domain)
> ttm_bo_pin(&bo->tbo);
>
> if (bo->tbo.resource->mem_type == TTM_PL_VRAM) {
> + int xcp = amdgpu_bo_get_xcp(bo);
> +
> atomic64_add(amdgpu_bo_size(bo), &adev->vram_pin_size);
> + if (xcp >= 0) {
> + atomic64_add(amdgpu_bo_size(bo),
> + &adev->vram_pin_size_xcp[xcp]);
> + } else {
> + atomic64_add(amdgpu_bo_size(bo),
> + &adev->vram_pin_size_common);
> + }
> + amdgpu_amdkfd_account_vram_pin(bo, xcp);
> atomic64_add(amdgpu_vram_mgr_bo_visible_size(bo),
> &adev->visible_pin_size);
> } else if (bo->tbo.resource->mem_type == TTM_PL_TT) { @@ -1036,7 +1059,17 @@ void amdgpu_bo_unpin(struct amdgpu_bo *bo)
> dma_buf_unpin(bo->tbo.base.import_attach);
>
> if (bo->tbo.resource->mem_type == TTM_PL_VRAM) {
> + int xcp = amdgpu_bo_get_xcp(bo);
> +
> atomic64_sub(amdgpu_bo_size(bo), &adev->vram_pin_size);
> + if (xcp >= 0) {
> + atomic64_sub(amdgpu_bo_size(bo),
> + &adev->vram_pin_size_xcp[xcp]);
> + } else {
> + atomic64_sub(amdgpu_bo_size(bo),
> + &adev->vram_pin_size_common);
> + }
> + amdgpu_amdkfd_account_vram_unpin(bo, xcp);
> atomic64_sub(amdgpu_vram_mgr_bo_visible_size(bo),
> &adev->visible_pin_size);
> } else if (bo->tbo.resource->mem_type == TTM_PL_TT) { diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> index 7fcfc150a7fc..2295f9bcc5e4 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> @@ -1069,11 +1069,19 @@ static int kfd_ioctl_get_available_memory(struct file *filep, {
> struct kfd_ioctl_get_available_memory_args *args = data;
> struct kfd_process_device *pdd = kfd_lock_pdd_by_id(p, args->gpu_id);
> + struct amdgpu_device *adev;
> + size_t available;
>
> if (!pdd)
> return -EINVAL;
> - args->available = amdgpu_amdkfd_get_available_memory(pdd->dev->adev,
> - pdd->dev->node_id);
> +
> + adev = pdd->dev->adev;
> + available = amdgpu_amdkfd_get_available_memory(adev, pdd->dev->node_id);
> + if (adev->apu_prefer_gtt)
> + args->available = ALIGN_DOWN(available, PAGE_SIZE);
> + else
> + args->available = ALIGN_DOWN(available, VRAM_AVAILABLITY_ALIGN);
> +
> kfd_unlock_pdd(pdd);
> return 0;
> }
> --
> 2.43.0
^ permalink raw reply [flat|nested] 2+ messages in thread