2012-02-17 18:15:26

by Weston Andros Adamson

[permalink] [raw]
Subject: [PATCH] NFS: include filelayout DS rpc stats in mountstats

Include RPC statistics from all data servers in /proc/self/mountstats for pNFS
filelayout mounts.

Signed-off-by: Weston Andros Adamson <[email protected]>
---
Update: don't increment tk_client stats if callback is used.

This still checks task->tk_client != NULL -- I'm not convinced it's unneeded
as it existed in rpc_count_iostats.

fs/nfs/nfs4filelayout.c | 19 +++++++++++++++++++
include/linux/sunrpc/metrics.h | 6 ++++--
include/linux/sunrpc/sched.h | 1 +
net/sunrpc/stats.c | 8 ++++----
net/sunrpc/xprt.c | 5 ++++-
5 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/fs/nfs/nfs4filelayout.c b/fs/nfs/nfs4filelayout.c
index 79be7ac..47e8f34 100644
--- a/fs/nfs/nfs4filelayout.c
+++ b/fs/nfs/nfs4filelayout.c
@@ -33,6 +33,8 @@
#include <linux/nfs_page.h>
#include <linux/module.h>

+#include <linux/sunrpc/metrics.h>
+
#include "internal.h"
#include "nfs4filelayout.h"

@@ -189,6 +191,13 @@ static void filelayout_read_call_done(struct rpc_task *task, void *data)
rdata->mds_ops->rpc_call_done(task, data);
}

+static void filelayout_read_count_stats(struct rpc_task *task, void *data)
+{
+ struct nfs_read_data *rdata = (struct nfs_read_data *)data;
+
+ rpc_count_iostats(task, NFS_SERVER(rdata->inode)->client->cl_metrics);
+}
+
static void filelayout_read_release(void *data)
{
struct nfs_read_data *rdata = (struct nfs_read_data *)data;
@@ -268,6 +277,13 @@ static void filelayout_write_call_done(struct rpc_task *task, void *data)
wdata->mds_ops->rpc_call_done(task, data);
}

+static void filelayout_write_count_stats(struct rpc_task *task, void *data)
+{
+ struct nfs_write_data *wdata = (struct nfs_write_data *)data;
+
+ rpc_count_iostats(task, NFS_SERVER(wdata->inode)->client->cl_metrics);
+}
+
static void filelayout_write_release(void *data)
{
struct nfs_write_data *wdata = (struct nfs_write_data *)data;
@@ -288,18 +304,21 @@ static void filelayout_commit_release(void *data)
struct rpc_call_ops filelayout_read_call_ops = {
.rpc_call_prepare = filelayout_read_prepare,
.rpc_call_done = filelayout_read_call_done,
+ .rpc_count_stats = filelayout_read_count_stats,
.rpc_release = filelayout_read_release,
};

struct rpc_call_ops filelayout_write_call_ops = {
.rpc_call_prepare = filelayout_write_prepare,
.rpc_call_done = filelayout_write_call_done,
+ .rpc_count_stats = filelayout_write_count_stats,
.rpc_release = filelayout_write_release,
};

struct rpc_call_ops filelayout_commit_call_ops = {
.rpc_call_prepare = filelayout_write_prepare,
.rpc_call_done = filelayout_write_call_done,
+ .rpc_count_stats = filelayout_write_count_stats,
.rpc_release = filelayout_commit_release,
};

diff --git a/include/linux/sunrpc/metrics.h b/include/linux/sunrpc/metrics.h
index b6edbc0..1565bbe 100644
--- a/include/linux/sunrpc/metrics.h
+++ b/include/linux/sunrpc/metrics.h
@@ -74,14 +74,16 @@ struct rpc_clnt;
#ifdef CONFIG_PROC_FS

struct rpc_iostats * rpc_alloc_iostats(struct rpc_clnt *);
-void rpc_count_iostats(struct rpc_task *);
+void rpc_count_iostats(const struct rpc_task *,
+ struct rpc_iostats *);
void rpc_print_iostats(struct seq_file *, struct rpc_clnt *);
void rpc_free_iostats(struct rpc_iostats *);

#else /* CONFIG_PROC_FS */

static inline struct rpc_iostats *rpc_alloc_iostats(struct rpc_clnt *clnt) { return NULL; }
-static inline void rpc_count_iostats(struct rpc_task *task) {}
+static inline void rpc_count_iostats(const struct rpc_task *task,
+ struct rpc_iostats *stats) {}
static inline void rpc_print_iostats(struct seq_file *seq, struct rpc_clnt *clnt) {}
static inline void rpc_free_iostats(struct rpc_iostats *stats) {}

diff --git a/include/linux/sunrpc/sched.h b/include/linux/sunrpc/sched.h
index 22dfc24..dc0c3cc 100644
--- a/include/linux/sunrpc/sched.h
+++ b/include/linux/sunrpc/sched.h
@@ -103,6 +103,7 @@ typedef void (*rpc_action)(struct rpc_task *);
struct rpc_call_ops {
void (*rpc_call_prepare)(struct rpc_task *, void *);
void (*rpc_call_done)(struct rpc_task *, void *);
+ void (*rpc_count_stats)(struct rpc_task *, void *);
void (*rpc_release)(void *);
};

diff --git a/net/sunrpc/stats.c b/net/sunrpc/stats.c
index 3c4f688..1eb3304 100644
--- a/net/sunrpc/stats.c
+++ b/net/sunrpc/stats.c
@@ -133,20 +133,19 @@ EXPORT_SYMBOL_GPL(rpc_free_iostats);
/**
* rpc_count_iostats - tally up per-task stats
* @task: completed rpc_task
+ * @stats: array of stat structures
*
* Relies on the caller for serialization.
*/
-void rpc_count_iostats(struct rpc_task *task)
+void rpc_count_iostats(const struct rpc_task *task, struct rpc_iostats *stats)
{
struct rpc_rqst *req = task->tk_rqstp;
- struct rpc_iostats *stats;
struct rpc_iostats *op_metrics;
ktime_t delta;

- if (!task->tk_client || !task->tk_client->cl_metrics || !req)
+ if (!stats || !req)
return;

- stats = task->tk_client->cl_metrics;
op_metrics = &stats[task->tk_msg.rpc_proc->p_statidx];

op_metrics->om_ops++;
@@ -164,6 +163,7 @@ void rpc_count_iostats(struct rpc_task *task)
delta = ktime_sub(ktime_get(), task->tk_start);
op_metrics->om_execute = ktime_add(op_metrics->om_execute, delta);
}
+EXPORT_SYMBOL_GPL(rpc_count_iostats);

static void _print_name(struct seq_file *seq, unsigned int op,
struct rpc_procinfo *procs)
diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
index efe5495..100b469 100644
--- a/net/sunrpc/xprt.c
+++ b/net/sunrpc/xprt.c
@@ -1132,7 +1132,10 @@ void xprt_release(struct rpc_task *task)
return;

xprt = req->rq_xprt;
- rpc_count_iostats(task);
+ if (task->tk_ops->rpc_count_stats != NULL)
+ task->tk_ops->rpc_count_stats(task, task->tk_calldata);
+ else if (task->tk_client)
+ rpc_count_iostats(task, task->tk_client->cl_metrics);
spin_lock_bh(&xprt->transport_lock);
xprt->ops->release_xprt(xprt, task);
if (xprt->ops->release_request)
--
1.7.4.4



2012-02-09 20:47:55

by Adamson, Dros

[permalink] [raw]
Subject: Re: [PATCH] NFS: include filelayout DS rpc stats in mountstats

On Feb 9, 2012, at 3:37 PM, Myklebust, Trond wrote:

> On Thu, 2012-02-09 at 20:23 +0000, Adamson, Dros wrote:
>> On Feb 9, 2012, at 2:58 PM, Myklebust, Trond wrote:
>>
>>> On Thu, 2012-02-09 at 19:48 +0000, Adamson, Dros wrote:
>>>>>
>>>>>> I do have an implementation that doesn't need to walk the deviceid list by allowing a shared rpc_iostats struct between multiple rpc_clients (in addition to the current rpc_iostats structure), but that required adding locking and reference counting -- all for printing stats (obviously not what we want).
>>>>>
>>>>> This might be more in line with what we want. Note that it should be
>>>>> easy to clone an rpc_client and then replace its rpc_iostats struct. I
>>>>> don't think that needs any extra locking. We're already ignoring locking
>>>>> here between different rpc_tasks, so throwing in different rpc_clients
>>>>> to the mix will make no difference.
>>>>
>>>> Yeah, that's easy enough and i guess we could ignore locking, but we are still left with the same problem: how is this supposed to account for different mount points using the same nfs_client? nfs_client only has one rpc_client member. I doubt we want to make a hash lookup on nfs_server to get the right rpc_client (which could all use the same underlying xprt).
>>>>
>>>> Maybe it's time to move these stats into fs/nfs/ where they really belong? We could get rid of the hack that overloads procnum with opnum from inside the compound for v4+ and finally only show a specific mount's RPC stats.
>>>
>>> Actually, how about just adding a callback into struct rpc_call_ops
>>> that, if it is defined, would be called instead of rpc_count_iostats().
>>> If you then modify rpc_count_iostats() to take the 'stats' variable as a
>>> parameter, you can simply have the new callback call rpc_count_iostats
>>> with the right arguments.
>>
>> Yeah, that could work! On my walk home from the cafe (they always seem to help) I came up with a slightly different strategy:
>>
>> - remove the cl_metrics (struct rpc_iostats) member from rpc_clnt
>> - add an *optional* rpc_iostats pointer to rpc_task (ignored if NULL)
>> - allocate one rpc_iostats structure (really array of structs, but you know what I mean) per nfs_server structure
>> - when setting up rpc_tasks in nfs-land, pass the correct rpc_iostats
>>
>> with this strategy we don't accumulate stats when they aren't ever used (like an rpc_client used for mount protocol). again, only NFS uses the rpc stats interface.
>>
>> I kind of like this better than a callback, but either way is fine with me. Which way would you prefer?
>
> I'd prefer not to have to grow the size of the rpc_task unless we really
> need to. That's why I suggested the callback instead.

Good to know! I'll do the callback method.

Also, I'm going to go ahead with removing the cl_metrics member from rpc_clnt -- once we have the callback it'll never be used.

Thanks for pointing me in the right direction. Expect a patch soon!

-dros


Attachments:
smime.p7s (1.34 kB)

2012-02-09 19:59:02

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH] NFS: include filelayout DS rpc stats in mountstats

