mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Matthew Brost <matthew.brost@intel.com>
To: Nathan Bourgeois <iridescentrosesfall@gmail.com>,
	Dave Airlie <airlied@gmail.com>
Cc: "Dave Airlie" <airlied@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>,
	"Christian König" <christian.koenig@amd.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 01:08:16 -0700	[thread overview]
Message-ID: <apfZcEF8IlDak/As@gsse-cloud1.jf.intel.com> (raw)
In-Reply-To: <ao+acQG/wyZYNYfW@gsse-cloud1.jf.intel.com>

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.
> 
> Another option, instead of validating, is to revert
> ttm_bo_setup_export() entirely and rethink the overall approach as part
> of [1].
> 
> Matt
> 
> [1] https://patchwork.freedesktop.org/series/169831/
> 
> > > Dave.

  reply	other threads:[~2026-09-02  8:08 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 [this message]
2026-09-02  8:54                   ` Dave Airlie
2026-09-02  8:58                     ` Matthew Brost
2026-09-02  9:04                       ` Christian König
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=apfZcEF8IlDak/As@gsse-cloud1.jf.intel.com \
    --to=matthew.brost@intel.com \
    --cc=airlied@gmail.com \
    --cc=christian.koenig@amd.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=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®