I had some trouble using mountstats to analyze READ_PLUS operations, so I
wrote these patches to fix my issues.
I found that /proc/self/mountstats was reporting "GETDEVICELIST" as "51",
most likely because it was removed from the client but the nfs4_procedures
array still has a placeholder for it. Mountstats crashes when it tries to
analyze "51", so the second patch fixes this issue by looking for numeric
operation names. I'm willing to put in stubs for GETDEVICELIST back into
the client if that would be a better fix.
Thanks,
Anna
Anna Schumaker (2):
mountstats: Resync NFSv4 Operations List
mountstats: Only count operations that have names
tools/mountstats/mountstats.py | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
--
2.3.5
GETDEVICELIST was removed from the kernel, but due to the way the client
is coded it still shows up in /proc/self/mountstats as "51". This
causes mountstats to crash when asked to output "raw" statistics, so
this patch teaches mountsats to ignore any operations that only have
numeric names.
Signed-off-by: Anna Schumaker <[email protected]>
---
tools/mountstats/mountstats.py | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/tools/mountstats/mountstats.py b/tools/mountstats/mountstats.py
index 8f2e26e..5f243ac 100644
--- a/tools/mountstats/mountstats.py
+++ b/tools/mountstats/mountstats.py
@@ -284,8 +284,9 @@ class DeviceData:
self.__rpc_data['per-op'] = words
else:
op = words[0][:-1]
- self.__rpc_data['ops'] += [op]
- self.__rpc_data[op] = [int(word) for word in words[1:]]
+ if not op.isnumeric():
+ self.__rpc_data['ops'] += [op]
+ self.__rpc_data[op] = [int(word) for word in words[1:]]
def parse_stats(self, lines):
"""Turn a list of lines from a mount stat file into a
--
2.3.5
We've added several operations and removed GETDEVICELIST, Let's keep
the Nfsv4ops list up to date with the kernel.
Signed-off-by: Anna Schumaker <[email protected]>
---
tools/mountstats/mountstats.py | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/tools/mountstats/mountstats.py b/tools/mountstats/mountstats.py
index 38943eb..8f2e26e 100644
--- a/tools/mountstats/mountstats.py
+++ b/tools/mountstats/mountstats.py
@@ -195,16 +195,18 @@ Nfsv4ops = [
'SEQUENCE',
'GET_LEASE_TIME',
'RECLAIM_COMPLETE',
- 'LAYOUTGET',
'GETDEVICEINFO',
+ 'LAYOUTGET',
'LAYOUTCOMMIT',
'LAYOUTRETURN',
'SECINFO_NO_NAME',
'TEST_STATEID',
'FREE_STATEID',
- 'GETDEVICELIST',
'BIND_CONN_TO_SESSION',
- 'DESTROY_CLIENTID'
+ 'DESTROY_CLIENTID',
+ 'SEEK',
+ 'ALLOCATE',
+ 'DEALLOCATE',
]
class DeviceData:
--
2.3.5
On Wed, Apr 8, 2015 at 12:47 PM, Anna Schumaker
<[email protected]> wrote:
> I had some trouble using mountstats to analyze READ_PLUS operations, so I
> wrote these patches to fix my issues.
>
> I found that /proc/self/mountstats was reporting "GETDEVICELIST" as "51",
Shouldn't we try to fix that as being a regression in the client? I
see no reason why we shouldn't be able to add in a p_name for
GETDEVICELIST.
> most likely because it was removed from the client but the nfs4_procedures
> array still has a placeholder for it. Mountstats crashes when it tries to
> analyze "51", so the second patch fixes this issue by looking for numeric
> operation names. I'm willing to put in stubs for GETDEVICELIST back into
> the client if that would be a better fix.
>
> Thanks,
> Anna
>
>
> Anna Schumaker (2):
> mountstats: Resync NFSv4 Operations List
> mountstats: Only count operations that have names
>
> tools/mountstats/mountstats.py | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> --
> 2.3.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
On 04/08/2015 01:01 PM, Trond Myklebust wrote:
> On Wed, Apr 8, 2015 at 12:47 PM, Anna Schumaker
> <[email protected]> wrote:
>> I had some trouble using mountstats to analyze READ_PLUS operations, so I
>> wrote these patches to fix my issues.
>>
>> I found that /proc/self/mountstats was reporting "GETDEVICELIST" as "51",
>
> Shouldn't we try to fix that as being a regression in the client? I
> see no reason why we shouldn't be able to add in a p_name for
> GETDEVICELIST.
I'm cool with that. I wasn't sure if it fell under "don't break userspace" if I'm the only person to have a problem in however long it's been since GETDEVICELIST was removed.
Anna
>
>> most likely because it was removed from the client but the nfs4_procedures
>> array still has a placeholder for it. Mountstats crashes when it tries to
>> analyze "51", so the second patch fixes this issue by looking for numeric
>> operation names. I'm willing to put in stubs for GETDEVICELIST back into
>> the client if that would be a better fix.
>>
>> Thanks,
>> Anna
>>
>>
>> Anna Schumaker (2):
>> mountstats: Resync NFSv4 Operations List
>> mountstats: Only count operations that have names
>>
>> tools/mountstats/mountstats.py | 13 ++++++++-----
>> 1 file changed, 8 insertions(+), 5 deletions(-)
>>
>> --
>> 2.3.5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Apr 08, 2015 at 12:47:26PM -0400, Anna Schumaker wrote:
> We've added several operations and removed GETDEVICELIST, Let's keep
> the Nfsv4ops list up to date with the kernel.
Please keep GETDEVICELIST, as newer mountstats should handle older
kernels as well.
On 04/08/2015 01:57 PM, Christoph Hellwig wrote:
> On Wed, Apr 08, 2015 at 12:47:26PM -0400, Anna Schumaker wrote:
>> We've added several operations and removed GETDEVICELIST, Let's keep
>> the Nfsv4ops list up to date with the kernel.
>
> Please keep GETDEVICELIST, as newer mountstats should handle older
> kernels as well.
>
Fair enough. I'm also working on a patch to add GETDEVICELIST back into the client.
Anna