[EMAIL PROTECTED] (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 [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