T24gVGh1LCAyMDEyLTAyLTA5IGF0IDE5OjQ4ICswMDAwLCBBZGFtc29uLCBEcm9zIHdyb3RlOg0K
PiA+IA0KPiA+PiBJIGRvIGhhdmUgYW4gaW1wbGVtZW50YXRpb24gdGhhdCBkb2Vzbid0IG5lZWQg
dG8gd2FsayB0aGUgZGV2aWNlaWQgbGlzdCBieSBhbGxvd2luZyBhIHNoYXJlZCBycGNfaW9zdGF0
cyBzdHJ1Y3QgYmV0d2VlbiBtdWx0aXBsZSBycGNfY2xpZW50cyAoaW4gYWRkaXRpb24gdG8gdGhl
IGN1cnJlbnQgcnBjX2lvc3RhdHMgc3RydWN0dXJlKSwgYnV0IHRoYXQgcmVxdWlyZWQgYWRkaW5n
IGxvY2tpbmcgYW5kIHJlZmVyZW5jZSBjb3VudGluZyAtLSBhbGwgZm9yIHByaW50aW5nIHN0YXRz
IChvYnZpb3VzbHkgbm90IHdoYXQgd2Ugd2FudCkuDQo+ID4gDQo+ID4gVGhpcyBtaWdodCBiZSBt
b3JlIGluIGxpbmUgd2l0aCB3aGF0IHdlIHdhbnQuIE5vdGUgdGhhdCBpdCBzaG91bGQgYmUNCj4g
PiBlYXN5IHRvIGNsb25lIGFuIHJwY19jbGllbnQgYW5kIHRoZW4gcmVwbGFjZSBpdHMgcnBjX2lv
c3RhdHMgc3RydWN0LiBJDQo+ID4gZG9uJ3QgdGhpbmsgdGhhdCBuZWVkcyBhbnkgZXh0cmEgbG9j
a2luZy4gV2UncmUgYWxyZWFkeSBpZ25vcmluZyBsb2NraW5nDQo+ID4gaGVyZSBiZXR3ZWVuIGRp
ZmZlcmVudCBycGNfdGFza3MsIHNvIHRocm93aW5nIGluIGRpZmZlcmVudCBycGNfY2xpZW50cw0K
PiA+IHRvIHRoZSBtaXggd2lsbCBtYWtlIG5vIGRpZmZlcmVuY2UuDQo+IA0KPiBZZWFoLCB0aGF0
J3MgZWFzeSBlbm91Z2ggYW5kIGkgZ3Vlc3Mgd2UgY291bGQgaWdub3JlIGxvY2tpbmcsIGJ1dCB3
ZSBhcmUgc3RpbGwgbGVmdCB3aXRoIHRoZSBzYW1lIHByb2JsZW06IGhvdyBpcyB0aGlzIHN1cHBv
c2VkIHRvIGFjY291bnQgZm9yIGRpZmZlcmVudCBtb3VudCBwb2ludHMgdXNpbmcgdGhlIHNhbWUg
bmZzX2NsaWVudD8gIG5mc19jbGllbnQgb25seSBoYXMgb25lIHJwY19jbGllbnQgbWVtYmVyLiAg
SSBkb3VidCB3ZSB3YW50IHRvIG1ha2UgYSBoYXNoIGxvb2t1cCBvbiBuZnNfc2VydmVyIHRvIGdl
dCB0aGUgcmlnaHQgcnBjX2NsaWVudCAod2hpY2ggY291bGQgYWxsIHVzZSB0aGUgc2FtZSB1bmRl
cmx5aW5nIHhwcnQpLg0KPiANCj4gTWF5YmUgaXQncyB0aW1lIHRvIG1vdmUgdGhlc2Ugc3RhdHMg
aW50byBmcy9uZnMvIHdoZXJlIHRoZXkgcmVhbGx5IGJlbG9uZz8gIFdlIGNvdWxkIGdldCByaWQg
b2YgdGhlIGhhY2sgdGhhdCBvdmVybG9hZHMgcHJvY251bSB3aXRoIG9wbnVtIGZyb20gaW5zaWRl
IHRoZSBjb21wb3VuZCBmb3IgdjQrIGFuZCBmaW5hbGx5IG9ubHkgc2hvdyBhIHNwZWNpZmljIG1v
dW50J3MgUlBDIHN0YXRzLg0KDQpBY3R1YWxseSwgaG93IGFib3V0IGp1c3QgYWRkaW5nIGEgY2Fs
bGJhY2sgaW50byBzdHJ1Y3QgcnBjX2NhbGxfb3BzDQp0aGF0LCBpZiBpdCBpcyBkZWZpbmVkLCB3
b3VsZCBiZSBjYWxsZWQgaW5zdGVhZCBvZiBycGNfY291bnRfaW9zdGF0cygpLiANCklmIHlvdSB0
aGVuIG1vZGlmeSBycGNfY291bnRfaW9zdGF0cygpIHRvIHRha2UgdGhlICdzdGF0cycgdmFyaWFi
bGUgYXMgYQ0KcGFyYW1ldGVyLCB5b3UgY2FuIHNpbXBseSBoYXZlIHRoZSBuZXcgY2FsbGJhY2sg
Y2FsbCBycGNfY291bnRfaW9zdGF0cw0Kd2l0aCB0aGUgcmlnaHQgYXJndW1lbnRzLg0KDQotLSAN
ClRyb25kIE15a2xlYnVzdA0KTGludXggTkZTIGNsaWVudCBtYWludGFpbmVyDQoNCk5ldEFwcA0K
VHJvbmQuTXlrbGVidXN0QG5ldGFwcC5jb20NCnd3dy5uZXRhcHAuY29tDQoNCg==

2012-02-16 21:08:26

by Adamson, Dros

[permalink] [raw]
Subject: Re: [PATCH] NFS: include filelayout DS rpc stats in mountstats


On Feb 16, 2012, at 4:01 PM, Myklebust, Trond wrote:

