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

[Andrew Morton]

cel@xxxxxxxxxxxxxx (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.
-
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/