On Thu, Jan 07, 2010 at 02:42:19PM +0800, Li Zefan wrote:
> KAMEZAWA Hiroyuki wrote:
> > On Wed, 6 Jan 2010 20:26:06 -0500
> > Ben Blum <[email protected]> wrote:
> >
> >> On Wed, Jan 06, 2010 at 04:04:14PM -0800, Andrew Morton wrote:
> >>> On Thu, 31 Dec 2009 00:10:50 -0500
> >>> Ben Blum <[email protected]> wrote:
> >>>
> >>>> This patch series implements support for building, loading, and
> >>>> unloading subsystems as modules, both within and outside the kernel
> >>>> source tree. It provides an interface cgroup_load_subsys() and
> >>>> cgroup_unload_subsys() which modular subsystems can use to register and
> >>>> depart during runtime. The net_cls classifier subsystem serves as the
> >>>> example for a subsystem which can be converted into a module using these
> >>>> changes.
> >>> What is the value in this? What are the usage scenarios? Why does the
> >>> benefit of this change exceed the cost/risk/etc of merging it?
> >> As discussed in the first posting of these patches, this provides the
> >> ability for arbitrary subsystems to be used with cgroups.. cls_cgroup
> >> would have already been a module except for a lack of support from
> >> cgroups, and the change also allows other module-loadable classifiers
> >> to add subsystems of their own.
> >
> > Hmm, do you have your own module in plan ?
> >
>
> Maybe the new blkio_cgroup can also be made module-able.

Ok, the following two patches make this happen (or at least pretend to
well enough to fool me). The first one adds use_id initialization in
cgroup_load_subsys, and the second rearranges config options and some
code as appropriate in block/ and adds EXPORT_SYMBOLs in cgroup.c.

-- bblum

---
block/Kconfig | 2 -
block/Kconfig.iosched | 2 -
block/blk-cgroup.c | 53 +++++++++++++++++++++++++++++++++++-----------
block/blk-cgroup.h | 10 ++++++--
include/linux/iocontext.h | 2 -
kernel/cgroup.c | 31 +++++++++++++++++++++-----
6 files changed, 77 insertions(+), 23 deletions(-)

2010-01-08 05:29:37

by Ben Blum

[permalink] [raw]

Subject: [RFC] [PATCH 1/2] cgroups: modular subsystems support for use_id

On Fri, Jan 08, 2010 at 12:27:34AM -0500, Ben Blum wrote:
> On Thu, Jan 07, 2010 at 02:42:19PM +0800, Li Zefan wrote:
> > KAMEZAWA Hiroyuki wrote:
> > > On Wed, 6 Jan 2010 20:26:06 -0500
> > > Ben Blum <[email protected]> wrote:
> > >
> > >> On Wed, Jan 06, 2010 at 04:04:14PM -0800, Andrew Morton wrote:
> > >>> On Thu, 31 Dec 2009 00:10:50 -0500
> > >>> Ben Blum <[email protected]> wrote:
> > >>>
> > >>>> This patch series implements support for building, loading, and
> > >>>> unloading subsystems as modules, both within and outside the kernel
> > >>>> source tree. It provides an interface cgroup_load_subsys() and
> > >>>> cgroup_unload_subsys() which modular subsystems can use to register and
> > >>>> depart during runtime. The net_cls classifier subsystem serves as the
> > >>>> example for a subsystem which can be converted into a module using these
> > >>>> changes.
> > >>> What is the value in this? What are the usage scenarios? Why does the
> > >>> benefit of this change exceed the cost/risk/etc of merging it?
> > >> As discussed in the first posting of these patches, this provides the
> > >> ability for arbitrary subsystems to be used with cgroups.. cls_cgroup
> > >> would have already been a module except for a lack of support from
> > >> cgroups, and the change also allows other module-loadable classifiers
> > >> to add subsystems of their own.
> > >
> > > Hmm, do you have your own module in plan ?
> > >
> >
> > Maybe the new blkio_cgroup can also be made module-able.
>
> Ok, the following two patches make this happen (or at least pretend to
> well enough to fool me). The first one adds use_id initialization in
> cgroup_load_subsys, and the second rearranges config options and some
> code as appropriate in block/ and adds EXPORT_SYMBOLs in cgroup.c.
>
> -- bblum
>
> ---
> block/Kconfig | 2 -
> block/Kconfig.iosched | 2 -
> block/blk-cgroup.c | 53 +++++++++++++++++++++++++++++++++++-----------
> block/blk-cgroup.h | 10 ++++++--
> include/linux/iocontext.h | 2 -
> kernel/cgroup.c | 31 +++++++++++++++++++++-----
> 6 files changed, 77 insertions(+), 23 deletions(-)
>
>

