Messages in this thread |  | | Date | Wed, 14 Aug 2002 19:37:02 +0100 | From | Christoph Hellwig <> | Subject | Re: [patch] Scalable statistics counters using seq_file interfaces |
| |
On Wed, Aug 14, 2002 at 04:50:49PM +0530, Ravikiran G Thirumalai wrote: > Christoph, since statctr_pentry describes only a proc entry for a > set of counters, I thought it'd be more descriptive to use > statctr_proc_entry or statctr_pentry...something to indicate it is > for proc reporting only.
My educated guess is that I will rewrite the code to not depend on procfs before 2.6 is out.. But as you use other procfs types it's just one more search and replace, so..
> diff -ruN -X dontdiff linux-2.5.31/fs/proc/root.c statctr-2.5.31/fs/proc/root.c > --- linux-2.5.31/fs/proc/root.c Sun Aug 11 07:11:50 2002 > +++ statctr-2.5.31/fs/proc/root.c Tue Aug 13 10:14:46 2002 > @@ -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);
Any reason you don't do this in kernel/statctr.c?
> +#ifndef _LINUX_STATCTR_H > +#define _LINUX_STATCTR_H > + > +#ifdef __KERNEL__
Is this needed?
> +static inline struct statctr_pentry > +*create_statctr_pentry(struct proc_dir_entry *parent, const char *name)
Shouldn't this be:
static inline struct statctr_pentry * create_statctr_pentry(struct proc_dir_entry *parent, const char *name)
> +{ > + return(NULL); > +}
return is not function-like and the additional braces are heavily disliked in the kernel (at least for new code)
> +static inline int __statctr_init(statctr_t *stctr, int gfp_mask) > +{ > + stctr->ctr = kmalloc_percpu(sizeof(*(stctr->ctr)), gfp_mask); > + if(!stctr->ctr)
Kernel coding style has a space after the if.
> + return -1;
-ENOMEM?
> +void free_statctr_pentry(struct statctr_pentry *pentry) > +{ > + if(!list_empty(&pentry->head))
Add an unlikely?
It might be worth to rework the code to follow Documentation/CodingStyle - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|  |