[rfc] drm/ttm/memcg: simplest initial memcg/ttm integration (v2)

[Dave Airlie]

On Mon, 19 May 2025 at 02:28, Christian König <christian.koenig at amd.com> wrote:
>
> On 5/16/25 22:25, Dave Airlie wrote:
> > On Sat, 17 May 2025 at 06:04, Johannes Weiner <hannes at cmpxchg.org> wrote:
> >>> The memory properties are similar to what GFP_DMA or GFP_DMA32
> >>> provide.
> >>>
> >>> The reasons we haven't moved this into the core memory management is
> >>> because it is completely x86 specific and only used by a rather
> >>> specific group of devices.
> >>
> >> I fully understand that. It's about memory properties.
> >>
> >> What I think you're also saying is that the best solution would be
> >> that you could ask the core MM for pages with a specific property, and
> >> it would hand you pages that were previously freed with those same
> >> properties. Or, if none such pages are on the freelists, it would grab
> >> free pages with different properties and convert them on the fly.
> >>
> >> For all intents and purposes, this free memory would then be trivially
> >> fungible between drm use, non-drm use, and different cgroups - except
> >> for a few CPU cycles when converting but that's *probably* negligible?
> >> And now you could get rid of the "hack" in drm and didn't have to hang
> >> on to special-property pages and implement a shrinker at all.
> >>
> >> So far so good.
> >>
> >> But that just isn't the implementation of today. And the devil is very
> >> much in the details with this:
> >>
> >> Your memory attribute conversions are currently tied to a *shrinker*.
> >>
> >> This means the conversion doesn't trivially happen in the allocator,
> >> it happens from *reclaim context*.
>
> Ah! At least I now understand your concern here.
>
> >> Now *your* shrinker is fairly cheap to run, so I do understand when
> >> you're saying in exasperation: We give this memory back if somebody
> >> needs it for other purposes. What *is* the big deal?
> >>
> >> The *reclaim context* is the big deal. The problem is *all the other
> >> shrinkers that run at this time as well*. Because you held onto those
> >> pages long enough that they contributed to a bonafide, general memory
> >> shortage situation. And *that* has consequences for other cgroups.
>
> No it doesn't, or at least not as much as you think.
>
> We have gone back and forth on this multiple times already when discussion the shrinker implementations. See the DRM mailing list about both the TTM and the GEM shared mem shrinker.
>
> The TTM pool shrinker is basically just a nice to have feature which is used to avoid deny of service attacks and allows to kick in when use cases change. E.g. between installing software (gcc) and running software (Blender, ROCm etc..).
>
> In other words the TTM shrinker is not even optimized and spends tons of extra CPU cycles because the expectation is that it never really triggers in practice.
>
> > I think this is where we have 2 options:
> > (a) moving this stuff into core mm and out of shrinker context
> > (b) fix our shrinker to be cgroup aware and solve that first.
>
> (c) give better priorities to the shrinker API.
>
> E.g. the shrinker for example assumes that the users of the API must scan the pages to be able to clean them up.

Well my again naive approach is to just add simpler low-overhead
shrinkers to the start of the shrinker list and if they free up enough
memory then win, otherwise we were in reclaim anyways,

however this asks the question if just going into reclaim and having
to touch any shrinkers at all is bad, if the overheads of just doing
that aren't acceptable then we would need to come up with a better way
I suspect?

adding a single shrinker flag to put the ttm shrinker at the top of
the list is pretty trivial.

Thanks for use-cases that probably matter, I can see the online gaming
workloads being useful overhead reduction.

There probably isn't much appetite to just migrate the ttm pools into
the core mm, I see a couple of other users like sound do set_memory_*
calls, but I doubt they are on the radar for how much it costs.

Dave.