Skip to contentSkip to conversation

Scalable statistics counters with /proc reporting

PATCH In review Ravikiran G Thirumalai 3 participants 5 messages Aug 7, 2002 – Aug 8, 2002 Proc filesystem

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Scalable statistics counters with /proc reporting

Here's the new statctr patch (2.5.29) on top of kmalloc_percpu dynamic allocator. This one passes GFP_ flags for statctr_init as suggested by Andrew Morton and makes use of cpu_possible. Counters can be grouped into one or multiple /proc files as per Rik's suggestion.
Here are the revised interfaces:

1. struct statctr_proc_entry
*create_statctr_proc_entry(struct proc_dir_entry *parent, const char *name); To create a /proc file to group counters for automatic /proc reporting

2. free_statctr_proc_entry(struct statctr_proc_entry *)

To free up the proc entry

3. int statctr_init(statctr_t *stctr, unsigned long val, struct statctr_proc_entry *pentry, const char *ctrname, int flags)

Allocates memory to the counter , initialises it, and links the counter with statctr_proc_entry for automatic /proc reporting. ctrname is the counter label as it appears in /proc, flags are the GFP_ hints. /proc reporting can be turned off if pentry is NULL.

4. statctr_cleanup(statctr_t *)
frees up counter memory and unlinks statctr_proc_entry association.

5. statctr_inc(statctr_t *) , statctr_dec(statctr_t *) ...... for fast counter ops.

Rik does this suit you?

Thanks,
Kiran

fs/proc/proc_misc.c +1 -1
diff -ruN -X dontdiff linux-2.5.29/fs/proc/proc_misc.c statctr-2.5.29-6/fs/proc/proc_misc.c
@@ -63,7 +63,7 @@
 extern int get_ds1286_status(char *);
 #endif
 
