[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <47F4378A.2000008@cn.fujitsu.com>
Date: Thu, 03 Apr 2008 09:48:58 +0800
From: Li Zefan <lizf@...fujitsu.com>
To: "Serge E. Hallyn" <serue@...ibm.com>
CC: Paul Menage <menage@...gle.com>,
lkml <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/1] cgroups: introduce cft->read_seq()
Serge E. Hallyn wrote:
> Hi Paul,
>
> the following (against 2.6.25-rc8-mm1) is a first attempt
> at a simple seq_file usage in cgroups. Comments much
> appreciated.
>
> thanks,
> -serge
>
>>>From bd0977a5819dc43866fff325ae1e2726e747e2f4 Mon Sep 17 00:00:00 2001
> From: Serge E. Hallyn <serue@...ibm.com>
> Date: Wed, 2 Apr 2008 17:54:21 -0700
> Subject: [PATCH 1/1] cgroups: introduce cft->read_seq()
>
> Introduce a read_seq() helper in cftype, which uses
> seq_file to print out lists. Use it in the devices
> cgroup. Also split devices.allow into two files, so
> now devices.deny and devices.allow are the ones to
> use to manipulate the whitelist, while devices.list
> outputs the cgroup's current whitelist.
>
> Signed-off-by: Serge E. Hallyn <serue@...ibm.com>
> ---
> include/linux/cgroup.h | 6 ++++
> kernel/cgroup.c | 22 ++++++++++++-
> security/device_cgroup.c | 74 ++++++++++++++-------------------------------
> 3 files changed, 49 insertions(+), 53 deletions(-)
>
> diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
> index 2d1d151..feb83dd 100644
> --- a/include/linux/cgroup.h
> +++ b/include/linux/cgroup.h
> @@ -227,6 +227,12 @@ struct cftype {
> */
> int (*read_map) (struct cgroup *cont, struct cftype *cft,
> struct cgroup_map_cb *cb);
> + /*
> + * read_seq() is used for outputting a simple sequence
> + * using seqfile.
> + */
> + int (*read_seq) (struct cgroup *cont, struct cftype *cft,
> + struct seq_file *m);
>
Can't we remove read_map() ?
btw: s/cont/cgrp
> ssize_t (*write) (struct cgroup *cgrp, struct cftype *cft,
> struct file *file,
> diff --git a/kernel/cgroup.c b/kernel/cgroup.c
> index 85f31ad..61dc509 100644
> --- a/kernel/cgroup.c
> +++ b/kernel/cgroup.c
> @@ -1518,7 +1518,7 @@ static int cgroup_map_add(struct cgroup_map_cb *cb, const char *key, u64 value)
> return seq_printf(sf, "%s %llu\n", key, (unsigned long long)value);
> }
>
> -static int cgroup_seqfile_show(struct seq_file *m, void *arg)
> +static int cgroup_map_seqfile_show(struct seq_file *m, void *arg)
> {
> struct cgroup_seqfile_state *state = m->private;
> struct cftype *cft = state->cft;
> @@ -1529,6 +1529,13 @@ static int cgroup_seqfile_show(struct seq_file *m, void *arg)
> return cft->read_map(state->cgroup, cft, &cb);
> }
>
> +static int cgroup_simple_seqfile_show(struct seq_file *m, void *arg)
> +{
> + struct cgroup_seqfile_state *state = m->private;
> + struct cftype *cft = state->cft;
> + return cft->read_seq(state->cgroup, cft, m);
> +}
> +
> int cgroup_seqfile_release(struct inode *inode, struct file *file)
> {
> struct seq_file *seq = file->private_data;
> @@ -1562,7 +1569,18 @@ static int cgroup_file_open(struct inode *inode, struct file *file)
> state->cft = cft;
> state->cgroup = __d_cgrp(file->f_dentry->d_parent);
> file->f_op = &cgroup_seqfile_operations;
> - err = single_open(file, cgroup_seqfile_show, state);
> + err = single_open(file, cgroup_map_seqfile_show, state);
> + if (err < 0)
> + kfree(state);
> + } else if (cft->read_seq) {
> + struct cgroup_seqfile_state *state =
> + kzalloc(sizeof(*state), GFP_USER);
> + if (!state)
> + return -ENOMEM;
> + state->cft = cft;
> + state->cgroup = __d_cgrp(file->f_dentry->d_parent);
> + file->f_op = &cgroup_seqfile_operations;
> + err = single_open(file, cgroup_simple_seqfile_show, state);
> if (err < 0)
> kfree(state);
> } else if (cft->open)
> diff --git a/security/device_cgroup.c b/security/device_cgroup.c
> index 4237b19..68a247c 100644
> --- a/security/device_cgroup.c
> +++ b/security/device_cgroup.c
> @@ -9,6 +9,7 @@
> #include <linux/ctype.h>
> #include <linux/list.h>
> #include <linux/uaccess.h>
> +#include <linux/seq_file.h>
>
> #define ACC_MKNOD 1
> #define ACC_READ 2
> @@ -201,11 +202,15 @@ static void devcgroup_destroy(struct cgroup_subsys *ss,
>
> #define DEVCG_ALLOW 1
> #define DEVCG_DENY 2
> +#define DEVCG_LIST 3
> +
> +#define MAJMINLEN 10
> +#define ACCLEN 4
>
> static void set_access(char *acc, short access)
> {
> int idx = 0;
> - memset(acc, 0, 4);
> + memset(acc, 0, ACCLEN);
> if (access & ACC_READ)
> acc[idx++] = 'r';
> if (access & ACC_WRITE)
> @@ -225,70 +230,33 @@ static char type_to_char(short type)
> return 'X';
> }
>
> -static void set_majmin(char *str, int len, unsigned m)
> +static void set_majmin(char *str, unsigned m)
> {
> - memset(str, 0, len);
> + memset(str, 0, MAJMINLEN);
> if (m == ~0)
> sprintf(str, "*");
> else
> - snprintf(str, len, "%d", m);
> + snprintf(str, MAJMINLEN, "%d", m);
> }
>
> -static char *print_whitelist(struct dev_cgroup *devcgroup, int *len)
> +static int devcgroup_seq_read(struct cgroup *cgroup, struct cftype *cft,
> + struct seq_file *m)
> {
> - char *buf, *s, acc[4];
> + struct dev_cgroup *devcgroup = cgroup_to_devcgroup(cgroup);
> struct dev_whitelist_item *wh;
> - int ret;
> - int count = 0;
> - char maj[10], min[10];
> -
> - buf = kmalloc(4096, GFP_KERNEL);
> - if (!buf)
> - return ERR_PTR(-ENOMEM);
> - s = buf;
> - *s = '\0';
> - *len = 0;
> + char maj[MAJMINLEN], min[MAJMINLEN], acc[ACCLEN];
>
> spin_lock(&devcgroup->lock);
> list_for_each_entry(wh, &devcgroup->whitelist, list) {
> set_access(acc, wh->access);
> - set_majmin(maj, 10, wh->major);
> - set_majmin(min, 10, wh->minor);
> - ret = snprintf(s, 4095-(s-buf), "%c %s:%s %s\n",
> - type_to_char(wh->type), maj, min, acc);
> - if (s+ret >= buf+4095) {
> - kfree(buf);
> - buf = ERR_PTR(-ENOMEM);
> - break;
> - }
> - s += ret;
> - *len += ret;
> - count++;
> + set_majmin(maj, wh->major);
> + set_majmin(min, wh->minor);
> + seq_printf(m, "%c %s:%s %s\n", type_to_char(wh->type),
> + maj, min, acc);
> }
> spin_unlock(&devcgroup->lock);
>
> - return buf;
> -}
> -
> -static ssize_t devcgroup_access_read(struct cgroup *cgroup,
> - struct cftype *cft, struct file *file,
> - char __user *userbuf, size_t nbytes, loff_t *ppos)
> -{
> - struct dev_cgroup *devcgroup = cgroup_to_devcgroup(cgroup);
> - int filetype = cft->private;
> - char *buffer;
> - int uninitialized_var(len);
> - int retval;
> -
> - if (filetype != DEVCG_ALLOW)
> - return -EINVAL;
> - buffer = print_whitelist(devcgroup, &len);
> - if (IS_ERR(buffer))
> - return PTR_ERR(buffer);
> -
> - retval = simple_read_from_buffer(userbuf, nbytes, ppos, buffer, len);
> - kfree(buffer);
> - return retval;
> + return 0;
> }
>
> /*
> @@ -501,7 +469,6 @@ out1:
> static struct cftype dev_cgroup_files[] = {
> {
> .name = "allow",
> - .read = devcgroup_access_read,
> .write = devcgroup_access_write,
> .private = DEVCG_ALLOW,
> },
> @@ -510,6 +477,11 @@ static struct cftype dev_cgroup_files[] = {
> .write = devcgroup_access_write,
> .private = DEVCG_DENY,
> },
> + {
> + .name = "list",
> + .read_seq = devcgroup_seq_read,
> + .private = DEVCG_LIST,
> + },
> };
>
> static int devcgroup_populate(struct cgroup_subsys *ss,
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists