Re: [patch] Scalable statistics counters using seq_file interfaces

[Christoph Hellwig <hch@infradead.org>]

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