> On Thu, 2012-02-16 at 20:44 +0000, Adamson, Dros wrote:
>> On Feb 16, 2012, at 2:48 PM, Myklebust, Trond wrote:
>>> 2. Shouldn't we be calling _either_ rpc_count_iostats(), _or_
>>> task->tk_ops->rpc_count_stats()? As far as I can see, the
>>> nfsstat output will now be double-counting and pNFS reads,
>>> writes and commits that are sent to the DS.
>>
>>
>> No, this doesn't double count with nfsstats. nfsstats uses rpc_clnt::cl_stats (and not rpc_clnt::cl_metrics).
>>
>> I probably should have done an if/else for this patch -- with the current code, the DSs' rpc_clnt:cl_metrics will never be used.
>> I left it accumulating here because we want to have per-DS stats eventually and my plan was to print out stats in /proc/fs/nfsfs per *client* (so not separated by mountpoint).
>>
>> I can repost with if/else, but looking at this some more made me realize that we are *still* doing this wrong :)
>>
>> The callback method in this patch fails to accumulate stats for operations to the DS other than read/write/commit -- that seems right, but what about null, exchange_id, session heartbeats, etc?
>>
>> In order to properly accumulate those we are back to two obvious choices:
>> 1) add a count_iostats callback to the ~25 other rpc_call_ops in nfs-land (yuck)
>> 2) add an 'additional stats' pointer to the rpc_task structure (trond already said you don't want to add to task struct)
>>
>> Or do we just not care about displaying those operations? For my purposes (nfsometer perf testing), it'd be nice to have *all* of the operations.
>
> As far as I'm concerned, the administrative traffic to the DS should not
> be accounted for in the mountstats: that would be wrong since DSes can
> be shared not only by different filesystems but even by different MDSes.
>
> So the performance overhead of lease and session setup to the DSes needs
> to be accounted for by some other mechanism.

Fair enough. The per-client stats should help me with that.

I'll repost with if/else and change it back with the next stats patch.

-dros


Attachments:
smime.p7s (1.34 kB)

2012-02-17 19:04:33

by Myklebust, Trond

[permalink] [raw]
Subject: RE: [PATCH] NFS: include filelayout DS rpc stats in mountstats

PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBBZGFtc29uLCBEcm9zDQo+IFNl
bnQ6IEZyaWRheSwgRmVicnVhcnkgMTcsIDIwMTIgMjowMiBQTQ0KPiBUbzogTXlrbGVidXN0LCBU
cm9uZA0KPiBDYzogQWRhbXNvbiwgRHJvczsgbGludXgtbmZzQHZnZXIua2VybmVsLm9yZw0KPiBT
dWJqZWN0OiBSZTogW1BBVENIXSBORlM6IGluY2x1ZGUgZmlsZWxheW91dCBEUyBycGMgc3RhdHMg
aW4gbW91bnRzdGF0cw0KPiANCj4gT24gRmViIDE3LCAyMDEyLCBhdCAxOjQ3IFBNLCBNeWtsZWJ1
c3QsIFRyb25kIHdyb3RlOg0KPiANCj4gPiBPbiBGcmksIDIwMTItMDItMTcgYXQgMTM6MTUgLTA1
MDAsIFdlc3RvbiBBbmRyb3MgQWRhbXNvbiB3cm90ZToNCj4gPj4gSW5jbHVkZSBSUEMgc3RhdGlz
dGljcyBmcm9tIGFsbCBkYXRhIHNlcnZlcnMgaW4gL3Byb2Mvc2VsZi9tb3VudHN0YXRzIGZvcg0K
PiBwTkZTDQo+ID4+IGZpbGVsYXlvdXQgbW91bnRzLg0KPiA+Pg0KPiA+PiBTaWduZWQtb2ZmLWJ5
OiBXZXN0b24gQW5kcm9zIEFkYW1zb24gPGRyb3NAbmV0YXBwLmNvbT4NCj4gPj4gLS0tDQo+ID4+
IFVwZGF0ZTogZG9uJ3QgaW5jcmVtZW50IHRrX2NsaWVudCBzdGF0cyBpZiBjYWxsYmFjayBpcyB1
c2VkLg0KPiA+Pg0KPiA+PiBUaGlzIHN0aWxsIGNoZWNrcyB0YXNrLT50a19jbGllbnQgIT0gTlVM
TCAtLSBJJ20gbm90IGNvbnZpbmNlZCBpdCdzIHVubmVlZGVkDQo+ID4+IGFzIGl0IGV4aXN0ZWQg
aW4gcnBjX2NvdW50X2lvc3RhdHMuDQo+ID4NCj4gPiBJJ2xsIHJlbW92ZSBpdCBiZWZvcmUgYXBw
bHlpbmcuDQo+IA0KPiBJIGNhbiByZXBvc3QgdGhlIHBhdGNoLCBpZiB0aGF0J3MgZWFzaWVyLg0K
DQpOYWguIEkndmUgYWxyZWFkeSBhcHBsaWVkIGFuZCBwdXNoZWQgdG8gdGhlIGdpdCByZXBvc2l0
b3J5Lg0KDQo=

2012-02-09 20:00:21

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH] NFS: include filelayout DS rpc stats in mountstats

T24gVGh1LCAyMDEyLTAyLTA5IGF0IDE5OjQ4ICswMDAwLCBBZGFtc29uLCBEcm9zIHdyb3RlOg0K
PiA+IA0KPiA+PiBJIGRvIGhhdmUgYW4gaW1wbGVtZW50YXRpb24gdGhhdCBkb2Vzbid0IG5lZWQg
dG8gd2FsayB0aGUgZGV2aWNlaWQgbGlzdCBieSBhbGxvd2luZyBhIHNoYXJlZCBycGNfaW9zdGF0
cyBzdHJ1Y3QgYmV0d2VlbiBtdWx0aXBsZSBycGNfY2xpZW50cyAoaW4gYWRkaXRpb24gdG8gdGhl
IGN1cnJlbnQgcnBjX2lvc3RhdHMgc3RydWN0dXJlKSwgYnV0IHRoYXQgcmVxdWlyZWQgYWRkaW5n
IGxvY2tpbmcgYW5kIHJlZmVyZW5jZSBjb3VudGluZyAtLSBhbGwgZm9yIHByaW50aW5nIHN0YXRz
IChvYnZpb3VzbHkgbm90IHdoYXQgd2Ugd2FudCkuDQo+ID4gDQo+ID4gVGhpcyBtaWdodCBiZSBt
b3JlIGluIGxpbmUgd2l0aCB3aGF0IHdlIHdhbnQuIE5vdGUgdGhhdCBpdCBzaG91bGQgYmUNCj4g
PiBlYXN5IHRvIGNsb25lIGFuIHJwY19jbGllbnQgYW5kIHRoZW4gcmVwbGFjZSBpdHMgcnBjX2lv
c3RhdHMgc3RydWN0LiBJDQo+ID4gZG9uJ3QgdGhpbmsgdGhhdCBuZWVkcyBhbnkgZXh0cmEgbG9j
a2luZy4gV2UncmUgYWxyZWFkeSBpZ25vcmluZyBsb2NraW5nDQo+ID4gaGVyZSBiZXR3ZWVuIGRp
ZmZlcmVudCBycGNfdGFza3MsIHNvIHRocm93aW5nIGluIGRpZmZlcmVudCBycGNfY2xpZW50cw0K
PiA+IHRvIHRoZSBtaXggd2lsbCBtYWtlIG5vIGRpZmZlcmVuY2UuDQo+IA0KPiBZZWFoLCB0aGF0
J3MgZWFzeSBlbm91Z2ggYW5kIGkgZ3Vlc3Mgd2UgY291bGQgaWdub3JlIGxvY2tpbmcsIGJ1dCB3
ZSBhcmUgc3RpbGwgbGVmdCB3aXRoIHRoZSBzYW1lIHByb2JsZW06IGhvdyBpcyB0aGlzIHN1cHBv
c2VkIHRvIGFjY291bnQgZm9yIGRpZmZlcmVudCBtb3VudCBwb2ludHMgdXNpbmcgdGhlIHNhbWUg
bmZzX2NsaWVudD8gIG5mc19jbGllbnQgb25seSBoYXMgb25lIHJwY19jbGllbnQgbWVtYmVyLiAg
SSBkb3VidCB3ZSB3YW50IHRvIG1ha2UgYSBoYXNoIGxvb2t1cCBvbiBuZnNfc2VydmVyIHRvIGdl
dCB0aGUgcmlnaHQgcnBjX2NsaWVudCAod2hpY2ggY291bGQgYWxsIHVzZSB0aGUgc2FtZSB1bmRl
cmx5aW5nIHhwcnQpLg0KPiANCj4gTWF5YmUgaXQncyB0aW1lIHRvIG1vdmUgdGhlc2Ugc3RhdHMg
aW50byBmcy9uZnMvIHdoZXJlIHRoZXkgcmVhbGx5IGJlbG9uZz8gIFdlIGNvdWxkIGdldCByaWQg
b2YgdGhlIGhhY2sgdGhhdCBvdmVybG9hZHMgcHJvY251bSB3aXRoIG9wbnVtIGZyb20gaW5zaWRl
IHRoZSBjb21wb3VuZCBmb3IgdjQrIGFuZCBmaW5hbGx5IG9ubHkgc2hvdyBhIHNwZWNpZmljIG1v
dW50J3MgUlBDIHN0YXRzLg0KDQpBY3R1YWxseSwgaG93IGFib3V0IGp1c3QgYWRkaW5nIGEgY2Fs
bGJhY2sgaW50byBzdHJ1Y3QgcnBjX2NhbGxfb3BzDQp0aGF0LCBpZiBpdCBpcyBkZWZpbmVkLCB3
b3VsZCBiZSBjYWxsZWQgaW5zdGVhZCBvZiBycGNfY291bnRfaW9zdGF0cygpLiANCklmIHlvdSB0
aGVuIG1vZGlmeSBycGNfY291bnRfaW9zdGF0cygpIHRvIHRha2UgdGhlICdzdGF0cycgdmFyaWFi
bGUgYXMgYQ0KcGFyYW1ldGVyLCB5b3UgY2FuIHNpbXBseSBoYXZlIHRoZSBuZXcgY2FsbGJhY2sg
Y2FsbCBycGNfY291bnRfaW9zdGF0cw0Kd2l0aCB0aGUgcmlnaHQgYXJndW1lbnRzLg0KDQotLSAN
ClRyb25kIE15a2xlYnVzdA0KTGludXggTkZTIGNsaWVudCBtYWludGFpbmVyDQoNCk5ldEFwcA0K
VHJvbmQuTXlrbGVidXN0QG5ldGFwcC5jb20NCnd3dy5uZXRhcHAuY29tDQoNCg0K