Attachments:

(No filename) (2.14 kB)
cgroups-module-use_id-support.patch (2.38 kB)
Download all attachments

2010-01-08 05:31:12

Convert blk-cgroup to be buildable as a module

From: Ben Blum <[email protected]>

This patch modifies the Block I/O cgroup subsystem to be able to be built as a
module. As the CFQ disk scheduler optionally depends on blk-cgroup, config
options in block/Kconfig, block/Kconfig.iosched, and block/blk-cgroup.h are
enhanced to support the new module dependency.

Signed-off-by: Ben Blum <[email protected]>
---
block/Kconfig | 2 +-
block/Kconfig.iosched | 2 +-
block/blk-cgroup.c | 53 +++++++++++++++++++++++++++++++++++----------
block/blk-cgroup.h | 10 +++++++-
include/linux/iocontext.h | 2 +-
kernel/cgroup.c | 9 ++++++++
6 files changed, 61 insertions(+), 17 deletions(-)

diff --git a/block/Kconfig b/block/Kconfig
index e20fbde..62a5921 100644
--- a/block/Kconfig
+++ b/block/Kconfig
@@ -78,7 +78,7 @@ config BLK_DEV_INTEGRITY
Protection. If in doubt, say N.

config BLK_CGROUP
- bool
+ tristate
depends on CGROUPS
default n
---help---
diff --git a/block/Kconfig.iosched b/block/Kconfig.iosched
index b71abfb..fc71cf0 100644
--- a/block/Kconfig.iosched
+++ b/block/Kconfig.iosched
@@ -23,6 +23,7 @@ config IOSCHED_DEADLINE

config IOSCHED_CFQ
tristate "CFQ I/O scheduler"
+ select BLK_CGROUP if CFQ_GROUP_IOSCHED
default y
---help---
The CFQ I/O scheduler tries to distribute bandwidth equally
@@ -35,7 +36,6 @@ config IOSCHED_CFQ
config CFQ_GROUP_IOSCHED
bool "CFQ Group Scheduling support"
depends on IOSCHED_CFQ && CGROUPS
- select BLK_CGROUP
default n
---help---
Enable group IO scheduling in CFQ.
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index 1fa2654..6c73380 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -23,6 +23,31 @@ static LIST_HEAD(blkio_list);
struct blkio_cgroup blkio_root_cgroup = { .weight = 2*BLKIO_WEIGHT_DEFAULT };
EXPORT_SYMBOL_GPL(blkio_root_cgroup);

+static struct cgroup_subsys_state *blkiocg_create(struct cgroup_subsys *,
+ struct cgroup *);
+static int blkiocg_can_attach(struct cgroup_subsys *, struct cgroup *,
+ struct task_struct *, bool);
+static void blkiocg_attach(struct cgroup_subsys *, struct cgroup *,
+ struct cgroup *, struct task_struct *, bool);
+static void blkiocg_destroy(struct cgroup_subsys *, struct cgroup *);
+static int blkiocg_populate(struct cgroup_subsys *, struct cgroup *);
+
+struct cgroup_subsys blkio_subsys = {
+ .name = "blkio",
+ .create = blkiocg_create,
+ .can_attach = blkiocg_can_attach,
+ .attach = blkiocg_attach,
+ .destroy = blkiocg_destroy,
+ .populate = blkiocg_populate,
+#ifdef CONFIG_BLK_CGROUP
+ /* note: blkio_subsys_id is otherwise defined in blk-cgroup.h */
+ .subsys_id = blkio_subsys_id,
+#endif
+ .use_id = 1,
+ .module = THIS_MODULE,
+};
+EXPORT_SYMBOL_GPL(blkio_subsys);
+
bool blkiocg_css_tryget(struct blkio_cgroup *blkcg)
{
if (!css_tryget(&blkcg->css))
@@ -267,7 +292,8 @@ remove_entry:
done:
free_css_id(&blkio_subsys, &blkcg->css);
rcu_read_unlock();
- kfree(blkcg);
+ if (blkcg != &blkio_root_cgroup)
+ kfree(blkcg);
}

