[lxc-devel] RFC: cgroups aware proc

[Serge Hallyn]

Quoting Marian Marinov (mm at yuhu.biz):
> On 01/07/2014 01:17 PM, Li Zefan wrote:
> >On 2014/1/5 8:12, Marian Marinov wrote:
> >>Happy new year guys.
> >>
> >>I need to have /proc cgroups aware, as I want to have LXC containers that see only the resources that are given to them.
> >>
> >>In order to do that I had to patch the kernel. I decided to start with cpuinfo, stat and interrupts and then continue
> >>with meminfo and loadavg.
> >>
> >>I managed to patch the Kernel (linux 3.12.0) and make /proc/cpuinfo, /proc/stat and /proc/interrupts be cgroups aware.
> >>
> >>Attached are the patches that make the necessary changes.
> >>
> >>The change for /proc/cpuinfo and /proc/interrupts is currently done only for x86 arch, but I will patch the rest of the
> >>architectures if the style of the patches is acceptable.
> >>
> >>Tomorrow I will check if the patches apply and build with the latest kernel.
> >>
> >
> >People tried to do this before, but got rejected by upstream maintainers,
> >and then the opinion was to do this in userspace throught FUSE.
> >
> >Seems libvirt-lxc already supports containerized /proc/meminfo in this way.
> >See:
> >	http://libvirt.org/drvlxc.html
> 
> I'm well aware of the FUSE approach and the fact that the kernel
> maintainers do not accept the this kind of changing the kernel but
> the simple truth is that FUSE is too have for this thing.
> 
> I'm setting up a repo on GitHub which will hold all the patches for

Thanks, that'll be easier to look at than the in-line patches.

>From my very quick look, I would recommend

1. coming up with some helpers to reduce the degree to which you are
negatively affecting the flow of the existing code.  Currently it
looks like you're obfuscating it a lot, and I think you can make it
so only a few clean lines are added per function.

For instance, in arch_show_interrupts(), instead of plopping

+#ifdef CONFIG_CPUSETS
+               if (tsk != NULL && cpumask_test_cpu(j, &tsk->cpus_allowed))
+#endif

in several places,

write
static inline bool task_has_cpu(tsk, cpu) {
#ifdef CONFIG_CPUSETS
        return (tsk != NULL && cpumask_test_cpu(cpu, &tsk->cpus_allowed));
#else
	return true;
#endif
}

and then just use 'if task_has_cpu(tsk, j)' several times.


2. showing performance degredation in the not-using-it case (that is,
with cgroups enabled but in the root cpuset for instance), which
hopefully will be near-nil.

If you can avoid confounding the readability of the code and not impact
the performance, that'll help your chances a lot.

> this and will keep updating it even if it is not accepted by the
> upstream maintainers. I'll give you the link within a few days.
> 
> I have already finished with CPU and Memory... the only thing that
> is left is the /proc/loadavg, which will take more time, but will be
> done.
> 
> I hope some of the scheduler maintainers at least to give me some comments on the patches that I have done.
> 
> Marian
> 
> >
> >
> >
>