Re: [PATCH 1/1] cgroups: introduce cft->read_seq()

[Serge E. Hallyn]

Quoting Li Zefan (lizf@xxxxxxxxxxxxxx):
> Li Zefan wrote:
> > 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@xxxxxxxxxx>
> >> 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@xxxxxxxxxx>
> >> ---
> >>  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
> > 
> 
> It seems read_seq() is the general case of read_map(). A read_seq() method can
> produce the output freely, and read_map() strictly produces string->u64 maps.

Right, the memcontroller cgroup is using read_map(), and I seem to recall
it was intended to be of pretty general use so I was going to just
leave it there.

As for s/cont/cgrp I agree but that's for a separate pure renaming
patch.  In the meantime I just wanted to follow the existing style.
I could rebase this patch on top of a renaming patch but that's
definately wrong for just soliciting comments on this being the right
approach.

Thanks for taking a look.

-serge
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/