linux-kernel - Re: [PATCH v2 1/4] riscv: defconfig: drop RT_GROUP

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] [day] [month] [year] [list]

Message-ID: <20240910125729.480594-2-CoelacanthusHex@gmail.com>
Date: Tue, 10 Sep 2024 20:57:30 +0800
From: Celeste Liu <coelacanthushex@...il.com>
To: conor.dooley@...rochip.com
Cc: anup@...infault.org,
	aou@...s.berkeley.edu,
	bcm-kernel-feedback-list@...adcom.com,
	chenhuacai@...nel.org,
	coelacanthushex@...il.com,
	dalias@...c.org,
	florian.fainelli@...adcom.com,
	glaubitz@...sik.fu-berlin.de,
	guoren@...nel.org,
	heinrich.schuchardt@...onical.com,
	jonathanh@...dia.com,
	kernel@...0n.name,
	linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org,
	linux-omap@...r.kernel.org,
	linux-riscv@...ts.infradead.org,
	linux-rpi-kernel@...ts.infradead.org,
	linux-sh@...r.kernel.org,
	linux-tegra@...r.kernel.org,
	linux@...linux.org.uk,
	loongarch@...ts.linux.dev,
	palmer@...belt.com,
	palmer@...osinc.com,
	paul.walmsley@...ive.com,
	rjui@...adcom.com,
	sbranden@...adcom.com,
	thierry.reding@...il.com,
	tony@...mide.com,
	ysato@...rs.sourceforge.jp,
	Celeste Liu <CoelacanthusHex@...il.com>
Subject: Re: [PATCH v2 1/4] riscv: defconfig: drop RT_GROUP_SCHED=y

On 2024-09-09 20:16, Conor Dooley wrote:
> On Mon, Sep 09, 2024 at 08:14:16PM +0800, Celeste Liu wrote:
> > On 2024-09-09 19:53, Conor Dooley wrote:
> > 
> > > On Fri, Aug 23, 2024 at 01:43:26AM +0800, Celeste Liu wrote:
> > >> Commit ba6cfef057e1 ("riscv: enable Docker requirements in defconfig")
> > >> introduced it because of Docker, but Docker has removed this requirement
> > >> since [1] (2023-04-19).
> > >>
> > >> For cgroup v1, if turned on, and there's any cgroup in the "cpu" hierarchy it
> > >> needs an RT budget assigned, otherwise the processes in it will not be able to
> > >> get RT at all. The problem with RT group scheduling is that it requires the
> > >> budget assigned but there's no way we could assign a default budget, since the
> > >> values to assign are both upper and lower time limits, are absolute, and need to
> > >> be sum up to < 1 for each individal cgroup. That means we cannot really come up
> > >> with values that would work by default in the general case.[2]
> > >>
> > >> For cgroup v2, it's almost unusable as well. If it turned on, the cpu controller
> > >> can only be enabled when all RT processes are in the root cgroup. But it will
> > >> lose the benefits of cgroup v2 if all RT process were placed in the same cgroup.
> > >>
> > >> Red Hat, Gentoo, Arch Linux and Debian all disable it. systemd also doesn't
> > >> support it.[3]
> > >>
> > >> [1]: https://github.com/moby/moby/commit/005150ed69c540fb0b5323e0f2208608c1204536
> > >> [2]: https://bugzilla.redhat.com/show_bug.cgi?id=1229700
> > >> [3]: https://github.com/systemd/systemd/issues/13781#issuecomment-549164383
> > >>
> > >> Fixes: ba6cfef057e1 ("riscv: enable Docker requirements in defconfig")
> > > 
> > > I don't think this fixes tag is suitable, the commit you cite in
> > > moby/docker is a year younger than the one in the fixes tag, so it was
> > > correct at the time it was written. I think the fixes tag should just be
> > > removed, since that commit was not wrong. Or am I missing something?
> > 
> > The Docker commit I cited introduce the fix, not the bug. So it should be later
> > than commit in fixes tag, otherwise Heinrich Schuchardt won't pick the wrong
> > config from upstream.
> > 
> > Timeline is:
> > 1) 2015-06-19 Qiang Huang introduce RT_GROUP_SCHED check in Docker.[1]
> > 2) 2022-06-08 Heinrich Schuchardt pick this to defconfig in commit
> >               in fixes tag.
> > 3) 2023-04-19 Florian Schmaus remove this requirement from Docker in
> >               commit I cited.
> 
> Yes, this is the way I understood things to be. IOW, when Heinrich wrote
> ba6cfef057e1 ("riscv: enable Docker requirements in defconfig"), it was
> a requirement for docker. The requirement later being removed doesn't
> make his patch incorrect, which is why I don't think this is a fix.

v3 has been sent.