static struct cgroup_subsys_state *
@@ -333,17 +359,6 @@ static void blkiocg_attach(struct cgroup_subsys *subsys, struct cgroup *cgroup,
task_unlock(tsk);
}

-struct cgroup_subsys blkio_subsys = {
- .name = "blkio",
- .create = blkiocg_create,
- .can_attach = blkiocg_can_attach,
- .attach = blkiocg_attach,
- .destroy = blkiocg_destroy,
- .populate = blkiocg_populate,
- .subsys_id = blkio_subsys_id,
- .use_id = 1,
-};
-
void blkio_policy_register(struct blkio_policy_type *blkiop)
{
spin_lock(&blkio_list_lock);
@@ -359,3 +374,17 @@ void blkio_policy_unregister(struct blkio_policy_type *blkiop)
spin_unlock(&blkio_list_lock);
}
EXPORT_SYMBOL_GPL(blkio_policy_unregister);
+
+static int __init init_cgroup_blkio(void)
+{
+ return cgroup_load_subsys(&blkio_subsys);
+}
+
+static void __exit exit_cgroup_blkio(void)
+{
+ cgroup_unload_subsys(&blkio_subsys);
+}
+
+module_init(init_cgroup_blkio);
+module_exit(exit_cgroup_blkio);
+MODULE_LICENSE("GPL");
diff --git a/block/blk-cgroup.h b/block/blk-cgroup.h
index 4d316df..57648c6 100644
--- a/block/blk-cgroup.h
+++ b/block/blk-cgroup.h
@@ -15,7 +15,13 @@

#include <linux/cgroup.h>

-#ifdef CONFIG_BLK_CGROUP
+#if defined(CONFIG_BLK_CGROUP) || defined(CONFIG_BLK_CGROUP_MODULE)
+
+#ifndef CONFIG_BLK_CGROUP
+/* When blk-cgroup is a module, its subsys_id isn't a compile-time constant */
+extern struct cgroup_subsys blkio_subsys;
+#define blkio_subsys_id blkio_subsys.subsys_id
+#endif