2012-02-17 18:47:11

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH] NFS: include filelayout DS rpc stats in mountstats

T24gRnJpLCAyMDEyLTAyLTE3IGF0IDEzOjE1IC0wNTAwLCBXZXN0b24gQW5kcm9zIEFkYW1zb24g
d3JvdGU6DQo+IEluY2x1ZGUgUlBDIHN0YXRpc3RpY3MgZnJvbSBhbGwgZGF0YSBzZXJ2ZXJzIGlu
IC9wcm9jL3NlbGYvbW91bnRzdGF0cyBmb3IgcE5GUw0KPiBmaWxlbGF5b3V0IG1vdW50cy4NCj4g
DQo+IFNpZ25lZC1vZmYtYnk6IFdlc3RvbiBBbmRyb3MgQWRhbXNvbiA8ZHJvc0BuZXRhcHAuY29t
Pg0KPiAtLS0NCj4gVXBkYXRlOiBkb24ndCBpbmNyZW1lbnQgdGtfY2xpZW50IHN0YXRzIGlmIGNh
bGxiYWNrIGlzIHVzZWQuDQo+IA0KPiBUaGlzIHN0aWxsIGNoZWNrcyB0YXNrLT50a19jbGllbnQg
IT0gTlVMTCAtLSBJJ20gbm90IGNvbnZpbmNlZCBpdCdzIHVubmVlZGVkDQo+IGFzIGl0IGV4aXN0
ZWQgaW4gcnBjX2NvdW50X2lvc3RhdHMuDQoNCkknbGwgcmVtb3ZlIGl0IGJlZm9yZSBhcHBseWlu
Zy4geHBydF9yZXF1ZXN0X2luaXQgd2lsbCBPb3BzIHdpdGhvdXQNCnRhc2stPnRrX2NsaWVudCwg
c28geW91J3JlIGd1YXJhbnRlZWQgdGhhdCBpZiB0YXNrLT50a19ycXN0cCBpcw0Kbm9uLW51bGws
IHRoZW4gdGFzay0+dGtfY2xpZW50IGlzIGEgdmFsaWQgcG9pbnRlci4uLg0KDQo+ICBmcy9uZnMv
bmZzNGZpbGVsYXlvdXQuYyAgICAgICAgfCAgIDE5ICsrKysrKysrKysrKysrKysrKysNCj4gIGlu
Y2x1ZGUvbGludXgvc3VucnBjL21ldHJpY3MuaCB8ICAgIDYgKysrKy0tDQo+ICBpbmNsdWRlL2xp
bnV4L3N1bnJwYy9zY2hlZC5oICAgfCAgICAxICsNCj4gIG5ldC9zdW5ycGMvc3RhdHMuYyAgICAg
ICAgICAgICB8ICAgIDggKysrKy0tLS0NCj4gIG5ldC9zdW5ycGMveHBydC5jICAgICAgICAgICAg
ICB8ICAgIDUgKysrKy0NCj4gIDUgZmlsZXMgY2hhbmdlZCwgMzIgaW5zZXJ0aW9ucygrKSwgNyBk
ZWxldGlvbnMoLSkNCj4gDQo+IGRpZmYgLS1naXQgYS9mcy9uZnMvbmZzNGZpbGVsYXlvdXQuYyBi
L2ZzL25mcy9uZnM0ZmlsZWxheW91dC5jDQo+IGluZGV4IDc5YmU3YWMuLjQ3ZThmMzQgMTAwNjQ0
DQo+IC0tLSBhL2ZzL25mcy9uZnM0ZmlsZWxheW91dC5jDQo+ICsrKyBiL2ZzL25mcy9uZnM0Zmls
ZWxheW91dC5jDQo+IEBAIC0zMyw2ICszMyw4IEBADQo+ICAjaW5jbHVkZSA8bGludXgvbmZzX3Bh
Z2UuaD4NCj4gICNpbmNsdWRlIDxsaW51eC9tb2R1bGUuaD4NCj4gIA0KPiArI2luY2x1ZGUgPGxp
bnV4L3N1bnJwYy9tZXRyaWNzLmg+DQo+ICsNCj4gICNpbmNsdWRlICJpbnRlcm5hbC5oIg0KPiAg
I2luY2x1ZGUgIm5mczRmaWxlbGF5b3V0LmgiDQo+ICANCj4gQEAgLTE4OSw2ICsxOTEsMTMgQEAg
c3RhdGljIHZvaWQgZmlsZWxheW91dF9yZWFkX2NhbGxfZG9uZShzdHJ1Y3QgcnBjX3Rhc2sgKnRh
c2ssIHZvaWQgKmRhdGEpDQo+ICAJcmRhdGEtPm1kc19vcHMtPnJwY19jYWxsX2RvbmUodGFzaywg
ZGF0YSk7DQo+ICB9DQo+ICANCj4gK3N0YXRpYyB2b2lkIGZpbGVsYXlvdXRfcmVhZF9jb3VudF9z
dGF0cyhzdHJ1Y3QgcnBjX3Rhc2sgKnRhc2ssIHZvaWQgKmRhdGEpDQo+ICt7DQo+ICsJc3RydWN0
IG5mc19yZWFkX2RhdGEgKnJkYXRhID0gKHN0cnVjdCBuZnNfcmVhZF9kYXRhICopZGF0YTsNCj4g
Kw0KPiArCXJwY19jb3VudF9pb3N0YXRzKHRhc2ssIE5GU19TRVJWRVIocmRhdGEtPmlub2RlKS0+
Y2xpZW50LT5jbF9tZXRyaWNzKTsNCj4gK30NCj4gKw0KPiAgc3RhdGljIHZvaWQgZmlsZWxheW91
dF9yZWFkX3JlbGVhc2Uodm9pZCAqZGF0YSkNCj4gIHsNCj4gIAlzdHJ1Y3QgbmZzX3JlYWRfZGF0
YSAqcmRhdGEgPSAoc3RydWN0IG5mc19yZWFkX2RhdGEgKilkYXRhOw0KPiBAQCAtMjY4LDYgKzI3
NywxMyBAQCBzdGF0aWMgdm9pZCBmaWxlbGF5b3V0X3dyaXRlX2NhbGxfZG9uZShzdHJ1Y3QgcnBj
X3Rhc2sgKnRhc2ssIHZvaWQgKmRhdGEpDQo+ICAJd2RhdGEtPm1kc19vcHMtPnJwY19jYWxsX2Rv
bmUodGFzaywgZGF0YSk7DQo+ICB9DQo+ICANCj4gK3N0YXRpYyB2b2lkIGZpbGVsYXlvdXRfd3Jp
dGVfY291bnRfc3RhdHMoc3RydWN0IHJwY190YXNrICp0YXNrLCB2b2lkICpkYXRhKQ0KPiArew0K
PiArCXN0cnVjdCBuZnNfd3JpdGVfZGF0YSAqd2RhdGEgPSAoc3RydWN0IG5mc193cml0ZV9kYXRh
ICopZGF0YTsNCj4gKw0KPiArCXJwY19jb3VudF9pb3N0YXRzKHRhc2ssIE5GU19TRVJWRVIod2Rh
dGEtPmlub2RlKS0+Y2xpZW50LT5jbF9tZXRyaWNzKTsNCj4gK30NCj4gKw0KPiAgc3RhdGljIHZv
aWQgZmlsZWxheW91dF93cml0ZV9yZWxlYXNlKHZvaWQgKmRhdGEpDQo+ICB7DQo+ICAJc3RydWN0
IG5mc193cml0ZV9kYXRhICp3ZGF0YSA9IChzdHJ1Y3QgbmZzX3dyaXRlX2RhdGEgKilkYXRhOw0K
PiBAQCAtMjg4LDE4ICszMDQsMjEgQEAgc3RhdGljIHZvaWQgZmlsZWxheW91dF9jb21taXRfcmVs
ZWFzZSh2b2lkICpkYXRhKQ0KPiAgc3RydWN0IHJwY19jYWxsX29wcyBmaWxlbGF5b3V0X3JlYWRf
Y2FsbF9vcHMgPSB7DQo+ICAJLnJwY19jYWxsX3ByZXBhcmUgPSBmaWxlbGF5b3V0X3JlYWRfcHJl
cGFyZSwNCj4gIAkucnBjX2NhbGxfZG9uZSA9IGZpbGVsYXlvdXRfcmVhZF9jYWxsX2RvbmUsDQo+
ICsJLnJwY19jb3VudF9zdGF0cyA9IGZpbGVsYXlvdXRfcmVhZF9jb3VudF9zdGF0cywNCj4gIAku
cnBjX3JlbGVhc2UgPSBmaWxlbGF5b3V0X3JlYWRfcmVsZWFzZSwNCj4gIH07DQo+ICANCj4gIHN0
cnVjdCBycGNfY2FsbF9vcHMgZmlsZWxheW91dF93cml0ZV9jYWxsX29wcyA9IHsNCj4gIAkucnBj
X2NhbGxfcHJlcGFyZSA9IGZpbGVsYXlvdXRfd3JpdGVfcHJlcGFyZSwNCj4gIAkucnBjX2NhbGxf
ZG9uZSA9IGZpbGVsYXlvdXRfd3JpdGVfY2FsbF9kb25lLA0KPiArCS5ycGNfY291bnRfc3RhdHMg
PSBmaWxlbGF5b3V0X3dyaXRlX2NvdW50X3N0YXRzLA0KPiAgCS5ycGNfcmVsZWFzZSA9IGZpbGVs
YXlvdXRfd3JpdGVfcmVsZWFzZSwNCj4gIH07DQo+ICANCj4gIHN0cnVjdCBycGNfY2FsbF9vcHMg
ZmlsZWxheW91dF9jb21taXRfY2FsbF9vcHMgPSB7DQo+ICAJLnJwY19jYWxsX3ByZXBhcmUgPSBm
aWxlbGF5b3V0X3dyaXRlX3ByZXBhcmUsDQo+ICAJLnJwY19jYWxsX2RvbmUgPSBmaWxlbGF5b3V0
X3dyaXRlX2NhbGxfZG9uZSwNCj4gKwkucnBjX2NvdW50X3N0YXRzID0gZmlsZWxheW91dF93cml0
ZV9jb3VudF9zdGF0cywNCj4gIAkucnBjX3JlbGVhc2UgPSBmaWxlbGF5b3V0X2NvbW1pdF9yZWxl
YXNlLA0KPiAgfTsNCj4gIA0KPiBkaWZmIC0tZ2l0IGEvaW5jbHVkZS9saW51eC9zdW5ycGMvbWV0
cmljcy5oIGIvaW5jbHVkZS9saW51eC9zdW5ycGMvbWV0cmljcy5oDQo+IGluZGV4IGI2ZWRiYzAu
LjE1NjViYmUgMTAwNjQ0DQo+IC0tLSBhL2luY2x1ZGUvbGludXgvc3VucnBjL21ldHJpY3MuaA0K
PiArKysgYi9pbmNsdWRlL2xpbnV4L3N1bnJwYy9tZXRyaWNzLmgNCj4gQEAgLTc0LDE0ICs3NCwx
NiBAQCBzdHJ1Y3QgcnBjX2NsbnQ7DQo+ICAjaWZkZWYgQ09ORklHX1BST0NfRlMNCj4gIA0KPiAg
c3RydWN0IHJwY19pb3N0YXRzICoJcnBjX2FsbG9jX2lvc3RhdHMoc3RydWN0IHJwY19jbG50ICop
Ow0KPiAtdm9pZAkJCXJwY19jb3VudF9pb3N0YXRzKHN0cnVjdCBycGNfdGFzayAqKTsNCj4gK3Zv
aWQJCQlycGNfY291bnRfaW9zdGF0cyhjb25zdCBzdHJ1Y3QgcnBjX3Rhc2sgKiwNCj4gKwkJCQkJ
ICBzdHJ1Y3QgcnBjX2lvc3RhdHMgKik7DQo+ICB2b2lkCQkJcnBjX3ByaW50X2lvc3RhdHMoc3Ry
dWN0IHNlcV9maWxlICosIHN0cnVjdCBycGNfY2xudCAqKTsNCj4gIHZvaWQJCQlycGNfZnJlZV9p
b3N0YXRzKHN0cnVjdCBycGNfaW9zdGF0cyAqKTsNCj4gIA0KPiAgI2Vsc2UgIC8qICBDT05GSUdf
UFJPQ19GUyAgKi8NCj4gIA0KPiAgc3RhdGljIGlubGluZSBzdHJ1Y3QgcnBjX2lvc3RhdHMgKnJw
Y19hbGxvY19pb3N0YXRzKHN0cnVjdCBycGNfY2xudCAqY2xudCkgeyByZXR1cm4gTlVMTDsgfQ0K
PiAtc3RhdGljIGlubGluZSB2b2lkIHJwY19jb3VudF9pb3N0YXRzKHN0cnVjdCBycGNfdGFzayAq
dGFzaykge30NCj4gK3N0YXRpYyBpbmxpbmUgdm9pZCBycGNfY291bnRfaW9zdGF0cyhjb25zdCBz
dHJ1Y3QgcnBjX3Rhc2sgKnRhc2ssDQo+ICsJCQkJICAgICBzdHJ1Y3QgcnBjX2lvc3RhdHMgKnN0
YXRzKSB7fQ0KPiAgc3RhdGljIGlubGluZSB2b2lkIHJwY19wcmludF9pb3N0YXRzKHN0cnVjdCBz
ZXFfZmlsZSAqc2VxLCBzdHJ1Y3QgcnBjX2NsbnQgKmNsbnQpIHt9DQo+ICBzdGF0aWMgaW5saW5l
IHZvaWQgcnBjX2ZyZWVfaW9zdGF0cyhzdHJ1Y3QgcnBjX2lvc3RhdHMgKnN0YXRzKSB7fQ0KPiAg
DQo+IGRpZmYgLS1naXQgYS9pbmNsdWRlL2xpbnV4L3N1bnJwYy9zY2hlZC5oIGIvaW5jbHVkZS9s
aW51eC9zdW5ycGMvc2NoZWQuaA0KPiBpbmRleCAyMmRmYzI0Li5kYzBjM2NjIDEwMDY0NA0KPiAt
LS0gYS9pbmNsdWRlL2xpbnV4L3N1bnJwYy9zY2hlZC5oDQo+ICsrKyBiL2luY2x1ZGUvbGludXgv
c3VucnBjL3NjaGVkLmgNCj4gQEAgLTEwMyw2ICsxMDMsNyBAQCB0eXBlZGVmIHZvaWQJCQkoKnJw
Y19hY3Rpb24pKHN0cnVjdCBycGNfdGFzayAqKTsNCj4gIHN0cnVjdCBycGNfY2FsbF9vcHMgew0K
PiAgCXZvaWQgKCpycGNfY2FsbF9wcmVwYXJlKShzdHJ1Y3QgcnBjX3Rhc2sgKiwgdm9pZCAqKTsN
Cj4gIAl2b2lkICgqcnBjX2NhbGxfZG9uZSkoc3RydWN0IHJwY190YXNrICosIHZvaWQgKik7DQo+
ICsJdm9pZCAoKnJwY19jb3VudF9zdGF0cykoc3RydWN0IHJwY190YXNrICosIHZvaWQgKik7DQo+
ICAJdm9pZCAoKnJwY19yZWxlYXNlKSh2b2lkICopOw0KPiAgfTsNCj4gIA0KPiBkaWZmIC0tZ2l0
IGEvbmV0L3N1bnJwYy9zdGF0cy5jIGIvbmV0L3N1bnJwYy9zdGF0cy5jDQo+IGluZGV4IDNjNGY2
ODguLjFlYjMzMDQgMTAwNjQ0DQo+IC0tLSBhL25ldC9zdW5ycGMvc3RhdHMuYw0KPiArKysgYi9u
ZXQvc3VucnBjL3N0YXRzLmMNCj4gQEAgLTEzMywyMCArMTMzLDE5IEBAIEVYUE9SVF9TWU1CT0xf
R1BMKHJwY19mcmVlX2lvc3RhdHMpOw0KPiAgLyoqDQo+ICAgKiBycGNfY291bnRfaW9zdGF0cyAt
IHRhbGx5IHVwIHBlci10YXNrIHN0YXRzDQo+ICAgKiBAdGFzazogY29tcGxldGVkIHJwY190YXNr
DQo+ICsgKiBAc3RhdHM6IGFycmF5IG9mIHN0YXQgc3RydWN0dXJlcw0KPiAgICoNCj4gICAqIFJl
bGllcyBvbiB0aGUgY2FsbGVyIGZvciBzZXJpYWxpemF0aW9uLg0KPiAgICovDQo+IC12b2lkIHJw
Y19jb3VudF9pb3N0YXRzKHN0cnVjdCBycGNfdGFzayAqdGFzaykNCj4gK3ZvaWQgcnBjX2NvdW50
X2lvc3RhdHMoY29uc3Qgc3RydWN0IHJwY190YXNrICp0YXNrLCBzdHJ1Y3QgcnBjX2lvc3RhdHMg
KnN0YXRzKQ0KPiAgew0KPiAgCXN0cnVjdCBycGNfcnFzdCAqcmVxID0gdGFzay0+dGtfcnFzdHA7
DQo+IC0Jc3RydWN0IHJwY19pb3N0YXRzICpzdGF0czsNCj4gIAlzdHJ1Y3QgcnBjX2lvc3RhdHMg
Km9wX21ldHJpY3M7DQo+ICAJa3RpbWVfdCBkZWx0YTsNCj4gIA0KPiAtCWlmICghdGFzay0+dGtf
Y2xpZW50IHx8ICF0YXNrLT50a19jbGllbnQtPmNsX21ldHJpY3MgfHwgIXJlcSkNCj4gKwlpZiAo
IXN0YXRzIHx8ICFyZXEpDQo+ICAJCXJldHVybjsNCj4gIA0KPiAtCXN0YXRzID0gdGFzay0+dGtf
Y2xpZW50LT5jbF9tZXRyaWNzOw0KPiAgCW9wX21ldHJpY3MgPSAmc3RhdHNbdGFzay0+dGtfbXNn
LnJwY19wcm9jLT5wX3N0YXRpZHhdOw0KPiAgDQo+ICAJb3BfbWV0cmljcy0+b21fb3BzKys7DQo+
IEBAIC0xNjQsNiArMTYzLDcgQEAgdm9pZCBycGNfY291bnRfaW9zdGF0cyhzdHJ1Y3QgcnBjX3Rh
c2sgKnRhc2spDQo+ICAJZGVsdGEgPSBrdGltZV9zdWIoa3RpbWVfZ2V0KCksIHRhc2stPnRrX3N0
YXJ0KTsNCj4gIAlvcF9tZXRyaWNzLT5vbV9leGVjdXRlID0ga3RpbWVfYWRkKG9wX21ldHJpY3Mt
Pm9tX2V4ZWN1dGUsIGRlbHRhKTsNCj4gIH0NCj4gK0VYUE9SVF9TWU1CT0xfR1BMKHJwY19jb3Vu
dF9pb3N0YXRzKTsNCj4gIA0KPiAgc3RhdGljIHZvaWQgX3ByaW50X25hbWUoc3RydWN0IHNlcV9m
aWxlICpzZXEsIHVuc2lnbmVkIGludCBvcCwNCj4gIAkJCXN0cnVjdCBycGNfcHJvY2luZm8gKnBy
b2NzKQ0KPiBkaWZmIC0tZ2l0IGEvbmV0L3N1bnJwYy94cHJ0LmMgYi9uZXQvc3VucnBjL3hwcnQu
Yw0KPiBpbmRleCBlZmU1NDk1Li4xMDBiNDY5IDEwMDY0NA0KPiAtLS0gYS9uZXQvc3VucnBjL3hw
cnQuYw0KPiArKysgYi9uZXQvc3VucnBjL3hwcnQuYw0KPiBAQCAtMTEzMiw3ICsxMTMyLDEwIEBA
IHZvaWQgeHBydF9yZWxlYXNlKHN0cnVjdCBycGNfdGFzayAqdGFzaykNCj4gIAkJcmV0dXJuOw0K
PiAgDQo+ICAJeHBydCA9IHJlcS0+cnFfeHBydDsNCj4gLQlycGNfY291bnRfaW9zdGF0cyh0YXNr
KTsNCj4gKwlpZiAodGFzay0+dGtfb3BzLT5ycGNfY291bnRfc3RhdHMgIT0gTlVMTCkNCj4gKwkJ
dGFzay0+dGtfb3BzLT5ycGNfY291bnRfc3RhdHModGFzaywgdGFzay0+dGtfY2FsbGRhdGEpOw0K
PiArCWVsc2UgaWYgKHRhc2stPnRrX2NsaWVudCkNCj4gKwkJcnBjX2NvdW50X2lvc3RhdHModGFz
aywgdGFzay0+dGtfY2xpZW50LT5jbF9tZXRyaWNzKTsNCj4gIAlzcGluX2xvY2tfYmgoJnhwcnQt
PnRyYW5zcG9ydF9sb2NrKTsNCj4gIAl4cHJ0LT5vcHMtPnJlbGVhc2VfeHBydCh4cHJ0LCB0YXNr
KTsNCj4gIAlpZiAoeHBydC0+b3BzLT5yZWxlYXNlX3JlcXVlc3QpDQoNCi0tIA0KVHJvbmQgTXlr
bGVidXN0DQpMaW51eCBORlMgY2xpZW50IG1haW50YWluZXINCg0KTmV0QXBwDQpUcm9uZC5NeWts
ZWJ1c3RAbmV0YXBwLmNvbQ0Kd3d3Lm5ldGFwcC5jb20NCg0K

