Re: [PATCH 2/2] NFS: add I/O performance counters
[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 . 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/
[PATCH 2/2] NFS: add I/O performance counters
Add an extensible per-superblock performance counter facility to the NFS client. This facility mimics the counters available for block devices and for networking. Expose these new counters via /proc/self/mountstats. Version: Mon, 14 Mar 2005 17:06:12 -0500 Signed-off-by: Chuck Lever <[EMAIL PROTECTED]> --- fs/nfs/dir.c |8 ++ fs/nfs/direct.c|5 + fs/nfs/file.c | 20 +++-- fs/nfs/inode.c | 126 +++-- fs/nfs/pagelist.c | 12 ++- fs/nfs/read.c |7 ++ fs/nfs/write.c | 10 ++ include/linux/nfs_fs_sb.h |5 + include/linux/nfs_iostat.h | 80 +++ 9 files changed, 256 insertions(+), 17 deletions(-) diff -X /home/cel/src/linux/dont-diff -Naurp 01-mountstats/fs/nfs/dir.c 02-nfs-iostat/fs/nfs/dir.c --- 01-mountstats/fs/nfs/dir.c 2005-03-02 02:38:09.0 -0500 +++ 02-nfs-iostat/fs/nfs/dir.c 2005-03-14 15:28:34.011484000 -0500 @@ -27,6 +27,7 @@ #include #include #include +#include #include #include #include @@ -428,6 +429,8 @@ static int nfs_readdir(struct file *filp lock_kernel(); + nfs_inc_stats(inode, NFS_VFS_GETDENTS); + res = nfs_revalidate_inode(NFS_SERVER(inode), inode); if (res < 0) { unlock_kernel(); @@ -584,6 +587,7 @@ static int nfs_lookup_revalidate(struct parent = dget_parent(dentry); lock_kernel(); dir = parent->d_inode; + nfs_inc_stats(dir, NFS_DENTRY_REVALIDATE); inode = dentry->d_inode; if (nd && !(nd->flags & LOOKUP_CONTINUE) && (nd->flags & LOOKUP_OPEN)) @@ -712,6 +716,7 @@ static struct dentry *nfs_lookup(struct dfprintk(VFS, "NFS: lookup(%s/%s)\n", dentry->d_parent->d_name.name, dentry->d_name.name); + nfs_inc_stats(dir, NFS_VFS_LOOKUP); res = ERR_PTR(-ENAMETOOLONG); if (dentry->d_name.len > NFS_SERVER(dir)->namelen) @@ -1116,6 +1121,7 @@ static int nfs_sillyrename(struct inode dfprintk(VFS, "NFS: silly-rename(%s/%s, ct=%d)\n", dentry->d_parent->d_name.name, dentry->d_name.name, atomic_read(>d_count)); + nfs_inc_stats(dir, NFS_SILLY_RENAME); #ifdef NFS_PARANOIA if (!dentry->d_inode) @@ -1500,6 +1506,8 @@ int nfs_permission(struct inode *inode, struct rpc_cred *cred; int res; + nfs_inc_stats(inode, NFS_VFS_ACCESS); + if (mask == 0) return 0; diff -X /home/cel/src/linux/dont-diff -Naurp 01-mountstats/fs/nfs/direct.c 02-nfs-iostat/fs/nfs/direct.c --- 01-mountstats/fs/nfs/direct.c 2005-03-02 02:38:25.0 -0500 +++ 02-nfs-iostat/fs/nfs/direct.c 2005-03-14 15:26:16.401349000 -0500 @@ -47,6 +47,7 @@ #include #include +#include #include #include @@ -354,6 +355,8 @@ static ssize_t nfs_direct_read_seg(struc result = nfs_direct_read_wait(dreq, clnt->cl_intr); rpc_clnt_sigunmask(clnt, ); + nfs_add_stats(inode, NFS_WIRE_READ_BYTES, result); + nfs_add_stats(inode, NFS_DIRECT_READ_BYTES, result); return result; } @@ -576,6 +579,8 @@ static ssize_t nfs_direct_write(struct i if (result < size) break; } + nfs_add_stats(inode, NFS_WIRE_WRITTEN_BYTES, tot_bytes); + nfs_add_stats(inode, NFS_DIRECT_WRITTEN_BYTES, tot_bytes); return tot_bytes; } diff -X /home/cel/src/linux/dont-diff -Naurp 01-mountstats/fs/nfs/file.c 02-nfs-iostat/fs/nfs/file.c --- 01-mountstats/fs/nfs/file.c 2005-03-02 02:38:38.0 -0500 +++ 02-nfs-iostat/fs/nfs/file.c 2005-03-14 15:42:52.446804000 -0500 @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -86,18 +87,15 @@ static int nfs_check_flags(int flags) static int nfs_file_open(struct inode *inode, struct file *filp) { - struct nfs_server *server = NFS_SERVER(inode); - int (*open)(struct inode *, struct file *); int res; res = nfs_check_flags(filp->f_flags); if (res) return res; + nfs_inc_stats(inode, NFS_VFS_OPEN); lock_kernel(); - /* Do NFSv4 open() call */ - if ((open = server->rpc_ops->file_open) != NULL) - res = open(inode, filp); + res = NFS_SERVER(inode)->rpc_ops->file_open(inode, filp); unlock_kernel(); return res; } @@ -105,6 +103,7 @@ nfs_file_open(struct inode *inode, struc static int nfs_file_release(struct inode *inode, struct file *filp) { + nfs_inc_stats(inode, NFS_VFS_CLOSE); return NFS_PROTO(inode)->file_release(inode, filp); } @@ -123,6 +122,7 @@ nfs_file_flush(struct file *file) if ((file->f_mode & FMODE_WRITE) == 0) return 0; + nfs_inc_stats(inode, NFS_VFS_FLUSH); lock_kernel(); /* Ensure that data+attribute caches are up to date after close() */
[PATCH 2/2] NFS: add I/O performance counters
Add an extensible per-superblock performance counter facility to the NFS client. This facility mimics the counters available for block devices and for networking. Expose these new counters via /proc/self/mountstats. Version: Mon, 14 Mar 2005 17:06:12 -0500 Signed-off-by: Chuck Lever [EMAIL PROTECTED] --- fs/nfs/dir.c |8 ++ fs/nfs/direct.c|5 + fs/nfs/file.c | 20 +++-- fs/nfs/inode.c | 126 +++-- fs/nfs/pagelist.c | 12 ++- fs/nfs/read.c |7 ++ fs/nfs/write.c | 10 ++ include/linux/nfs_fs_sb.h |5 + include/linux/nfs_iostat.h | 80 +++ 9 files changed, 256 insertions(+), 17 deletions(-) diff -X /home/cel/src/linux/dont-diff -Naurp 01-mountstats/fs/nfs/dir.c 02-nfs-iostat/fs/nfs/dir.c --- 01-mountstats/fs/nfs/dir.c 2005-03-02 02:38:09.0 -0500 +++ 02-nfs-iostat/fs/nfs/dir.c 2005-03-14 15:28:34.011484000 -0500 @@ -27,6 +27,7 @@ #include linux/mm.h #include linux/sunrpc/clnt.h #include linux/nfs_fs.h +#include linux/nfs_iostat.h #include linux/nfs_mount.h #include linux/pagemap.h #include linux/smp_lock.h @@ -428,6 +429,8 @@ static int nfs_readdir(struct file *filp lock_kernel(); + nfs_inc_stats(inode, NFS_VFS_GETDENTS); + res = nfs_revalidate_inode(NFS_SERVER(inode), inode); if (res 0) { unlock_kernel(); @@ -584,6 +587,7 @@ static int nfs_lookup_revalidate(struct parent = dget_parent(dentry); lock_kernel(); dir = parent-d_inode; + nfs_inc_stats(dir, NFS_DENTRY_REVALIDATE); inode = dentry-d_inode; if (nd !(nd-flags LOOKUP_CONTINUE) (nd-flags LOOKUP_OPEN)) @@ -712,6 +716,7 @@ static struct dentry *nfs_lookup(struct dfprintk(VFS, NFS: lookup(%s/%s)\n, dentry-d_parent-d_name.name, dentry-d_name.name); + nfs_inc_stats(dir, NFS_VFS_LOOKUP); res = ERR_PTR(-ENAMETOOLONG); if (dentry-d_name.len NFS_SERVER(dir)-namelen) @@ -1116,6 +1121,7 @@ static int nfs_sillyrename(struct inode dfprintk(VFS, NFS: silly-rename(%s/%s, ct=%d)\n, dentry-d_parent-d_name.name, dentry-d_name.name, atomic_read(dentry-d_count)); + nfs_inc_stats(dir, NFS_SILLY_RENAME); #ifdef NFS_PARANOIA if (!dentry-d_inode) @@ -1500,6 +1506,8 @@ int nfs_permission(struct inode *inode, struct rpc_cred *cred; int res; + nfs_inc_stats(inode, NFS_VFS_ACCESS); + if (mask == 0) return 0; diff -X /home/cel/src/linux/dont-diff -Naurp 01-mountstats/fs/nfs/direct.c 02-nfs-iostat/fs/nfs/direct.c --- 01-mountstats/fs/nfs/direct.c 2005-03-02 02:38:25.0 -0500 +++ 02-nfs-iostat/fs/nfs/direct.c 2005-03-14 15:26:16.401349000 -0500 @@ -47,6 +47,7 @@ #include linux/kref.h #include linux/nfs_fs.h +#include linux/nfs_iostat.h #include linux/nfs_page.h #include linux/sunrpc/clnt.h @@ -354,6 +355,8 @@ static ssize_t nfs_direct_read_seg(struc result = nfs_direct_read_wait(dreq, clnt-cl_intr); rpc_clnt_sigunmask(clnt, oldset); + nfs_add_stats(inode, NFS_WIRE_READ_BYTES, result); + nfs_add_stats(inode, NFS_DIRECT_READ_BYTES, result); return result; } @@ -576,6 +579,8 @@ static ssize_t nfs_direct_write(struct i if (result size) break; } + nfs_add_stats(inode, NFS_WIRE_WRITTEN_BYTES, tot_bytes); + nfs_add_stats(inode, NFS_DIRECT_WRITTEN_BYTES, tot_bytes); return tot_bytes; } diff -X /home/cel/src/linux/dont-diff -Naurp 01-mountstats/fs/nfs/file.c 02-nfs-iostat/fs/nfs/file.c --- 01-mountstats/fs/nfs/file.c 2005-03-02 02:38:38.0 -0500 +++ 02-nfs-iostat/fs/nfs/file.c 2005-03-14 15:42:52.446804000 -0500 @@ -22,6 +22,7 @@ #include linux/fcntl.h #include linux/stat.h #include linux/nfs_fs.h +#include linux/nfs_iostat.h #include linux/nfs_mount.h #include linux/mm.h #include linux/slab.h @@ -86,18 +87,15 @@ static int nfs_check_flags(int flags) static int nfs_file_open(struct inode *inode, struct file *filp) { - struct nfs_server *server = NFS_SERVER(inode); - int (*open)(struct inode *, struct file *); int res; res = nfs_check_flags(filp-f_flags); if (res) return res; + nfs_inc_stats(inode, NFS_VFS_OPEN); lock_kernel(); - /* Do NFSv4 open() call */ - if ((open = server-rpc_ops-file_open) != NULL) - res = open(inode, filp); + res = NFS_SERVER(inode)-rpc_ops-file_open(inode, filp); unlock_kernel(); return res; } @@ -105,6 +103,7 @@ nfs_file_open(struct inode *inode, struc static int nfs_file_release(struct inode *inode, struct file *filp) { + nfs_inc_stats(inode, NFS_VFS_CLOSE); return NFS_PROTO(inode)-file_release(inode, filp); } @@ -123,6 +122,7
Re: [PATCH 2/2] NFS: add I/O performance counters
[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/