Re: [devel] [PATCH hasher-priv v1 3/3] Add cgroup support

[Arseny Maslennikov <arseny@altlinux.org>]

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

On Thu, Oct 01, 2020 at 09:17:33PM +0200, Alexey Gladkov wrote:
> On Thu, Sep 17, 2020 at 04:11:07PM +0300, Arseny Maslennikov wrote:
> > On Fri, Dec 13, 2019 at 12:42:05PM +0100, Alex Gladkov wrote:
> > > From: Alexey Gladkov <legion@altlinux.org>
> > > 
> > 
> > Could you please explain what you're trying to do with this patch?
> > Even if it's obvious from the source itself, we still must have an
> > opportunity to discuss, and a decent explanation should stay in the
> > project history.
> 
> I think this patch is simple enough.

There's a misunderstanding here. I'm not asking to explain the
semantics (what this patch does) — I repeat, it's rather obvious from
the source itself, the patch is indeed simple. I'm trying to get how the
patch's author would describe the pragmatic value of this patch. IOW:
we see this patch does XXX. What, in Alexey's view, are we trying to
achieve by implementing XXX?

Descriptive commit messages are done (and are enforced in successful
communities, e. g. LKML) for a reason.

The above essentially is my previous comment here, reworded and clarified.

If for some reason you believe it's shameful or rude to the community to
"waste time" on textual explanations, fair enough — I'll maybe write a commit
message myself (with my take on why this might be useful) and then most
likely ACK the same patch, with authorship reattributed to you via From:
in the patch body and the new commit message. Or else NAK this
particular revision with an empty commit message and leave it up to
ldv@.
If it were up to me, I would not approve of empty commit messages in a
lasting, crucial project like hasher-privd. People are forgetful, and
commit messages exist to help.

> 
> > Most likely, it'll turn out we _at least_ have to pass Delegate=yes to
> > the systemd service:
> > 
> >        Delegate=
> >            Turns on delegation of further resource control
> >            partitioning to processes of the unit. Units where
> >            this is enabled may create and manage their own
> >            private subhierarchy of control groups below the
> >            control group of the unit itself.
> > Manual page systemd.resource-control(5): lines 786-791
> 
> I'm pretty sure the hasher-priv shouldn't be tied to systemd.

I agree.

> I'm also
> convinced that the server will not be tied. The hasher-privd must be able
> to run on systems without systemd.

Sure it must. No one is trying to drop support for anything-but-systemd
from hasher-privd.

Here I was talking about the operational details of the daemon running
_under_ systemd, not without systemd. The insight about Delegate=yes
does not interfere with non-systemd installations.

> 
> > Do we only support cgroup2 and ignore cgroup1? If yes, great, but
> > perhaps then we might want to have a setting to not fiddle with cgroup
> > trees, to support the unfortunate users that have to run Docker and
> > other garbage.
> 
> Yeah, I didn't plan on supporting legacy version of cgroups. Docker
> already can work with cgroupsv2.

Oh, I heard they were just recently working on cgroup2 support.
Okay.


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