2012-02-09 20:37:48

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH] NFS: include filelayout DS rpc stats in mountstats

T24gVGh1LCAyMDEyLTAyLTA5IGF0IDIwOjIzICswMDAwLCBBZGFtc29uLCBEcm9zIHdyb3RlOg0K
PiBPbiBGZWIgOSwgMjAxMiwgYXQgMjo1OCBQTSwgTXlrbGVidXN0LCBUcm9uZCB3cm90ZToNCj4g
DQo+ID4gT24gVGh1LCAyMDEyLTAyLTA5IGF0IDE5OjQ4ICswMDAwLCBBZGFtc29uLCBEcm9zIHdy
b3RlOg0KPiA+Pj4gDQo+ID4+Pj4gSSBkbyBoYXZlIGFuIGltcGxlbWVudGF0aW9uIHRoYXQgZG9l
c24ndCBuZWVkIHRvIHdhbGsgdGhlIGRldmljZWlkIGxpc3QgYnkgYWxsb3dpbmcgYSBzaGFyZWQg
cnBjX2lvc3RhdHMgc3RydWN0IGJldHdlZW4gbXVsdGlwbGUgcnBjX2NsaWVudHMgKGluIGFkZGl0
aW9uIHRvIHRoZSBjdXJyZW50IHJwY19pb3N0YXRzIHN0cnVjdHVyZSksIGJ1dCB0aGF0IHJlcXVp
cmVkIGFkZGluZyBsb2NraW5nIGFuZCByZWZlcmVuY2UgY291bnRpbmcgLS0gYWxsIGZvciBwcmlu
dGluZyBzdGF0cyAob2J2aW91c2x5IG5vdCB3aGF0IHdlIHdhbnQpLg0KPiA+Pj4gDQo+ID4+PiBU
aGlzIG1pZ2h0IGJlIG1vcmUgaW4gbGluZSB3aXRoIHdoYXQgd2Ugd2FudC4gTm90ZSB0aGF0IGl0
IHNob3VsZCBiZQ0KPiA+Pj4gZWFzeSB0byBjbG9uZSBhbiBycGNfY2xpZW50IGFuZCB0aGVuIHJl
cGxhY2UgaXRzIHJwY19pb3N0YXRzIHN0cnVjdC4gSQ0KPiA+Pj4gZG9uJ3QgdGhpbmsgdGhhdCBu
ZWVkcyBhbnkgZXh0cmEgbG9ja2luZy4gV2UncmUgYWxyZWFkeSBpZ25vcmluZyBsb2NraW5nDQo+
ID4+PiBoZXJlIGJldHdlZW4gZGlmZmVyZW50IHJwY190YXNrcywgc28gdGhyb3dpbmcgaW4gZGlm
ZmVyZW50IHJwY19jbGllbnRzDQo+ID4+PiB0byB0aGUgbWl4IHdpbGwgbWFrZSBubyBkaWZmZXJl
bmNlLg0KPiA+PiANCj4gPj4gWWVhaCwgdGhhdCdzIGVhc3kgZW5vdWdoIGFuZCBpIGd1ZXNzIHdl
IGNvdWxkIGlnbm9yZSBsb2NraW5nLCBidXQgd2UgYXJlIHN0aWxsIGxlZnQgd2l0aCB0aGUgc2Ft
ZSBwcm9ibGVtOiBob3cgaXMgdGhpcyBzdXBwb3NlZCB0byBhY2NvdW50IGZvciBkaWZmZXJlbnQg
bW91bnQgcG9pbnRzIHVzaW5nIHRoZSBzYW1lIG5mc19jbGllbnQ/ICBuZnNfY2xpZW50IG9ubHkg
aGFzIG9uZSBycGNfY2xpZW50IG1lbWJlci4gIEkgZG91YnQgd2Ugd2FudCB0byBtYWtlIGEgaGFz
aCBsb29rdXAgb24gbmZzX3NlcnZlciB0byBnZXQgdGhlIHJpZ2h0IHJwY19jbGllbnQgKHdoaWNo
IGNvdWxkIGFsbCB1c2UgdGhlIHNhbWUgdW5kZXJseWluZyB4cHJ0KS4NCj4gPj4gDQo+ID4+IE1h
eWJlIGl0J3MgdGltZSB0byBtb3ZlIHRoZXNlIHN0YXRzIGludG8gZnMvbmZzLyB3aGVyZSB0aGV5
IHJlYWxseSBiZWxvbmc/ICBXZSBjb3VsZCBnZXQgcmlkIG9mIHRoZSBoYWNrIHRoYXQgb3Zlcmxv
YWRzIHByb2NudW0gd2l0aCBvcG51bSBmcm9tIGluc2lkZSB0aGUgY29tcG91bmQgZm9yIHY0KyBh
bmQgZmluYWxseSBvbmx5IHNob3cgYSBzcGVjaWZpYyBtb3VudCdzIFJQQyBzdGF0cy4NCj4gPiAN
Cj4gPiBBY3R1YWxseSwgaG93IGFib3V0IGp1c3QgYWRkaW5nIGEgY2FsbGJhY2sgaW50byBzdHJ1
Y3QgcnBjX2NhbGxfb3BzDQo+ID4gdGhhdCwgaWYgaXQgaXMgZGVmaW5lZCwgd291bGQgYmUgY2Fs
bGVkIGluc3RlYWQgb2YgcnBjX2NvdW50X2lvc3RhdHMoKS4gDQo+ID4gSWYgeW91IHRoZW4gbW9k
aWZ5IHJwY19jb3VudF9pb3N0YXRzKCkgdG8gdGFrZSB0aGUgJ3N0YXRzJyB2YXJpYWJsZSBhcyBh
DQo+ID4gcGFyYW1ldGVyLCB5b3UgY2FuIHNpbXBseSBoYXZlIHRoZSBuZXcgY2FsbGJhY2sgY2Fs
bCBycGNfY291bnRfaW9zdGF0cw0KPiA+IHdpdGggdGhlIHJpZ2h0IGFyZ3VtZW50cy4NCj4gDQo+
IFllYWgsIHRoYXQgY291bGQgd29yayEgIE9uIG15IHdhbGsgaG9tZSBmcm9tIHRoZSBjYWZlICh0
aGV5IGFsd2F5cyBzZWVtIHRvIGhlbHApIEkgY2FtZSB1cCB3aXRoIGEgc2xpZ2h0bHkgZGlmZmVy
ZW50IHN0cmF0ZWd5Og0KPiANCj4gLSByZW1vdmUgdGhlIGNsX21ldHJpY3MgKHN0cnVjdCBycGNf
aW9zdGF0cykgbWVtYmVyIGZyb20gcnBjX2NsbnQNCj4gLSBhZGQgYW4gKm9wdGlvbmFsKiBycGNf
aW9zdGF0cyBwb2ludGVyIHRvIHJwY190YXNrIChpZ25vcmVkIGlmIE5VTEwpDQo+IC0gYWxsb2Nh
dGUgb25lIHJwY19pb3N0YXRzIHN0cnVjdHVyZSAocmVhbGx5IGFycmF5IG9mIHN0cnVjdHMsIGJ1
dCB5b3Uga25vdyB3aGF0IEkgbWVhbikgcGVyIG5mc19zZXJ2ZXIgc3RydWN0dXJlDQo+IC0gd2hl
biBzZXR0aW5nIHVwIHJwY190YXNrcyBpbiBuZnMtbGFuZCwgcGFzcyB0aGUgY29ycmVjdCBycGNf
aW9zdGF0cw0KPiANCj4gd2l0aCB0aGlzIHN0cmF0ZWd5IHdlIGRvbid0IGFjY3VtdWxhdGUgc3Rh
dHMgd2hlbiB0aGV5IGFyZW4ndCBldmVyIHVzZWQgKGxpa2UgYW4gcnBjX2NsaWVudCB1c2VkIGZv
ciBtb3VudCBwcm90b2NvbCkuICBhZ2Fpbiwgb25seSBORlMgdXNlcyB0aGUgcnBjIHN0YXRzIGlu
dGVyZmFjZS4NCj4gDQo+IEkga2luZCBvZiBsaWtlIHRoaXMgYmV0dGVyIHRoYW4gYSBjYWxsYmFj
aywgYnV0IGVpdGhlciB3YXkgaXMgZmluZSB3aXRoIG1lLiAgV2hpY2ggd2F5IHdvdWxkIHlvdSBw
cmVmZXI/DQoNCkknZCBwcmVmZXIgbm90IHRvIGhhdmUgdG8gZ3JvdyB0aGUgc2l6ZSBvZiB0aGUg
cnBjX3Rhc2sgdW5sZXNzIHdlIHJlYWxseQ0KbmVlZCB0by4gVGhhdCdzIHdoeSBJIHN1Z2dlc3Rl
ZCB0aGUgY2FsbGJhY2sgaW5zdGVhZC4NCg0KQ2hlZXJzDQogIFRyb25kDQoNCi0tIA0KVHJvbmQg
TXlrbGVidXN0DQpMaW51eCBORlMgY2xpZW50IG1haW50YWluZXINCg0KTmV0QXBwDQpUcm9uZC5N
eWtsZWJ1c3RAbmV0YXBwLmNvbQ0Kd3d3Lm5ldGFwcC5jb20NCg0K

