From: Daniel Wagner <wagi-kQCPcA+X3s7YtjvyW6yDsg@public•gmane.org>
To: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public•gmane.org>
Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public•gmane.org,
cgroups-u79uwXL29TY76Z2rM5mHXA@public•gmane.org,
"David S. Miller" <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public•gmane.org>,
Andrew Morton
<akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public•gmane.org>,
Eric Dumazet <edumazet-hpIqsD4AKlfQT0dZR+AlfA@public•gmane.org>,
Gao feng <gaofeng-BthXqXjhjHXQFUHtdCDX3A@public•gmane.org>,
Glauber Costa <glommer-bzQdu9zFT3WakBO8gow8eQ@public•gmane.org>,
Jamal Hadi Salim <jhs-jkUAjuhPggJWk0Htik3J/w@public•gmane.org>,
John Fastabend
<john.r.fastabend-ral2JQCrhuEAvxtiuMwx3w@public•gmane.org>,
Kamezawa Hiroyuki
<kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public•gmane.org>,
Li Zefan <lizefan-hv44wF8Li93QT0dZR+AlfA@public•gmane.org>,
Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public•gmane.org>
Subject: Re: [PATCH v2 06/10] cgroup: Assign subsystem IDs during compile time
Date: Sat, 25 Aug 2012 19:11:31 +0200 [thread overview]
Message-ID: <50390743.2090203@monom.org> (raw)
In-Reply-To: <20120824233810.GT21325-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
On 25.08.2012 01:38, Tejun Heo wrote:
> Hello,
>
> On Fri, Aug 24, 2012 at 04:01:40PM +0200, Daniel Wagner wrote:
>> We are able to safe some space when we assign the subsystem
> ^ ^
> save if we assign both builtin and
> module susbsystem IDs at compile time?
>
>> IDs at compile time. Instead of allocating per cgroup
>> cgroup->subsys[CGROUP_SUBSYS_COUNT] where CGROUP_SUBSYS_COUNT is
>> always 64, we allocate at max 12 (at this point there are 12
>> subsystem).
>
> Please note (in big fat ugly way) that this disallows support for
> modular controller which isn't known at kernel compile time.
yep, will do
>> 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.
>> diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
>> index 3787872..ada517f 100644
>> --- a/include/linux/cgroup.h
>> +++ b/include/linux/cgroup.h
>> @@ -43,18 +43,27 @@ extern void cgroup_unload_subsys(struct cgroup_subsys *ss);
>>
>> extern const struct file_operations proc_cgroup_operations;
>>
>> -/* Define the enumeration of all builtin cgroup subsystems */
>> -#define SUBSYS(_x) _x ## _subsys_id,
>> +/*
>> + * Define the enumeration of all builtin cgroup subsystems.
>> + * For the builtin subsystems the subsys_id needs to be indentical
>> + * with the index in css->subsys. Therefore, all the builtin
>> + * subsys are listed first and then the modules ids.
>> + */
>> enum cgroup_subsys_id {
>> +#define SUBSYS(_x) _x ## _subsys_id,
>> +
>> +#define IS_SUBSYS_ENABLED(option) IS_BUILTIN(option)
>> #include <linux/cgroup_subsys.h>
>> -};
>> +#undef IS_SUBSYS_ENABLED
>> +
>> +#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.
>> diff --git a/include/net/cls_cgroup.h b/include/net/cls_cgroup.h
>> index 24443d2..43fae13 100644
>> --- a/include/net/cls_cgroup.h
>> +++ b/include/net/cls_cgroup.h
>> @@ -49,22 +49,16 @@ static inline u32 task_cls_classid(struct task_struct *p)
>> extern struct static_key cgroup_cls_enabled;
>> #define clscg_enabled static_key_false(&cgroup_cls_enabled)
>>
>> -extern int net_cls_subsys_id;
>> -
>> static inline u32 task_cls_classid(struct task_struct *p)
>> {
>> - int id;
>> - u32 classid = 0;
>> + u32 classid;
>>
>> if (!clscg_enabled || in_interrupt())
>> return 0;
>>
>> rcu_read_lock();
>> - id = rcu_dereference_index_check(net_cls_subsys_id,
>> - rcu_read_lock_held());
>> - if (id >= 0)
>> - classid = container_of(task_subsys_state(p, id),
>> - struct cgroup_cls_state, css)->classid;
>> + classid = container_of(task_subsys_state(p, net_cls_subsys_id),
>> + struct cgroup_cls_state, css)->classid;
>> rcu_read_unlock();
>>
>> return classid;
>
> 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.
next prev parent reply other threads:[~2012-08-25 17:11 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-08-24 14:01 [PATCH v2 00/10] cgroup: Assign subsystem IDs during compile time Daniel Wagner
2012-08-24 14:01 ` [PATCH v2 01/10] cgroup: net_cls: Use empty task_cls_classid() when !CONFIG_NET_CLS(_MODULE) Daniel Wagner
[not found] ` <1345816904-21745-2-git-send-email-wagi-kQCPcA+X3s7YtjvyW6yDsg@public.gmane.org>
2012-08-24 23:22 ` Tejun Heo
2012-08-27 2:32 ` Li Zefan
2012-08-24 14:01 ` [PATCH v2 03/10] cgroup: net_cls: Protect access to task_cls_classid() when built as module Daniel Wagner
[not found] ` <1345816904-21745-4-git-send-email-wagi-kQCPcA+X3s7YtjvyW6yDsg@public.gmane.org>
2012-08-24 23:26 ` Tejun Heo
2012-08-25 16:56 ` Daniel Wagner
[not found] ` <503903BD.6020208-kQCPcA+X3s7YtjvyW6yDsg@public.gmane.org>
2012-08-28 14:47 ` Paul E. McKenney
[not found] ` <20120828144725.GR2961-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
2012-08-28 16:41 ` Daniel Wagner
2012-08-24 14:01 ` [PATCH v2 05/10] cgroup: Remove CGROUP_BUILTIN_SUBSYS_COUNT Daniel Wagner
[not found] ` <1345816904-21745-6-git-send-email-wagi-kQCPcA+X3s7YtjvyW6yDsg@public.gmane.org>
2012-08-24 23:28 ` Tejun Heo
[not found] ` <20120824232840.GS21325-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2012-08-25 16:59 ` Daniel Wagner
[not found] ` <5039046D.1040402-kQCPcA+X3s7YtjvyW6yDsg@public.gmane.org>
2012-09-06 22:23 ` Tejun Heo
2012-08-24 14:01 ` [PATCH v2 06/10] cgroup: Assign subsystem IDs during compile time Daniel Wagner
[not found] ` <1345816904-21745-7-git-send-email-wagi-kQCPcA+X3s7YtjvyW6yDsg@public.gmane.org>
2012-08-24 23:38 ` Tejun Heo
[not found] ` <20120824233810.GT21325-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2012-08-25 17:11 ` Daniel Wagner [this message]
[not found] ` <50390743.2090203-kQCPcA+X3s7YtjvyW6yDsg@public.gmane.org>
2012-09-06 22:32 ` Tejun Heo
2012-08-24 14:01 ` [PATCH v2 08/10] cgroup: net_cls: Merge builtin and module version of task_cls_classid() Daniel Wagner
[not found] ` <1345816904-21745-9-git-send-email-wagi-kQCPcA+X3s7YtjvyW6yDsg@public.gmane.org>
2012-08-24 23:41 ` Tejun Heo
[not found] ` <1345816904-21745-1-git-send-email-wagi-kQCPcA+X3s7YtjvyW6yDsg@public.gmane.org>
2012-08-24 14:01 ` [PATCH v2 02/10] cgroup: net_cls: Move sock_update_classid() decleration to cls_cgroup.h Daniel Wagner
2012-08-24 14:01 ` [PATCH v2 04/10] cgroup: net_prio: Protect access to task_netprioidx() when built as module Daniel Wagner
[not found] ` <1345816904-21745-5-git-send-email-wagi-kQCPcA+X3s7YtjvyW6yDsg@public.gmane.org>
2012-08-24 23:26 ` Tejun Heo
2012-08-24 14:01 ` [PATCH v2 07/10] cgroup: net_cls: Simplify ifdef logic Daniel Wagner
[not found] ` <1345816904-21745-8-git-send-email-wagi-kQCPcA+X3s7YtjvyW6yDsg@public.gmane.org>
2012-08-24 23:39 ` Tejun Heo
2012-08-24 14:01 ` [PATCH v2 09/10] cgroup: net_prio: " Daniel Wagner
[not found] ` <1345816904-21745-10-git-send-email-wagi-kQCPcA+X3s7YtjvyW6yDsg@public.gmane.org>
2012-08-24 23:42 ` Tejun Heo
2012-08-24 14:01 ` [PATCH v2 10/10] cgroup: net_prio: Merge builtin and module version of task_netprioidx() Daniel Wagner
2012-08-24 15:01 ` [PATCH v2 00/10] cgroup: Assign subsystem IDs during compile time Daniel Wagner
2012-08-24 23:15 ` Tejun Heo
[not found] ` <20120824231528.GO21325-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2012-08-25 16:49 ` Daniel Wagner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=50390743.2090203@monom.org \
--to=wagi-kqcpca+x3s7ytjvyw6ydsg@public$(echo .)gmane.org \
--cc=akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public$(echo .)gmane.org \
--cc=cgroups-u79uwXL29TY76Z2rM5mHXA@public$(echo .)gmane.org \
--cc=davem-fT/PcQaiUtIeIZ0/mPfg9Q@public$(echo .)gmane.org \
--cc=edumazet-hpIqsD4AKlfQT0dZR+AlfA@public$(echo .)gmane.org \
--cc=gaofeng-BthXqXjhjHXQFUHtdCDX3A@public$(echo .)gmane.org \
--cc=glommer-bzQdu9zFT3WakBO8gow8eQ@public$(echo .)gmane.org \
--cc=jhs-jkUAjuhPggJWk0Htik3J/w@public$(echo .)gmane.org \
--cc=john.r.fastabend-ral2JQCrhuEAvxtiuMwx3w@public$(echo .)gmane.org \
--cc=kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public$(echo .)gmane.org \
--cc=lizefan-hv44wF8Li93QT0dZR+AlfA@public$(echo .)gmane.org \
--cc=netdev-u79uwXL29TY76Z2rM5mHXA@public$(echo .)gmane.org \
--cc=nhorman-2XuSBdqkA4R54TAoqtyWWQ@public$(echo .)gmane.org \
--cc=tj-DgEjT+Ai2ygdnm+yROfE0A@public$(echo .)gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox