Hi Oak, Christian
On 11/2/23 05:32, Oak Zeng wrote:
We plan to implement xe driver's shared virtual memory
manager (aka SVM) without buffer object concept. This
means we won't build our shared virtual memory manager
upon TTM infrastructure like amdgpu does.
Even though this approach is more efficient, it does
create a problem for memory eviction when there is
memory pressure: memory allocated by SVM and memory
allocated by TTM should be able to mutually evict
from each other. TTM's resource manager maintains
a LRU list for each memory type and this list is used
to pick up the memory eviction victim. Since we don't
use TTM for SVM implementation, SVM allocated memory
can't be added to TTM resource manager's LRU list. Thus
SVM allocated memory and TTM allocated memory are not
mutually evictable.
See more discussion on this topic here:
https://www.spinics.net/lists/dri-devel/msg410740.html
This series solve this problem by creating a shared
LRU list b/t SVM and TTM, or any other resource manager.
The basic idea is, abstract a drm_lru_entity structure
which is supposed to be embedded in ttm_resource structure,
or any other resource manager. The resource LRU list is a
list of drm_lru_entity. drm_lru_entity has eviction function
pointers which can be used to call back drivers' specific
eviction function to evict a memory resource.
Introduce global drm_lru_manager to struct drm_device
to manage LRU lists. Each memory type or memory region
can have a LRU list. TTM resource manager's LRU list functions
including bulk move functions are moved to drm lru manager.
drm lru manager provides a evict_first function to evict
the first memory resource from LRU list. This function can
be called from TTM, SVM or any other resource manager, so
all the memory allocated in the drm sub-system can be mutually
evicted.
The lru_lock is also moved from struct ttm_device to struct
drm_device.
Opens:
1) memory accounting: currently the ttm resource manager's
memory accounting functions is kept at ttm resource manager.
Since memory accounting should be cross TTM and SVM, it should
be ideally in the drm lru manager layer. This will be polished
in the future.
2) eviction callback function interface: The current eviction
function interface is designed to meet TTM memory eviction
requirements. When SVM is in the picture, this interface
need to be futher tunned to meet SVM requirement also.
This series is not tested and it is only compiled for xe
driver. Some minor changes are needed for other driver
such as amdgpu, nouveau etc. I intended to send this out
as a request for comment series to get some early feedback,
to see whether this is the right direction to go. I will
futher polish this series after a direction is agreed.
Oak Zeng (11):
drm/ttm: re-parameter ttm_device_init
drm: move lru_lock from ttm_device to drm_device
drm: introduce drm evictable LRU
drm: Add evict function pointer to drm lru entity
drm: Replace ttm macros with drm macros
drm/ttm: Set lru manager to ttm resource manager
drm/ttm: re-parameterize a few ttm functions
drm: Initialize drm lru manager
drm/ttm: Use drm LRU manager iterator
drm/ttm: Implement ttm memory evict functions
drm/ttm: Write ttm functions using drm lru manager functions
drivers/gpu/drm/Makefile | 1 +
drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 6 +
.../gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c | 6 +
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 10 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 6 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 10 +-
drivers/gpu/drm/drm_drv.c | 1 +
drivers/gpu/drm/drm_evictable_lru.c | 266 ++++++++++++++++++
drivers/gpu/drm/drm_gem_vram_helper.c | 10 +-
drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 6 +-
drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 10 +
drivers/gpu/drm/i915/intel_region_ttm.c | 4 +-
drivers/gpu/drm/i915/selftests/mock_region.c | 2 +-
drivers/gpu/drm/loongson/lsdc_ttm.c | 10 +-
drivers/gpu/drm/nouveau/nouveau_ttm.c | 22 +-
drivers/gpu/drm/qxl/qxl_ttm.c | 6 +-
drivers/gpu/drm/radeon/radeon_ttm.c | 10 +-
drivers/gpu/drm/ttm/tests/ttm_device_test.c | 2 +-
drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c | 2 +-
drivers/gpu/drm/ttm/ttm_bo.c | 247 ++++++++++++----
drivers/gpu/drm/ttm/ttm_bo_util.c | 20 +-
drivers/gpu/drm/ttm/ttm_bo_vm.c | 2 +-
drivers/gpu/drm/ttm/ttm_device.c | 55 ++--
drivers/gpu/drm/ttm/ttm_module.h | 3 +-
drivers/gpu/drm/ttm/ttm_range_manager.c | 14 +-
drivers/gpu/drm/ttm/ttm_resource.c | 242 +++-------------
drivers/gpu/drm/ttm/ttm_sys_manager.c | 8 +-
drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_bo.h | 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 6 +-
.../gpu/drm/vmwgfx/vmwgfx_system_manager.c | 6 +
drivers/gpu/drm/xe/xe_bo.c | 48 ++--
drivers/gpu/drm/xe/xe_bo.h | 5 +-
drivers/gpu/drm/xe/xe_device.c | 2 +-
drivers/gpu/drm/xe/xe_dma_buf.c | 4 +-
drivers/gpu/drm/xe/xe_exec.c | 6 +-
drivers/gpu/drm/xe/xe_migrate.c | 6 +-
drivers/gpu/drm/xe/xe_res_cursor.h | 10 +-
drivers/gpu/drm/xe/xe_ttm_sys_mgr.c | 8 +-
drivers/gpu/drm/xe/xe_ttm_vram_mgr.c | 18 +-
drivers/gpu/drm/xe/xe_vm.c | 6 +-
drivers/gpu/drm/xe/xe_vm_types.h | 2 +-
include/drm/drm_device.h | 12 +
include/drm/drm_evictable_lru.h | 260 +++++++++++++++++
include/drm/ttm/ttm_bo.h | 10 +-
include/drm/ttm/ttm_device.h | 13 +-
include/drm/ttm/ttm_range_manager.h | 17 +-
include/drm/ttm/ttm_resource.h | 117 +++-----
49 files changed, 1042 insertions(+), 501 deletions(-)
create mode 100644 drivers/gpu/drm/drm_evictable_lru.c
create mode 100644 include/drm/drm_evictable_lru.h
Some additional comments after looking through this patchset and the
comments.
1) General: IMO a good start.
2) Where should this reside? I'm not a big fan of *prescribing* that a
component should be part of a specific device structure. IMO that leads
to dumping the whole drm namespace into this component rather than to
keep it isolated with as few dependencies as possible. I would make the
part that should reside in the drm device a "struct drm_lru_device", and
the resource base class a "struct drm_lru_resource". Whether the drm
device then wants to embed the struct drm_lru_device (nice if you need
this component) or have a pointer to it (nice if you don't need this
component) and whether the ttm_device should subclass the drm device
becomes pretty orthogonal to the actual implementation, and we'd avoid
hard to follow code cross-references all over the place. The drm_gpuvm
IMO did a good job with this, but I know there are other opinions on
this, and I don't want it to become a blocker.
3) Christian's comment about cursors into the LRU for LRU traversal restart
This is quickly becoming a must for the Xe driver, and a very-nice to
have for a working TTM shrinker, but I think we should start to bring
what we have in TTM currently over and do the cursors as a separate
task. I have it pretty much on top of my priority list currently. Both
downstream i915 and drm_gpuvm have a way to handle this nicely by
permutating the LRU list just before the lock needs to be released, so I
was thinking something similar. The complication would be the bulk move
tracking.
4) The struct drm_lru_evict_ctx - This one needs to be common across all
users since it's set up by the evictor and used by the evictee ops.
/Thomas
