| Message ID | 20170509092010.30752-30-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
T24gVHVlLCAyMDE3LTA1LTA5IGF0IDExOjIwICswMjAwLCBDaHJpc3RvcGggSGVsbHdpZyB3cm90 ZToNCj4gcGNfY291bnQgaXMgdGhlIG9ubHkgd3JpdGVhYmxlIG1lbWViZXIgb2Ygc3RydWN0IHN2 Y19wcm9jaW5mbywgd2hpY2gNCj4gaXMNCj4gYSBnb29kIGNhbmRpZGF0ZSB0byBiZSBjb25zdC1p ZmllZCBhcyBpdCBjb250YWlucyBmdW5jdGlvbiBwb2ludGVycy4NCj4gDQo+IFRoaXMgcGF0Y2gg anVzdCByZW1vdmVzIGl0IGFuZCByZXR1cm5zIHplcm8gaW4gdGhlIC9wcm9jIGZpbGUgYXMgdGhl DQo+IGNvdW50IG9mIGNhbGxzIHBlciBwcm9jZWR1cmUgZG9lc24ndCBzZWVtIGFsbCB0aGF0IHVz ZWZ1bC7CoMKgQnV0IGl0J3MNCj4ganVzdCBhIGR1bWIgaGFjayBhbmQgd2UgbWlnaHQgbmVlZCBh IHByb3BlciBmaXggaW5zdGVhZC4NCj4gDQo+IFNpZ25lZC1vZmYtYnk6IENocmlzdG9waCBIZWxs d2lnIDxoY2hAbHN0LmRlPg0KPiAtLS0NCj4gDQoNCkZvciBub3csIHdoeSBub3QganVzdCBtb3Zl IGl0IHRvIGEgc2VwYXJhdGUgbm9uLWNvbnN0IHN0cnVjdHVyZSB0aGF0IGlzDQpwb2ludGVkIHRv IGJ5IHRoZSBjb25zdCBvbmU/IFdlIHNob3VsZG4ndCBkZWxpYmVyYXRlbHkgYnJlYWsgbmZzc3Rh dC4uLg0KDQotLSANClRyb25kIE15a2xlYnVzdA0KTGludXggTkZTIGNsaWVudCBtYWludGFpbmVy LCBQcmltYXJ5RGF0YQ0KdHJvbmQubXlrbGVidXN0QHByaW1hcnlkYXRhLmNvbQ0K -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> On May 9, 2017, at 8:16 AM, Trond Myklebust <trondmy@primarydata.com> wrote: > >> On Tue, 2017-05-09 at 11:20 +0200, Christoph Hellwig wrote: >> pc_count is the only writeable memeber of struct svc_procinfo, which >> is >> a good candidate to be const-ified as it contains function pointers. >> >> This patch just removes it and returns zero in the /proc file as the >> count of calls per procedure doesn't seem all that useful.  But it's >> just a dumb hack and we might need a proper fix instead. >> >> Signed-off-by: Christoph Hellwig <hch@lst.de> >> --- >> > > For now, why not just move it to a separate non-const structure that is > pointed to by the const one? We shouldn't deliberately break nfsstat... Agree on both counts. > -- > Trond Myklebust > Linux NFS client maintainer, PrimaryData > trond.myklebust@primarydata.com > N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±û"žØ^n‡r¡ö¦zËëh™¨èÚ&¢ø ®G«éh®(階ŠÝ¢j"ú¶m§ÿïêäz¹Þ–Šàþf£¢·hšˆ§~ˆmš -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2017-05-09 at 11:20 +0200, Christoph Hellwig wrote: > pc_count is the only writeable memeber of struct svc_procinfo, which is > a good candidate to be const-ified as it contains function pointers. > > This patch just removes it and returns zero in the /proc file as the > count of calls per procedure doesn't seem all that useful. But it's > just a dumb hack and we might need a proper fix instead. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > include/linux/sunrpc/svc.h | 1 - > net/sunrpc/stats.c | 2 +- > net/sunrpc/svc.c | 3 --- > 3 files changed, 1 insertion(+), 5 deletions(-) > > diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h > index 8eeb39286422..cab77b0904b0 100644 > --- a/include/linux/sunrpc/svc.h > +++ b/include/linux/sunrpc/svc.h > @@ -430,7 +430,6 @@ struct svc_procedure { > void (*pc_release)(struct svc_rqst *); > unsigned int pc_argsize; /* argument struct size */ > unsigned int pc_ressize; /* result struct size */ > - unsigned int pc_count; /* call count */ > unsigned int pc_cachetype; /* cache info (NFS) */ > unsigned int pc_xdrressize; /* maximum size of XDR reply */ > }; > diff --git a/net/sunrpc/stats.c b/net/sunrpc/stats.c > index 42053510b96b..f37d0fc835d7 100644 > --- a/net/sunrpc/stats.c > +++ b/net/sunrpc/stats.c > @@ -102,7 +102,7 @@ void svc_seq_show(struct seq_file *seq, const struct svc_stat *statp) { > continue; > seq_printf(seq, "proc%d %u", i, vers->vs_nproc); > for (j = 0; j < vers->vs_nproc; j++, proc++) > - seq_printf(seq, " %u", proc->pc_count); > + seq_printf(seq, " %u", 0); The rest of the patches also look fine, aside from this one where I agree with Trond and Chuck that we don't want to break nfsstat. > seq_putc(seq, '\n'); > } > } > diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c > index bbf8d938812e..af74604fdadd 100644 > --- a/net/sunrpc/svc.c > +++ b/net/sunrpc/svc.c > @@ -1202,9 +1202,6 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv) > statp = resv->iov_base +resv->iov_len; > svc_putnl(resv, RPC_SUCCESS); > > - /* Bump per-procedure stats counter */ > - procp->pc_count++; > - > /* Initialize storage for argp and resp */ > memset(rqstp->rq_argp, 0, procp->pc_argsize); > memset(rqstp->rq_resp, 0, procp->pc_ressize); Agree with Chuck and Trond's comments on
diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h index 8eeb39286422..cab77b0904b0 100644 --- a/include/linux/sunrpc/svc.h +++ b/include/linux/sunrpc/svc.h @@ -430,7 +430,6 @@ struct svc_procedure { void (*pc_release)(struct svc_rqst *); unsigned int pc_argsize; /* argument struct size */ unsigned int pc_ressize; /* result struct size */ - unsigned int pc_count; /* call count */ unsigned int pc_cachetype; /* cache info (NFS) */ unsigned int pc_xdrressize; /* maximum size of XDR reply */ }; diff --git a/net/sunrpc/stats.c b/net/sunrpc/stats.c index 42053510b96b..f37d0fc835d7 100644 --- a/net/sunrpc/stats.c +++ b/net/sunrpc/stats.c @@ -102,7 +102,7 @@ void svc_seq_show(struct seq_file *seq, const struct svc_stat *statp) { continue; seq_printf(seq, "proc%d %u", i, vers->vs_nproc); for (j = 0; j < vers->vs_nproc; j++, proc++) - seq_printf(seq, " %u", proc->pc_count); + seq_printf(seq, " %u", 0); seq_putc(seq, '\n'); } } diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c index bbf8d938812e..af74604fdadd 100644 --- a/net/sunrpc/svc.c +++ b/net/sunrpc/svc.c @@ -1202,9 +1202,6 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv) statp = resv->iov_base +resv->iov_len; svc_putnl(resv, RPC_SUCCESS); - /* Bump per-procedure stats counter */ - procp->pc_count++; - /* Initialize storage for argp and resp */ memset(rqstp->rq_argp, 0, procp->pc_argsize); memset(rqstp->rq_resp, 0, procp->pc_ressize);
pc_count is the only writeable memeber of struct svc_procinfo, which is a good candidate to be const-ified as it contains function pointers. This patch just removes it and returns zero in the /proc file as the count of calls per procedure doesn't seem all that useful. But it's just a dumb hack and we might need a proper fix instead. Signed-off-by: Christoph Hellwig <hch@lst.de> --- include/linux/sunrpc/svc.h | 1 - net/sunrpc/stats.c | 2 +- net/sunrpc/svc.c | 3 --- 3 files changed, 1 insertion(+), 5 deletions(-)