2012-02-17 19:16:49

by Adamson, Dros

[permalink] [raw]
Subject: Re: [PATCH] NFS: include filelayout DS rpc stats in mountstats


On Feb 17, 2012, at 2:04 PM, Myklebust, Trond wrote:

>> -----Original Message-----
>> From: Adamson, Dros
>> Sent: Friday, February 17, 2012 2:02 PM
>> To: Myklebust, Trond
>> Cc: Adamson, Dros; [email protected]
>> Subject: Re: [PATCH] NFS: include filelayout DS rpc stats in mountstats
>>
>> On Feb 17, 2012, at 1:47 PM, Myklebust, Trond wrote:
>>
>>> On Fri, 2012-02-17 at 13:15 -0500, Weston Andros Adamson wrote:
>>>> Include RPC statistics from all data servers in /proc/self/mountstats for
>> pNFS
>>>> filelayout mounts.
>>>>
>>>> Signed-off-by: Weston Andros Adamson <[email protected]>
>>>> ---
>>>> Update: don't increment tk_client stats if callback is used.
>>>>
>>>> This still checks task->tk_client != NULL -- I'm not convinced it's unneeded
>>>> as it existed in rpc_count_iostats.
>>>
>>> I'll remove it before applying.
>>
>> I can repost the patch, if that's easier.
>
> Nah. I've already applied and pushed to the git repository.
>

