Re: [PATCH] drm/xe: Fix unnecessary host-side population of ttm_tt on non-TT resources

[Christian König]

On 9/2/26 10:58, Matthew Brost wrote:
> On Wed, Sep 02, 2026 at 06:54:55PM +1000, Dave Airlie wrote:
>> On Wed, 2 Sept 2026 at 18:08, Matthew Brost <matthew.brost@xxxxxxxxx> wrote:
>>>
>>> On Wed, Aug 26, 2026 at 07:01:21PM -0700, Matthew Brost wrote:
>>>
>>> Dave ping. Question below.
>>>
>>>> On Thu, Aug 20, 2026 at 10:34:49PM -0400, Nathan Bourgeois wrote:
>>>>>> Shouldn't we just be calling xe_bo_validate() here instead of
>>>>>> ttm_tt_populate? (With the correct xe_validation_guard() wrapping).
>>>>>
>>>>>> Yes, this might be a better solution, making ttm_bo_setup_export()
>>>>>> completely unnecessary.
>>>>>
>>>>> If ttm_bo_setup_export() is unnecessary, I'm happy to change the patch
>>>>> or make a new patch. I will attempt to implement and test this locally.
>>>>>
>>>>>> This part looks good as different patch from what I'm assuming will be a
>>>>>> TTM fix.
>>>>>
>>>>> Regarding this, what do you recommend I do, assuming the patch
>>>>> remains local to drm/xe? I'm still learning the ropes of contributing.
>>>>>
>>>>
>>>>
>>>> For Xe I believe Thomas and I aligned a xe_bo_validate with a correct
>>>> xe_validation_guard is the Xe preferred solution in the existing
>>>> design... But a question to Dave below before I commit to anything.
>>>>
>>>>> Nathan
>>>>>
>>>>> On Thu, Aug 20, 2026 at 9:08 PM Dave Airlie <airlied@xxxxxxxxx> wrote:
>>>>>>
>>>>>>> Yes, this might be a better solution, making ttm_bo_setup_export()
>>>>>>> completely unnecessary.
>>>>>>>
>>>>>>> It's also a bit odd that, in flows where we don't have backing storage
>>>>>>> on export, we populate with pages and charge the system memory cgroup,
>>>>>>> only to move the data to VRAM when the import attach is triggered,
>>>>>>> resulting in a copy and a change in cgroup charging.
>>>>>>>
>>>>>>> I guess the question is why was ttm_bo_setup_export() introduced over
>>>>>>> just a validation at export?
>>>>>>>
>>>>>>
>>>>>> I'd like to think I had an answer for that, but I don't. Likely
>>>>>> because I wasn't thinking about VRAM charging at all, and just
>>>>>> worrying about making sure we had populated some pages for system
>>>>>> memory ones, so the other side couldn't DoS us.
>>>>>>
>>>>
>>>> Dave:
>>>>
>>>> We don't charge any cgroups yet, right? This would only come into play
>>>> once a version of [1] merges, correct?
>>>>
>>>> What would prevent the pages populated for a TTM BO from being
>>>> immediately reclaimed and discarded? I'm fairly certain Xe's shrinker
>>>> could do exactly that, since we don't pin those pages. This seems to
>>>> imply that we'd need to store the cgroup associated with the TTM BO at
>>>> creation time and charge allocations to that cgroup, regardless of which
>>>> task ultimately triggers the page allocation.
>>
>> This was actually to fix a non-cgroup bug with a possible priority
>> inversion problems.
>>
>> i.e. a client could allocate a BO export it to a compositor, and then
>> the compositor would populate it for the first time and get ENOMEM.
>>
>> This was to avoid that case by making sure a client had tried to
>> allocate all the pages for the BO before exporting it, so it would get
>> the failure at that time.
>>
> 
> Ah, this makes more sense.

I was wondering why we did that as well, but yeah now I remember.

> 
>> I don't believe xe should just be reclaiming and discarding these
>> pages without swapping them to shmem first?
> 
> Yes, the pages would be in shmem if swapped.
> 
>>
>> Validating is probably fine as well.
>>
> 
> Nathan - the conclusion is validate in Xe.

I think we ran into a similar issue during some recent amdgpu work as well, so adding Felix on CC.

Basically we sometimes need empty BO objects without resource nor TT object to fill them with some resource object at a specific location.

So far we just gave them a dummy system placement for the initial validation, but that is far from ideal.

Any idea how to do that cleanly without breaking the compositor fix?

Thanks,
Christian.

> 
> Matt
> 
>> Dave.