Comment 5 for bug 829628

Revision history for this message
Serge Hallyn (serge-hallyn) wrote : Re: [Bug 829628] Re: [FFE] Add cgroup-lite package

Quoting StefanPotyra (<email address hidden>):

Thanks for looking, Stefan.

> This looks odd:
>
> +cd /sys/fs/cgroup
> +for d in `/bin/ls`; do
> ________
> + umount $d || true
> +done

Thanks for drawing my attention to this. This is indeed a problem. As an example, I did:

 (as root) mount --bind /dev/pts /mnt
 (as serge) (cd /sys/fs/cgroup; ln -s /mnt)
 (as root) umount /sys/fs/cgroup/mnt

and /mnt got umounted.

Of course serge shouldn't have been able to create a link under /sys/fs/cgroup
in the first place. Stephane, I'm actually out this week, I'll push a fix next
week, but if you have time to post a debdiff this week, that'd be great.

As for the "for d in `/bin/ls`; do" bit, what exactly looks odd about it? I do
it all the time by hand, but that doesn't mean it shouldn't be changed here...

I had considered specifying a (configurable) list of cgroups to mount,
and then only umount those. I.e.

CGROUPS="memory devices cpu freezer"

at top of the upstart job. Maybe 'all' option which then grabs all from 'cat
/proc/cgroups". Would that be preferable?

> Is the following sufficient (no clue really, I've never been too intimate with upstart scripts)?
> +start on mounted MOUNTPOINT=/sys

The cgroups get mounted under /sys/fs/cgroup, and have no userspace
dependencies, so it should be sufficient.

> apart from that, I don't see if any mentioned bugs have been forwarded
> to upstream. Can you shed some light here please? Seeing upstream

The bug about cgclear failing (so that the job fails to stop, and the
package can't be removed) was mentioned upstream, but I don't believe
they've pushed a patch for it yet. They announced in the last few days
that they expect all functionality to be pulled into systemd, and therefore
will stop developing.

> response (or even lack thereof) would certainly help to sort this
> request.

Note we don't intend to stop support of cgroup-bin (now), and want to fix
those bugs. However, some of the bugs are hard, and some I argue are
impossible, due to design flaws: the kernel cgroup API simply does not
provide for a race-free way for a daemon to reliably reclassify programs
as they start up.

This is an important point. It is why the libcgroup upstream is moving
the functionality into systemd. For us, the functionality belongs in
upstart (for initialization and service startup) and pam (for logins).

> If the issue at hand was only boot speed, I'd probably wave through this
> request as all changes look sensible to me.

IMO it's both boot speed and not running extraneous daemons on your
system.

thanks for your time,
-serge