Re: [PATCH v2 0/7] kernel/cgroups: Add "dmem" memory accounting cgroup.

[Maxime Ripard <mripard@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 1848 bytes --]

On Fri, Dec 13, 2024 at 03:53:13PM +0100, Maarten Lankhorst wrote:
> 
> 
> Den 2024-12-13 kl. 14:03, skrev Maxime Ripard:
> > Hi,
> > 
> > Thanks for the new update!
> > 
> > On Wed, Dec 04, 2024 at 02:44:00PM +0100, Maarten Lankhorst wrote:
> > > New update. Instead of calling it the 'dev' cgroup, it's now the
> > > 'dmem' cgroup.
> > > 
> > > Because it only deals with memory regions, the UAPI has been updated
> > > to use dmem.min/low/max/current, and to make the API cleaner, the
> > > names are changed too.
> > 
> > The API is much nicer, and fits much better into other frameworks too.
> > 
> > > dmem.current could contain a line like:
> > > "drm/0000:03:00.0/vram0 1073741824"
> > > 
> > > But I think using "drm/card0/vram0" instead of PCIID would perhaps be
> > > good too. I'm open to changing it to that based on feedback.
> > 
> > Do we have any sort of guarantee over the name card0 being stable across
> > reboots?
> > 
> > I also wonder if we should have a "total" device that limits the amount
> > of memory we can allocate from any region?
>
> I don't think it is useful. Say your app can use 1 GB of main memory or 2 GB
> of VRAM, it wouldn't make sense to limit the total of those. In a lot of
> cases there is only 1 region, so the total of that would still be the same.
> 
> On top, we just separated the management of each region, adding a 'total'
> would require unseparating it again. :-)

I didn't mean the total for a device, but for the system. It would
definitely not make sense for a VRAM, but for CMA for example, you have
a single, limited, allocator that will be accessible from heaps, v4l2
and DRM devices.

If an application has to allocate both from v4l2 and DRM buffers, we
should be able to limit its total usage of CMA, not just on a single
device.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]