-static int proc_calc_metrics(char *page, char **start, off_t off,
+int proc_calc_metrics(char *page, char **start, off_t off,
 				 int count, int *eof, int len)
 {
 	if (len <= off+count) *eof = 1;
fs/proc/root.c +2 -0
diff -ruN -X dontdiff linux-2.5.29/fs/proc/root.c statctr-2.5.29-6/fs/proc/root.c
@@ -19,6 +19,7 @@
 #include <linux/smp_lock.h>
 
 struct proc_dir_entry *proc_net, *proc_bus, *proc_root_fs, *proc_root_driver;
+struct proc_dir_entry *proc_statistics;
 
 #ifdef CONFIG_SYSCTL
 struct proc_dir_entry *proc_sys_root;
@@ -77,6 +78,7 @@
 	proc_rtas_init();
 #endif
 	proc_bus = proc_mkdir("bus", 0);
+	proc_statistics = proc_mkdir("statistics", 0);
 }
 
 static struct dentry *proc_root_lookup(struct inode * dir, struct dentry * dentry)
include/linux/proc_fs.h +1 -0
diff -ruN -X dontdiff linux-2.5.29/include/linux/proc_fs.h statctr-2.5.29-6/include/linux/proc_fs.h
@@ -101,6 +101,7 @@
 extern struct inode * proc_get_inode(struct super_block *, int, struct proc_dir_entry *);
 
 extern int proc_match(int, const char *,struct proc_dir_entry *);
+extern int proc_calc_metrics(char *, char **, off_t, int, int *, int);
 
 /*
  * These are generic /proc routines that use the internal
include/linux/statctr.h +254 -0

254 lines not on this page · raw message

kernel/Makefile +2 -2
diff -ruN -X dontdiff linux-2.5.29/kernel/Makefile statctr-2.5.29-6/kernel/Makefile
@@ -10,12 +10,12 @@
 O_TARGET := kernel.o
 
 export-objs = signal.o sys.o kmod.o context.o ksyms.o pm.o exec_domain.o \
-		printk.o platform.o suspend.o
+		printk.o platform.o suspend.o statctr.o
 
 obj-y     = sched.o dma.o fork.o exec_domain.o panic.o printk.o \
 	    module.o exit.o itimer.o time.o softirq.o resource.o \
 	    sysctl.o capability.o ptrace.o timer.o user.o \
-	    signal.o sys.o kmod.o context.o futex.o platform.o
+	    signal.o sys.o kmod.o context.o futex.o platform.o statctr.o
 
 obj-$(CONFIG_SMP) += cpu.o
 obj-$(CONFIG_UID16) += uid16.o
kernel/statctr.c +213 -0

213 lines not on this page · raw message

#1 lore raw
Quoted 14 lines · Ravikiran G Thirumalai

Here's the new statctr patch (2.5.29) on top of kmalloc_percpu dynamic allocator. This one passes GFP_ flags for statctr_init as suggested by Andrew Morton and makes use of cpu_possible. Counters can be grouped into one or multiple /proc files as per Rik's suggestion.
Here are the revised interfaces:

1. struct statctr_proc_entry
*create_statctr_proc_entry(struct proc_dir_entry *parent, const char *name); To create a /proc file to group counters for automatic /proc reporting

2. free_statctr_proc_entry(struct statctr_proc_entry *)

To free up the proc entry

Quoted 8 lines

3. int statctr_init(statctr_t *stctr, unsigned long val, struct statctr_proc_entry *pentry, const char *ctrname, int flags)

Allocates memory to the counter , initialises it, and links the counter with statctr_proc_entry for automatic /proc reporting. ctrname is the counter label as it appears in /proc, flags are the GFP_ hints. /proc reporting can be turned off if pentry is NULL.

What about a general s/_proc//g for the API?

For statctr_init the flags argument would much better be named gfp_mask, the val argument should be removed and the counter initialized to zero by default - that's 90% of the uses..

Also the /proc-based implementation is really ugly. It should be moved over to the seq_file interface at least, a simple ramfs-style own filesystem ("stats" filesystem type) would be the cleanest solution.

#2 lore raw
Quoted 5 lines · Christoph Hellwig

...
Also the /proc-based implementation is really ugly. It should be moved over to the seq_file interface at least, a simple ramfs-style own filesystem ("stats" filesystem type) would be the cleanest solution.

I'd suggest that it be just a single stat counter per /proc file. A simple sprintf is fine for that.

If it's necessary to write a new filesystem to just export a bunch of integers to userspace then something is seriously wrong.

#3 lore raw
Quoted 7 lines · Andrew Morton
Quoted 5 lines · Christoph Hellwig

...
Also the /proc-based implementation is really ugly. It should be moved over to the seq_file interface at least, a simple ramfs-style own filesystem ("stats" filesystem type) would be the cleanest solution.

Quoted 5 lines

I'd suggest that it be just a single stat counter per /proc file. A simple sprintf is fine for that.

If it's necessary to write a new filesystem to just export a bunch of integers to userspace then something is seriously wrong.

Yes, that was how my previous release worked. But Rik wanted to be able to group a bunch of counters into a single /proc file for his use. Rik, your comments please... what do you prefer? current /proc style, seq_file, or the earlier "one counter per /proc" style?

Thanks,
Kiran

#4 lore raw
In reply to Christoph Hellwig
Quoted 2 lines · Christoph Hellwig

...
What about a general s/_proc//g for the API?

Sure ...I was initially thinking of something like statctr_pentry, but then thought statctr_proc_entry would be explicit although lengthy.

Quoted 2 lines

For statctr_init the flags argument would much better be named gfp_mask,

I'll do that. I was just following the same conventions as kmalloc and kmem_cache_alloc; but gfp_mask is much better; thanx.

Quoted 2 lines

the val argument should be removed and the counter initialized to zero by default - that's 90% of the uses..

Yup, I'll do this too.

Quoted 4 lines

Also the /proc-based implementation is really ugly. It should be moved over to the seq_file interface at least, a simple ramfs-style own filesystem ("stats" filesystem type) would be the cleanest solution.

If I want to create difft proc files for difft groups of statctrs like say /proc/statistics/vmstats, /proc/statistics/netstats etc., dynamically, using seq_file implementation might be not very neat...I have to confess I don't know much about seq_file interfaces....so pls correct me if I am missing something......I just looked at them after your pointer.

We wanted to keep things simple right now ... IMHO a custom file system right now might be complicating the patch.....maybe later when there are many users of statctrs 8-) ...

Thanks,
Kiran