Re: [PATCH 2/2] NFS: add I/O performance counters

[Andrew Morton <akpm@osdl.org>]

cel@citi.umich.edu (Chuck Lever) wrote:
>
> +static inline void nfs_inc_stats(struct inode *inode, unsigned int stat)
> +{
> +	struct nfs_iostats *iostats = NFS_SERVER(inode)->io_stats;
> +	iostats[smp_processor_id()].counts[stat]++;
> +}

The use of smp_processor_id() outside locks should spit a runtime warning. 
And it is racy: if you switch CPUs between the read and the write (via
preemption), the stats will be corrupted.

A preempt_disable()/enable() will fix those things up.

> +static inline struct nfs_iostats *nfs_alloc_iostats(void)
> +{
> +	struct nfs_iostats *new;
> +	new = kmalloc(sizeof(struct nfs_iostats) * NR_CPUS, GFP_KERNEL);
> +	if (new)
> +		memset(new, 0, sizeof(struct nfs_iostats) * NR_CPUS);
> +	return new;
> +}
> +

You'd be better off using alloc_percpu() here, so each CPU's counter goes
into its node-local memory.

Or simply use <linux/percpu_counter.h>.  AFACIT the warning at the top of
that file isn't true any more.  A 4-byte counter on a 32-way should consume
just a little over 256 bytes.