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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241213-flying-naughty-marmot-6a82a2@houat>
Date: Fri, 13 Dec 2024 16:22:40 +0100
From: Maxime Ripard <mripard@...nel.org>
To: Maarten Lankhorst <dev@...khorst.se>
Cc: Friedrich Vock <friedrich.vock@....de>, linux-kernel@...r.kernel.org, 
	intel-xe@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org, Tejun Heo <tj@...nel.org>, 
	Zefan Li <lizefan.x@...edance.com>, Johannes Weiner <hannes@...xchg.org>, 
	Andrew Morton <akpm@...ux-foundation.org>, cgroups@...r.kernel.org, linux-mm@...ck.org, 
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>
Subject: Re: [PATCH v2 0/7] kernel/cgroups: Add "dmem" memory accounting
 cgroup.

On Fri, Dec 13, 2024 at 03:13:23PM +0100, Maarten Lankhorst wrote:
> Hey,
> 
> Den 2024-12-13 kl. 14:07, skrev Maxime Ripard:
> > On Sun, Dec 08, 2024 at 01:15:34PM +0100, Friedrich Vock wrote:
> > > Hi,
> > > 
> > > On 04.12.24 14:44, Maarten Lankhorst wrote:
> > > 
> > > > 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.
> > > > 
> > > > 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.
> > > 
> > > Agree, allowing userspace to reference DRM devices via "cardN" syntax
> > > sounds good.
> > > 
> > > What about other subsystems potentially using dmem cgroups?
> > > I'm not familiar with the media subsystem, but I imagine we might be
> > > dealing with things like USB devices there? Is something like a
> > > "deviceN" possible there as well, or would device IDs look completely
> > > different?
>
> I'd just take what makes sense for each driver. dev_name() would be a good
> approximation.

Yeah, dev_name() seems good enough to me too.

> I agree that cardN is not stable.
> 
> > > I have some patches to enable the cgroup in GEM-based drivers, media
> > ones and dma-buf heaps. The dma-buf heaps are simple enough since the
> > heaps names are supposed to be stable.
> 
> I've used your patch as a base enable cgroup in drivers that use the VRAM
> manager. I didn't want to enable it for all of GEM, because it would
> conflict with drivers using TTM. Some more discussion is needed first.
> 
> For DMA-BUF heaps, I think it's fine and there is a lot less need of
> discussion. I just felt it should be sent separately from the initial
> enablement.

Definitely.

> > I don't think using card0 vs card1 (or v4l0 vs v4l1 for example) will
> > work because I don't think we have any sort of guarantee that these
> > names will always point to the same devices across reboots or updates.
> > 
> > If the module is loaded later than it used to for example, we could very
> > well end up in a situation where card0 and card1 are swapped, while the
> > constraints apply to the previous situation.
>
> I agree, just put it out there for discussion. I don't think the benefits
> weigh up against the downsides :-)

Oh absolutely. The way to define a stable name is going to be framework
specific anyway. My point was that we wanted to have a stable name.

Maxime

Download attachment "signature.asc" of type "application/pgp-signature" (274 bytes)