Messages in this thread | | | Date | Thu, 03 Apr 2008 09:48:58 +0800 | From | Li Zefan <> | 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@us.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@us.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,
| |