The file nfsfh.c contains two static variables nfsd_nr_verified and
nfsd_nr_put. These are counters which are incremented as a side
effect of the fh_verify() fh_compose() and fh_put() operations,
i.e. at least twice per NFS call for any non-trivial workload.
Needless to say this makes the cacheline that contains them (and any
other innocent victims) a very hot contention point indeed under high
call-rate workloads on multiprocessor NFS server. It also turns out
that these counters are not used anywhere. They're not reported to
userspace, they're not used in logic, they're not even exported from
the object file (let alone the module). All they do is waste CPU time.
So this patch removes them.
Tests on a 16 CPU Altix A4700 with 2 10gige Myricom cards, configured
separately (no bonding). Workload is 640 client threads doing directory
traverals with random small reads, from server RAM.
Before
======
Kernel profile:
% cumulative self self total
time samples samples calls 1/call 1/call name
6.05 2716.00 2716.00 30406 0.09 1.02 svc_process
4.44 4706.00 1990.00 1975 1.01 1.01 spin_unlock_irqrestore
3.72 6376.00 1670.00 1666 1.00 1.00 svc_export_put
3.41 7907.00 1531.00 1786 0.86 1.02 nfsd_ofcache_lookup
3.25 9363.00 1456.00 10965 0.13 1.01 nfsd_dispatch
3.10 10752.00 1389.00 1376 1.01 1.01 nfsd_cache_lookup
2.57 11907.00 1155.00 4517 0.26 1.03 svc_tcp_recvfrom
...
2.21 15352.00 1003.00 1081 0.93 1.00 nfsd_choose_ofc <----
^^^^
Here the function nfsd_choose_ofc() reads a global variable
which by accident happened to be located in the same cacheline as
nfsd_nr_verified.
Call rate:
nullarbor:~ # pmdumptext nfs3.server.calls
...
Thu Dec 13 00:15:27 184780.663
Thu Dec 13 00:15:28 184885.881
Thu Dec 13 00:15:29 184449.215
Thu Dec 13 00:15:30 184971.058
Thu Dec 13 00:15:31 185036.052
Thu Dec 13 00:15:32 185250.475
Thu Dec 13 00:15:33 184481.319
Thu Dec 13 00:15:34 185225.737
Thu Dec 13 00:15:35 185408.018
Thu Dec 13 00:15:36 185335.764
After
=====
kernel profile:
% cumulative self self total
time samples samples calls 1/call 1/call name
6.33 2813.00 2813.00 29979 0.09 1.01 svc_process
4.66 4883.00 2070.00 2065 1.00 1.00 spin_unlock_irqrestore
4.06 6687.00 1804.00 2182 0.83 1.00 nfsd_ofcache_lookup
3.20 8110.00 1423.00 10932 0.13 1.00 nfsd_dispatch
3.03 9456.00 1346.00 1343 1.00 1.00 nfsd_cache_lookup
2.62 10622.00 1166.00 4645 0.25 1.01 svc_tcp_recvfrom
[...]
0.10 42586.00 44.00 74 0.59 1.00 nfsd_choose_ofc <--- HA!!
^^^^
Call rate:
nullarbor:~ # pmdumptext nfs3.server.calls
...
Thu Dec 13 01:45:28 194677.118
Thu Dec 13 01:45:29 193932.692
Thu Dec 13 01:45:30 194294.364
Thu Dec 13 01:45:31 194971.276
Thu Dec 13 01:45:32 194111.207
Thu Dec 13 01:45:33 194999.635
Thu Dec 13 01:45:34 195312.594
Thu Dec 13 01:45:35 195707.293
Thu Dec 13 01:45:36 194610.353
Thu Dec 13 01:45:37 195913.662
Thu Dec 13 01:45:38 194808.675
i.e. about a 5.3% improvement in call rate.
Signed-off-by: Greg Banks <gnb-cP1dWloDopni96+mSzHFpQC/[email protected]>
Reviewed-by: David Chinner <[email protected]>
---
fs/nfsd/nfsfh.c | 6 ------
1 file changed, 6 deletions(-)
Index: bfields/fs/nfsd/nfsfh.c
===================================================================
--- bfields.orig/fs/nfsd/nfsfh.c
+++ bfields/fs/nfsd/nfsfh.c
@@ -27,9 +27,6 @@
#define NFSDDBG_FACILITY NFSDDBG_FH
-static int nfsd_nr_verified;
-static int nfsd_nr_put;
-
/*
* our acceptability function.
* if NOSUBTREECHECK, accept anything
@@ -251,7 +248,6 @@ static __be32 nfsd_set_fh_dentry(struct
fhp->fh_dentry = dentry;
fhp->fh_export = exp;
- nfsd_nr_verified++;
return 0;
out:
exp_put(exp);
@@ -552,7 +548,6 @@ fh_compose(struct svc_fh *fhp, struct sv
return nfserr_opnotsupp;
}
- nfsd_nr_verified++;
return 0;
}
@@ -609,7 +604,6 @@ fh_put(struct svc_fh *fhp)
fhp->fh_pre_saved = 0;
fhp->fh_post_saved = 0;
#endif
- nfsd_nr_put++;
}
if (exp) {
cache_put(&exp->h, &svc_export_cache);
--
Greg
On Wed, Apr 01, 2009 at 07:28:21AM +1100, Greg Banks wrote:
> The file nfsfh.c contains two static variables nfsd_nr_verified and
> nfsd_nr_put. These are counters which are incremented as a side
> effect of the fh_verify() fh_compose() and fh_put() operations,
> i.e. at least twice per NFS call for any non-trivial workload.
> Needless to say this makes the cacheline that contains them (and any
> other innocent victims) a very hot contention point indeed under high
> call-rate workloads on multiprocessor NFS server. It also turns out
> that these counters are not used anywhere. They're not reported to
> userspace, they're not used in logic, they're not even exported from
> the object file (let alone the module). All they do is waste CPU time.
>
> So this patch removes them.
Clearly right, thanks! Applied for 2.6.31.
>
> Tests on a 16 CPU Altix A4700 with 2 10gige Myricom cards, configured
> separately (no bonding). Workload is 640 client threads doing directory
> traverals with random small reads, from server RAM.
>
> Before
> ======
>
> Kernel profile:
>
> % cumulative self self total
> time samples samples calls 1/call 1/call name
> 6.05 2716.00 2716.00 30406 0.09 1.02 svc_process
> 4.44 4706.00 1990.00 1975 1.01 1.01 spin_unlock_irqrestore
> 3.72 6376.00 1670.00 1666 1.00 1.00 svc_export_put
> 3.41 7907.00 1531.00 1786 0.86 1.02 nfsd_ofcache_lookup
> 3.25 9363.00 1456.00 10965 0.13 1.01 nfsd_dispatch
> 3.10 10752.00 1389.00 1376 1.01 1.01 nfsd_cache_lookup
> 2.57 11907.00 1155.00 4517 0.26 1.03 svc_tcp_recvfrom
> ...
> 2.21 15352.00 1003.00 1081 0.93 1.00 nfsd_choose_ofc <----
> ^^^^
>
> Here the function nfsd_choose_ofc() reads a global variable
> which by accident happened to be located in the same cacheline as
> nfsd_nr_verified.
nfsd_choose_ofc is something not in mainline? But, OK, it's interesting
in any case to see that this small a change is measureable.--b.
>
> Call rate:
>
> nullarbor:~ # pmdumptext nfs3.server.calls
> ...
> Thu Dec 13 00:15:27 184780.663
> Thu Dec 13 00:15:28 184885.881
> Thu Dec 13 00:15:29 184449.215
> Thu Dec 13 00:15:30 184971.058
> Thu Dec 13 00:15:31 185036.052
> Thu Dec 13 00:15:32 185250.475
> Thu Dec 13 00:15:33 184481.319
> Thu Dec 13 00:15:34 185225.737
> Thu Dec 13 00:15:35 185408.018
> Thu Dec 13 00:15:36 185335.764
>
>
> After
> =====
>
> kernel profile:
>
> % cumulative self self total
> time samples samples calls 1/call 1/call name
> 6.33 2813.00 2813.00 29979 0.09 1.01 svc_process
> 4.66 4883.00 2070.00 2065 1.00 1.00 spin_unlock_irqrestore
> 4.06 6687.00 1804.00 2182 0.83 1.00 nfsd_ofcache_lookup
> 3.20 8110.00 1423.00 10932 0.13 1.00 nfsd_dispatch
> 3.03 9456.00 1346.00 1343 1.00 1.00 nfsd_cache_lookup
> 2.62 10622.00 1166.00 4645 0.25 1.01 svc_tcp_recvfrom
> [...]
> 0.10 42586.00 44.00 74 0.59 1.00 nfsd_choose_ofc <--- HA!!
> ^^^^
>
> Call rate:
>
> nullarbor:~ # pmdumptext nfs3.server.calls
> ...
> Thu Dec 13 01:45:28 194677.118
> Thu Dec 13 01:45:29 193932.692
> Thu Dec 13 01:45:30 194294.364
> Thu Dec 13 01:45:31 194971.276
> Thu Dec 13 01:45:32 194111.207
> Thu Dec 13 01:45:33 194999.635
> Thu Dec 13 01:45:34 195312.594
> Thu Dec 13 01:45:35 195707.293
> Thu Dec 13 01:45:36 194610.353
> Thu Dec 13 01:45:37 195913.662
> Thu Dec 13 01:45:38 194808.675
>
> i.e. about a 5.3% improvement in call rate.
>
> Signed-off-by: Greg Banks <gnb-cP1dWloDopni96+mSzHFpQC/[email protected]>
> Reviewed-by: David Chinner <[email protected]>
> ---
>
> fs/nfsd/nfsfh.c | 6 ------
> 1 file changed, 6 deletions(-)
>
> Index: bfields/fs/nfsd/nfsfh.c
> ===================================================================
> --- bfields.orig/fs/nfsd/nfsfh.c
> +++ bfields/fs/nfsd/nfsfh.c
> @@ -27,9 +27,6 @@
> #define NFSDDBG_FACILITY NFSDDBG_FH
>
>
> -static int nfsd_nr_verified;
> -static int nfsd_nr_put;
> -
> /*
> * our acceptability function.
> * if NOSUBTREECHECK, accept anything
> @@ -251,7 +248,6 @@ static __be32 nfsd_set_fh_dentry(struct
>
> fhp->fh_dentry = dentry;
> fhp->fh_export = exp;
> - nfsd_nr_verified++;
> return 0;
> out:
> exp_put(exp);
> @@ -552,7 +548,6 @@ fh_compose(struct svc_fh *fhp, struct sv
> return nfserr_opnotsupp;
> }
>
> - nfsd_nr_verified++;
> return 0;
> }
>
> @@ -609,7 +604,6 @@ fh_put(struct svc_fh *fhp)
> fhp->fh_pre_saved = 0;
> fhp->fh_post_saved = 0;
> #endif
> - nfsd_nr_put++;
> }
> if (exp) {
> cache_put(&exp->h, &svc_export_cache);
>
> --
> Greg