Cool, thanks!

-dros


Attachments:
smime.p7s (1.34 kB)

2012-02-16 21:01:26

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH] NFS: include filelayout DS rpc stats in mountstats

T24gVGh1LCAyMDEyLTAyLTE2IGF0IDIwOjQ0ICswMDAwLCBBZGFtc29uLCBEcm9zIHdyb3RlOg0K
PiBPbiBGZWIgMTYsIDIwMTIsIGF0IDI6NDggUE0sIE15a2xlYnVzdCwgVHJvbmQgd3JvdGU6DQo+
ID4gICAgIDIuIFNob3VsZG4ndCB3ZSBiZSBjYWxsaW5nIF9laXRoZXJfIHJwY19jb3VudF9pb3N0
YXRzKCksIF9vcl8NCj4gPiAgICAgICAgdGFzay0+dGtfb3BzLT5ycGNfY291bnRfc3RhdHMoKT8g
QXMgZmFyIGFzIEkgY2FuIHNlZSwgdGhlDQo+ID4gICAgICAgIG5mc3N0YXQgb3V0cHV0IHdpbGwg
bm93IGJlIGRvdWJsZS1jb3VudGluZyBhbmQgcE5GUyByZWFkcywNCj4gPiAgICAgICAgd3JpdGVz
IGFuZCBjb21taXRzIHRoYXQgYXJlIHNlbnQgdG8gdGhlIERTLg0KPiANCj4gDQo+IE5vLCB0aGlz
IGRvZXNuJ3QgZG91YmxlIGNvdW50IHdpdGggbmZzc3RhdHMuICBuZnNzdGF0cyB1c2VzIHJwY19j
bG50OjpjbF9zdGF0cyAoYW5kIG5vdCBycGNfY2xudDo6Y2xfbWV0cmljcykuDQo+IA0KPiBJIHBy
b2JhYmx5IHNob3VsZCBoYXZlIGRvbmUgYW4gaWYvZWxzZSBmb3IgdGhpcyBwYXRjaCAtLSB3aXRo
IHRoZSBjdXJyZW50IGNvZGUsIHRoZSBEU3MnIHJwY19jbG50OmNsX21ldHJpY3Mgd2lsbCBuZXZl
ciBiZSB1c2VkLg0KPiBJIGxlZnQgaXQgYWNjdW11bGF0aW5nIGhlcmUgYmVjYXVzZSB3ZSB3YW50
IHRvIGhhdmUgcGVyLURTIHN0YXRzIGV2ZW50dWFsbHkgYW5kIG15IHBsYW4gd2FzIHRvIHByaW50
IG91dCBzdGF0cyBpbiAvcHJvYy9mcy9uZnNmcyBwZXIgKmNsaWVudCogKHNvIG5vdCBzZXBhcmF0
ZWQgYnkgbW91bnRwb2ludCkuDQo+IA0KPiBJIGNhbiByZXBvc3Qgd2l0aCBpZi9lbHNlLCBidXQg
bG9va2luZyBhdCB0aGlzIHNvbWUgbW9yZSBtYWRlIG1lIHJlYWxpemUgdGhhdCB3ZSBhcmUgKnN0
aWxsKiBkb2luZyB0aGlzIHdyb25nIDopDQo+IA0KPiBUaGUgY2FsbGJhY2sgbWV0aG9kIGluIHRo
aXMgcGF0Y2ggZmFpbHMgdG8gYWNjdW11bGF0ZSBzdGF0cyBmb3Igb3BlcmF0aW9ucyB0byB0aGUg
RFMgb3RoZXIgdGhhbiByZWFkL3dyaXRlL2NvbW1pdCAtLSB0aGF0IHNlZW1zIHJpZ2h0LCBidXQg
d2hhdCBhYm91dCBudWxsLCBleGNoYW5nZV9pZCwgc2Vzc2lvbiBoZWFydGJlYXRzLCBldGM/DQo+
IA0KPiBJbiBvcmRlciB0byBwcm9wZXJseSBhY2N1bXVsYXRlIHRob3NlIHdlIGFyZSBiYWNrIHRv
IHR3byBvYnZpb3VzIGNob2ljZXM6DQo+ICAgMSkgYWRkIGEgY291bnRfaW9zdGF0cyBjYWxsYmFj
ayB0byB0aGUgfjI1IG90aGVyIHJwY19jYWxsX29wcyBpbiBuZnMtbGFuZCAoeXVjaykNCj4gICAy
KSBhZGQgYW4gJ2FkZGl0aW9uYWwgc3RhdHMnIHBvaW50ZXIgdG8gdGhlIHJwY190YXNrIHN0cnVj
dHVyZSAodHJvbmQgYWxyZWFkeSBzYWlkIHlvdSBkb24ndCB3YW50IHRvIGFkZCB0byB0YXNrIHN0
cnVjdCkNCj4gDQo+IE9yIGRvIHdlIGp1c3Qgbm90IGNhcmUgYWJvdXQgZGlzcGxheWluZyB0aG9z
ZSBvcGVyYXRpb25zPyAgRm9yIG15IHB1cnBvc2VzIChuZnNvbWV0ZXIgcGVyZiB0ZXN0aW5nKSwg
aXQnZCBiZSBuaWNlIHRvIGhhdmUgKmFsbCogb2YgdGhlIG9wZXJhdGlvbnMuDQoNCkFzIGZhciBh
cyBJJ20gY29uY2VybmVkLCB0aGUgYWRtaW5pc3RyYXRpdmUgdHJhZmZpYyB0byB0aGUgRFMgc2hv
dWxkIG5vdA0KYmUgYWNjb3VudGVkIGZvciBpbiB0aGUgbW91bnRzdGF0czogdGhhdCB3b3VsZCBi
ZSB3cm9uZyBzaW5jZSBEU2VzIGNhbg0KYmUgc2hhcmVkIG5vdCBvbmx5IGJ5IGRpZmZlcmVudCBm
aWxlc3lzdGVtcyBidXQgZXZlbiBieSBkaWZmZXJlbnQgTURTZXMuDQoNClNvIHRoZSBwZXJmb3Jt
YW5jZSBvdmVyaGVhZCBvZiBsZWFzZSBhbmQgc2Vzc2lvbiBzZXR1cCB0byB0aGUgRFNlcyBuZWVk
cw0KdG8gYmUgYWNjb3VudGVkIGZvciBieSBzb21lIG90aGVyIG1lY2hhbmlzbS4NCg0KLS0gDQpU
cm9uZCBNeWtsZWJ1c3QNCkxpbnV4IE5GUyBjbGllbnQgbWFpbnRhaW5lcg0KDQpOZXRBcHANClRy
b25kLk15a2xlYnVzdEBuZXRhcHAuY29tDQp3d3cubmV0YXBwLmNvbQ0KDQo=

