Subject: Re: [PATCH v2 06/10] cgroup: Assign subsystem IDs during compile time

From: Tejun Heo <[email-masked]>
Date: Thu, 6 Sep 2012 15:32:38 -0700

Hello, Daniel.

On Sat, Aug 25, 2012 at 07:11:31PM +0200, Daniel Wagner wrote:
> On 25.08.2012 01:38, Tejun Heo wrote:
> >>task_cls_classid() and task_netprioidx() (when built as
> >>module) are protected by a jump label and therefore we can
> >>simply replace the subsystem index lookup with the enum.
> >
> >Can we put these in a separate patch?  ie. The first patch makes all
> >subsys IDs constant and then patches to simplify users.
> 
> Wouldn't this break bisection? I merged this step so that all steps
> in this series are able to compile and run.

I don't see why it should but maybe I'm missing something.  If so,
please enlighten me.

> >>+#define IS_SUBSYS_ENABLED(option) IS_MODULE(option)
> >>+#include <linux/cgroup_subsys.h>
> >>+#undef IS_SUBSYS_ENABLED
> >>+
> >
> >Why do we need to segregate in-kernel and modular ones at all?  What's
> >wrong with just defining them in one go?
> 
> I have done that but the result was a panic. There seems some code
> which expects this ordering. Let me dig into this and fix it.

Yes, please.

> >Hmm... patch sequence looks odd to me.  If you first make all IDs
> >constant, you can first remove module specific ones and then later add
> >jump labels as separate patches.  Wouldn't that be simpler?
> 
> As said above, I tried to keep all steps usable so bisection would
> work. I think your steps would lead to non working versions of the
> kernel.

Hmmm... Yes, it should be bisectable but again I don't see why being
biesectable interferes with the patch ordering here.

Thanks.

-- 
tejun