| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <c8ac36f3-06ea-40c3-a5e6-da3964ac3b22@amd.com>
Date: Thu, 3 Jul 2025 10:37:45 +0200
From: Christian König <christian.koenig@....com>
To: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@....com>,
Alex Deucher <alexander.deucher@....com>,
Christian König <christian.koenig@....com>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
Matthew Auld <matthew.auld@...el.com>,
Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@....com>
Cc: amd-gfx@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 1/3] drm/buddy: add a flag to disable trimming of non
cleared blocks
On 02.07.25 18:12, Pierre-Eric Pelloux-Prayer wrote:
> A vkcts test case is triggering a case where the drm buddy allocator
> wastes lots of memory and performs badly:
>
> dEQP-VK.memory.allocation.basic.size_8KiB.reverse.count_4000
>
> For each memory pool type, the test will allocate 4000 8kB objects,
> and then will release them. The alignment request is 256kB.
>
> For each object, the allocator will select a 256kB block (to
> match the alignment), and then trim it to 8kB, adding lots of free
> entries to the free_lists of order 5 to 1.
> On deallocation, none of these objects will be merged with their
> buddy because their "clear status" is different: only the block
> that was handed over to the driver might come back cleared.
> Also since the test don't allocate much memory, the allocator don't
> need to force the merge process so it will repeat the same logic
> for each run.
>
> As a result, after the first run (which takes about 6sec), the
> freelists look like this:
>
> chunk_size: 4KiB, total: 16368MiB, free: 15354MiB, clear_free: 397MiB
> [...]
> order- 5 free: 1914 MiB, blocks: 15315
> order- 4 free: 957 MiB, blocks: 15325
> order- 3 free: 480 MiB, blocks: 15360
> order- 2 free: 239 MiB, blocks: 15347
> order- 1 free: 238 MiB, blocks: 30489
>
> After the second run (19 sec):
>
> chunk_size: 4KiB, total: 16368MiB, free: 15374MiB, clear_free: 537MiB
> [...]
> order- 5 free: 3326 MiB, blocks: 26615
> order- 4 free: 1663 MiB, blocks: 26619
> order- 3 free: 833 MiB, blocks: 26659
> order- 2 free: 416 MiB, blocks: 26643
> order- 1 free: 414 MiB, blocks: 53071
>
> list_insert_sorted is part of the problem here since it iterates
> over the free_list to figure out where to insert the new blocks.
>
> To fix this while keeping the clear tracking information, a new
> bit is exposed to drivers, allowing them to disable trimming for
> blocks that aren't "clear". This bit is used by amdgpu because
> it always returns cleared memory to drm_buddy.
That's an extremely good catch, but I don't think this is the right solution.
If I understood it correctly we now give back 256k instead of 8k to the caller and that is a really big no-go.
Regards,
Christian.
>
> With this bit set, the "merge buddies on deallocation logic" can
> work again, and the free_list are not growing indefinitely anymore.
>
> So after a run we get:
>
> chunk_size: 4KiB, total: 16368MiB, free: 15306MiB, clear_free: 1734MiB
> [...]
> order- 5 free: 2 MiB, blocks: 17
> order- 4 free: 2 MiB, blocks: 35
> order- 3 free: 1 MiB, blocks: 41
> order- 2 free: 656 KiB, blocks: 41
> order- 1 free: 256 KiB, blocks: 32
>
> The runtime is better (2 sec) and stable across multiple runs, and we
> also see that the reported "clear_free" amount is larger than without
> the patch.
>
> Fixes: 96950929eb23 ("drm/buddy: Implement tracking clear page feature")
> Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@....com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 8 ++++++++
> drivers/gpu/drm/drm_buddy.c | 1 +
> include/drm/drm_buddy.h | 1 +
> 3 files changed, 10 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> index abdc52b0895a..dbbaa15a973e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> @@ -499,6 +499,14 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man,
>
> INIT_LIST_HEAD(&vres->blocks);
>
> + /* Trimming create smaller blocks that may never be given to the driver.
> + * Such blocks won't be cleared until being seen by the driver, which might
> + * never occur (for instance UMD might request large alignment) => in such
> + * case, upon release of the block, the drm_buddy allocator won't merge them
> + * back, because their clear status is different.
> + */
> + vres->flags = DRM_BUDDY_TRIM_IF_CLEAR;
> +
> if (place->flags & TTM_PL_FLAG_TOPDOWN)
> vres->flags |= DRM_BUDDY_TOPDOWN_ALLOCATION;
>
> diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
> index a1e652b7631d..555c72abce4c 100644
> --- a/drivers/gpu/drm/drm_buddy.c
> +++ b/drivers/gpu/drm/drm_buddy.c
> @@ -1092,6 +1092,7 @@ int drm_buddy_alloc_blocks(struct drm_buddy *mm,
>
> /* Trim the allocated block to the required size */
> if (!(flags & DRM_BUDDY_TRIM_DISABLE) &&
> + (!(flags & DRM_BUDDY_TRIM_IF_CLEAR) || drm_buddy_block_is_clear(block)) &&
> original_size != size) {
> struct list_head *trim_list;
> LIST_HEAD(temp);
> diff --git a/include/drm/drm_buddy.h b/include/drm/drm_buddy.h
> index 9689a7c5dd36..c338d03028c3 100644
> --- a/include/drm/drm_buddy.h
> +++ b/include/drm/drm_buddy.h
> @@ -28,6 +28,7 @@
> #define DRM_BUDDY_CLEAR_ALLOCATION BIT(3)
> #define DRM_BUDDY_CLEARED BIT(4)
> #define DRM_BUDDY_TRIM_DISABLE BIT(5)
> +#define DRM_BUDDY_TRIM_IF_CLEAR BIT(6)
>
> struct drm_buddy_block {
> #define DRM_BUDDY_HEADER_OFFSET GENMASK_ULL(63, 12)
Powered by blists - more mailing lists