2012-02-09 20:23:36

by Adamson, Dros

[permalink] [raw]
Subject: Re: [PATCH] NFS: include filelayout DS rpc stats in mountstats


On Feb 9, 2012, at 2:58 PM, Myklebust, Trond wrote:

> On Thu, 2012-02-09 at 19:48 +0000, Adamson, Dros wrote:
>>>
>>>> I do have an implementation that doesn't need to walk the deviceid list by allowing a shared rpc_iostats struct between multiple rpc_clients (in addition to the current rpc_iostats structure), but that required adding locking and reference counting -- all for printing stats (obviously not what we want).
>>>
>>> This might be more in line with what we want. Note that it should be
>>> easy to clone an rpc_client and then replace its rpc_iostats struct. I
>>> don't think that needs any extra locking. We're already ignoring locking
>>> here between different rpc_tasks, so throwing in different rpc_clients
>>> to the mix will make no difference.
>>
>> Yeah, that's easy enough and i guess we could ignore locking, but we are still left with the same problem: how is this supposed to account for different mount points using the same nfs_client? nfs_client only has one rpc_client member. I doubt we want to make a hash lookup on nfs_server to get the right rpc_client (which could all use the same underlying xprt).
>>
>> Maybe it's time to move these stats into fs/nfs/ where they really belong? We could get rid of the hack that overloads procnum with opnum from inside the compound for v4+ and finally only show a specific mount's RPC stats.
>
> Actually, how about just adding a callback into struct rpc_call_ops
> that, if it is defined, would be called instead of rpc_count_iostats().
> If you then modify rpc_count_iostats() to take the 'stats' variable as a
> parameter, you can simply have the new callback call rpc_count_iostats
> with the right arguments.

Yeah, that could work! On my walk home from the cafe (they always seem to help) I came up with a slightly different strategy:

- remove the cl_metrics (struct rpc_iostats) member from rpc_clnt
- add an *optional* rpc_iostats pointer to rpc_task (ignored if NULL)
- allocate one rpc_iostats structure (really array of structs, but you know what I mean) per nfs_server structure
- when setting up rpc_tasks in nfs-land, pass the correct rpc_iostats

with this strategy we don't accumulate stats when they aren't ever used (like an rpc_client used for mount protocol). again, only NFS uses the rpc stats interface.

I kind of like this better than a callback, but either way is fine with me. Which way would you prefer?

-dros

>
> --
> Trond Myklebust
> Linux NFS client maintainer
>
> NetApp
> [email protected]
> http://www.netapp.com
>


Attachments:
smime.p7s (1.34 kB)

2012-02-17 19:02:32

by Adamson, Dros

[permalink] [raw]
Subject: Re: [PATCH] NFS: include filelayout DS rpc stats in mountstats

On Feb 17, 2012, at 1:47 PM, Myklebust, Trond wrote:

> On Fri, 2012-02-17 at 13:15 -0500, Weston Andros Adamson wrote:
>> Include RPC statistics from all data servers in /proc/self/mountstats for pNFS
>> filelayout mounts.
>>
>> Signed-off-by: Weston Andros Adamson <[email protected]>
>> ---
>> Update: don't increment tk_client stats if callback is used.
>>
>> This still checks task->tk_client != NULL -- I'm not convinced it's unneeded
>> as it existed in rpc_count_iostats.
>
> I'll remove it before applying.

I can repost the patch, if that's easier.

-dros


Attachments:
smime.p7s (1.34 kB)