On Wed, Sep 02, 2026 at 06:54:55PM +1000, Dave Airlie wrote: > On Wed, 2 Sept 2026 at 18:08, Matthew Brost <[email protected]> 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 <[email protected]> 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 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. Matt > Dave.
