mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Christian König" <christian.koenig@amd.com>
To: Matthew Brost <matthew.brost@intel.com>,
	Dave Airlie <airlied@gmail.com>,
	"Kuehling, Felix" <Felix.Kuehling@amd.com>
Cc: "Nathan Bourgeois" <iridescentrosesfall@gmail.com>,
	"Thomas Hellström" <thomas.hellstrom@linux.intel.com>,
	intel-xe@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
	linux-kernel@vger.kernel.org,
	"Rodrigo Vivi" <rodrigo.vivi@intel.com>,
	"Matthew Auld" <matthew.auld@intel.com>,
	"Simona Vetter" <simona@ffwll.ch>
Subject: Re: [PATCH] drm/xe: Fix unnecessary host-side population of ttm_tt on non-TT resources
Date: Wed, 2 Sep 2026 11:04:16 +0200	[thread overview]
Message-ID: <60f8132f-efc9-4dc1-bd02-b62391bf3fa0@amd.com> (raw)
In-Reply-To: <apflRScB/NTEiEzU@gsse-cloud1.jf.intel.com>

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@intel.com> 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@gmail.com> 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.


  reply	other threads:[~2026-09-02  9:04 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-20  3:19 Nathan Bourgeois
2026-08-20 17:03 ` Matthew Brost
     [not found]   ` <CAMpTW2cxRrG4oJ0HrRoKhHrVHxWO01-L+QDoWMLCAUaSfw75sA@mail.gmail.com>
2026-08-20 18:56     ` Nathan Bourgeois
2026-08-20 20:03       ` Thomas Hellström
2026-08-20 23:57         ` Matthew Brost
2026-08-21  1:08           ` Dave Airlie
2026-08-21  2:34             ` Nathan Bourgeois
2026-08-27  2:01               ` Matthew Brost
2026-09-02  8:08                 ` Matthew Brost
2026-09-02  8:54                   ` Dave Airlie
2026-09-02  8:58                     ` Matthew Brost
2026-09-02  9:04                       ` Christian König [this message]
2026-09-03  3:52                         ` Nathan Bourgeois

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=60f8132f-efc9-4dc1-bd02-b62391bf3fa0@amd.com \
    --to=christian.koenig@amd.com \
    --cc=Felix.Kuehling@amd.com \
    --cc=airlied@gmail.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=iridescentrosesfall@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=matthew.auld@intel.com \
    --cc=matthew.brost@intel.com \
    --cc=rodrigo.vivi@intel.com \
    --cc=simona@ffwll.ch \
    --cc=thomas.hellstrom@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®