[+Christian]
I'm looking into virtual address reservations in amdgpu and what's
reported by AMDGPU_INFO_DEV_INFO. So far I only found
AMDGPU_VA_RESERVED_SIZE, which is reserved both at the start of the
lower virtual address range and the end of the upper virtual address range.
The reservation size is currently 2MB. The upper reservation is used by
the CSA, which is currently 128KB. That leaves plenty of room for the
TMA/TBA above the CSA. I'll create a patch for that.
Regards,
Felix
On 2024-01-10 12:45, Marek Olšák wrote:
It looks like this would cause failures even with regular 64-bit
allocations because the virtual address range allocator in libdrm asks
the kernel what ranges of addresses are free, and the kernel doesn't
exclude the KFD allocation from that.
Basically, no VM allocations can be done by the kernel outside the
ranges reserved for the kernel.
Marek
On Sat, Jan 6, 2024 at 1:48 AM Marek Olšák <maraeo@xxxxxxxxx> wrote:
The 32-bit address space means the high 32 bits are constant and predetermined and it's definitely somewhere in the upper range of the address space. If ROCm or KFD occupy that space, even accidentally, other UMDs that use libdrm for VA allocation won't be able to start. The VA range allocator is in libdrm.
Marek
On Fri, Jan 5, 2024, 15:20 Felix Kuehling <felix.kuehling@xxxxxxx> wrote:
TBA/TMA were relocated to the upper half of the canonical address space.
I don't think that qualifies as 32-bit by definition. But maybe you're
using a different definition.
That said, if Mesa manages its own virtual address space in user mode,
and KFD maps the TMA/TBA at an address that Mesa believes to be free, I
can see how that would lead to problems.
That said, the fence refcount bug is another problem that may have been
exposed by the way that a crashing Mesa application shuts down.
Reverting Jay's patch certainly didn't fix that, but only hides the problem.
Regards,
Felix
On 2024-01-04 13:29, Marek Olšák wrote:
Hi,
I have received information that the original commit makes all 32-bit
userspace VA allocations fail, so UMDs like Mesa can't even initialize
and they either crash or fail to load. If TBA/TMA was relocated to the
32-bit address range, it would explain why UMDs can't allocate
anything in that range.
Marek
On Wed, Jan 3, 2024 at 2:50 PM Jay Cornwall <jay.cornwall@xxxxxxx> wrote:
On 1/3/2024 12:58, Felix Kuehling wrote:
A segfault in Mesa seems to be a different issue from what's mentioned
in the commit message. I'd let Christian or Marek comment on
compatibility with graphics UMDs. I'm not sure why this patch would
affect them at all.
I was referencing this issue in OpenCL/OpenGL interop, which certainly looked related:
[ 91.769002] amdgpu 0000:0a:00.0: amdgpu: bo 000000009bba4692 va 0x0800000000-0x08000001ff conflict with 0x0800000000-0x0800000002
[ 91.769141] ocltst[2781]: segfault at b2 ip 00007f3fb90a7c39 sp 00007ffd3c011ba0 error 4 in radeonsi_dri.so[7f3fb888e000+1196000] likely on CPU 15 (core 7, socket 0)
Looking at the logs in the tickets, it looks like a fence reference
counting error. I don't see how Jay's patch could have caused that. I
made another change in that code recently that could make a difference
for this issue:
commit 8f08c5b24ced1be7eb49692e4816c1916233c79b
Author: Felix Kuehling <Felix.Kuehling@xxxxxxx>
Date: Fri Oct 27 18:21:55 2023 -0400
drm/amdkfd: Run restore_workers on freezable WQs
Make restore workers freezable so we don't have to explicitly
flush them
in suspend and GPU reset code paths, and we don't accidentally
try to
restore BOs while the GPU is suspended. Not having to flush
restore_work
also helps avoid lock/fence dependencies in the GPU reset case
where we're
not allowed to wait for fences.
A side effect of this is, that we can now have multiple
concurrent threads
trying to signal the same eviction fence. Rework eviction fence
signaling
and replacement to account for that.
The GPU reset path can no longer rely on restore_process_worker
to resume
queues because evict/restore workers can run independently of
it. Instead
call a new restore_process_helper directly.
This is an RFC and request for testing.
v2:
- Reworked eviction fence signaling
- Introduced restore_process_helper
v3:
- Handle unsignaled eviction fences in restore_process_bos
Signed-off-by: Felix Kuehling <Felix.Kuehling@xxxxxxx>
Acked-by: Christian König <christian.koenig@xxxxxxx>
Tested-by: Emily Deng <Emily.Deng@xxxxxxx>
Signed-off-by: Alex Deucher <alexander.deucher@xxxxxxx>
FWIW, I built a plain 6.6 kernel, and was not able to reproduce the
crash with some simple tests.
Regards,
Felix
So I agree, let's revert it.
Reviewed-by: Jay Cornwall <jay.cornwall@xxxxxxx>