struct blkio_cgroup {
struct cgroup_subsys_state css;
@@ -94,7 +100,7 @@ static inline void blkiocg_update_blkio_group_dequeue_stats(
struct blkio_group *blkg, unsigned long dequeue) {}
#endif

-#ifdef CONFIG_BLK_CGROUP
+#if defined(CONFIG_BLK_CGROUP) || defined(CONFIG_BLK_CGROUP_MODULE)
extern struct blkio_cgroup blkio_root_cgroup;
extern struct blkio_cgroup *cgroup_to_blkio_cgroup(struct cgroup *cgroup);
extern void blkiocg_add_blkio_group(struct blkio_cgroup *blkcg,
diff --git a/include/linux/iocontext.h b/include/linux/iocontext.h
index a632359..b9f109d 100644
--- a/include/linux/iocontext.h
+++ b/include/linux/iocontext.h
@@ -68,7 +68,7 @@ struct io_context {
unsigned short ioprio;
unsigned short ioprio_changed;

-#ifdef CONFIG_BLK_CGROUP
+#if defined(CONFIG_BLK_CGROUP) || defined(CONFIG_BLK_CGROUP_MODULE)
unsigned short cgroup_changed;
#endif

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index b4ae6ef..845a2e7 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -690,6 +690,7 @@ void cgroup_lock(void)
{
mutex_lock(&cgroup_mutex);
}
+EXPORT_SYMBOL_GPL(cgroup_lock);

/**
* cgroup_unlock - release lock on cgroup changes
@@ -700,6 +701,7 @@ void cgroup_unlock(void)
{
mutex_unlock(&cgroup_mutex);
}
+EXPORT_SYMBOL_GPL(cgroup_unlock);

/*
* A couple of forward declarations required, due to cyclic reference loop:
@@ -1622,6 +1624,7 @@ int cgroup_path(const struct cgroup *cgrp, char *buf, int buflen)
memmove(buf, start, buf + buflen - start);
return 0;
}
+EXPORT_SYMBOL_GPL(cgroup_path);

/**
* cgroup_attach_task - attach task 'tsk' to cgroup 'cgrp'
@@ -1762,6 +1765,7 @@ bool cgroup_lock_live_group(struct cgroup *cgrp)
}
return true;
}
+EXPORT_SYMBOL_GPL(cgroup_lock_live_group);

static int cgroup_release_agent_write(struct cgroup *cgrp, struct cftype *cft,
const char *buffer)
@@ -4036,6 +4040,7 @@ void __css_put(struct cgroup_subsys_state *css)
rcu_read_unlock();
WARN_ON_ONCE(val < 1);
}
+EXPORT_SYMBOL_GPL(__css_put);

/*
* Notify userspace when a cgroup is released, by running the
@@ -4151,6 +4156,7 @@ unsigned short css_id(struct cgroup_subsys_state *css)
return cssid->id;
return 0;
}
+EXPORT_SYMBOL_GPL(css_id);

unsigned short css_depth(struct cgroup_subsys_state *css)
{
@@ -4160,6 +4166,7 @@ unsigned short css_depth(struct cgroup_subsys_state *css)
return cssid->depth;
return 0;
}
+EXPORT_SYMBOL_GPL(css_depth);

bool css_is_ancestor(struct cgroup_subsys_state *child,
const struct cgroup_subsys_state *root)
@@ -4196,6 +4203,7 @@ void free_css_id(struct cgroup_subsys *ss, struct cgroup_subsys_state *css)
spin_unlock(&ss->id_lock);
call_rcu(&id->rcu_head, __free_css_id_cb);
}
+EXPORT_SYMBOL_GPL(free_css_id);

/*
* This is called by init or create(). Then, calls to this function are
@@ -4312,6 +4320,7 @@ struct cgroup_subsys_state *css_lookup(struct cgroup_subsys *ss, int id)

return rcu_dereference(cssid->css);
}
+EXPORT_SYMBOL_GPL(css_lookup);

/**
* css_get_next - lookup next cgroup under specified hierarchy.

Attachments:

cgroups-blkio-as-module.patch (7.68 kB)

2010-01-13 00:35:41

On Thu, Jan 14, 2010 at 03:02:09PM +0530, Balbir Singh wrote:
> On Tue, Jan 12, 2010 at 5:51 AM, KAMEZAWA Hiroyuki
> <[email protected]> wrote:
> > On Fri, 8 Jan 2010 10:10:38 -0500
> > Vivek Goyal <[email protected]> wrote:
> >
> >> On Fri, Jan 08, 2010 at 12:30:21AM -0500, Ben Blum wrote:
> >> > Convert blk-cgroup to be buildable as a module
> >> >
> >> > From: Ben Blum <[email protected]>
> >> >
> >> > This patch modifies the Block I/O cgroup subsystem to be able to be built as a
> >> > module. As the CFQ disk scheduler optionally depends on blk-cgroup, config
> >> > options in block/Kconfig, block/Kconfig.iosched, and block/blk-cgroup.h are
> >> > enhanced to support the new module dependency.
> >> >
> >>
> >> Hi Ben,
> >>
> >> I will give this patch a try.
> >>
> >> So from blk-cgroup perspective, the advantage of allowing it as module
> >> will be that we can save some memory if we are not using the controller?
> >>
> > Is "moduled" blkio cgroup safe after page-tracking by page_cgroup is
> > introduced ?
> >
>
> My guess is it won't be, unless we start exposing page_cgroup API and
> then make the module depend on memcg.

I think I agree. When we introduce page_cgroup based page tracking, either
we need to export page_cgroup API or we can force blkio controller to
compile as in-kernel if user selects the CONFIG_PAGE_TRACKING option.

So as of now, I can't think why we should not we allow compiling blkio as
module as long as core cgroup functionality supports it safely.

Thanks
Vivek