Changes in 3.1 kernel break NFS monitoring tools

PATCH In review Steve Dickson 4 participants 10 messages Sep 21, 2011 1 message not in the archive

Subsystems (3)

VFS NFS client NFS server

lore

Share & tools

lore

Steve Dickson

author

2011-09-21 18:16 UTC #0 lore raw

cover Changes in 3.1 kernel break NFS monitoring tools

Both parser of nfsiostat and mountstats depend on the the first word in /proc/self/mountstats to be 'device' for them to parse through all the devices.

NFS mounts entries, in 3.1, no longer have start with the word 'device'. They start with the actual device, which confused both set of parsing routines.

Steve Dickson (2):
nfsiostat: Breaks on 3.1 kernels
mountstats: Breaks on 3.1 kernels

 tools/mountstats/mountstats.py |    9 +++++++++
 tools/nfs-iostat/nfs-iostat.py |   11 +++++++++--
 2 files changed, 18 insertions(+), 2 deletions(-)

Signature · 1 line

1.7.6.2

Steve Dickson

author

2011-09-21 18:16 UTC #1 lore raw

PATCH 1/2 nfsiostat: Breaks on 3.1 kernels

nfsiostat depend on all devices entries in /proc/self/mountstats to start with the word 'device'. With 3.1 kernels, NFS entries start with the actual device (i.e. server:/export) not the word 'device'. This change confused nfsiostat parsing.

Signed-off-by Steve Dickson <[email protected]>

---
 tools/nfs-iostat/nfs-iostat.py |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

tools/nfs-iostat/nfs-iostat.py +9 -2

index 4e55dad..d909632 100644
@@ -86,6 +86,12 @@ class DeviceData:
             self.__nfs_data['fstype'] = words[7]
             if words[7] == 'nfs':
                 self.__nfs_data['statvers'] = words[8]
+        elif 'nfs' in words or 'nfs4' in words:
+            self.__nfs_data['export'] = words[0]
+            self.__nfs_data['mountpoint'] = words[3]
+            self.__nfs_data['fstype'] = words[6]
+            if words[6] == 'nfs':
+                self.__nfs_data['statvers'] = words[7]
         elif words[0] == 'age:':
             self.__nfs_data['age'] = long(words[1])
         elif words[0] == 'opts:':

@@ -426,6 +432,9 @@ def parse_stats_file(filename):
         if words[0] == 'device':
             key = words[4]
             new = [ line.strip() ]
+        elif 'nfs' in words or 'nfs4' in words:
+            key = words[3]
+            new = [ line.strip() ]
         else:
             new += [ line.strip() ]
         ms_dict[key] = new

@@ -436,7 +445,6 @@ def parse_stats_file(filename):
 def print_iostat_summary(old, new, devices, time, options):
     stats = {}
     diff_stats = {}
-
     if old:
         # Trim device list to only include intersection of old and new data,
         # this addresses umounts due to autofs mountpoints

@@ -553,7 +561,6 @@ client are listed.
     parser.add_option_group(displaygroup)
 
     (options, args) = parser.parse_args(sys.argv)
-
     for arg in args:
 
         if arg == sys.argv[0]:

Signature · 1 line

1.7.6.2

Steve Dickson

author

2011-09-21 18:16 UTC #2 lore raw

PATCH 2/2 mountstats: Breaks on 3.1 kernels

In reply to Steve Dickson

mountstats depend on all devices entries in /proc/self/mountstats to start with the word 'device'. With 3.1 kernels, NFS entries start with the actual device (i.e. server:/export) not the word 'device'. This change confused mountstats parsing.

Signed-off-by Steve Dickson <[email protected]>

---
 tools/mountstats/mountstats.py |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

tools/mountstats/mountstats.py +9 -0

index 280e15c..b95b71d 100644
@@ -46,6 +46,12 @@ class DeviceData:
             self.__nfs_data['fstype'] = words[7]
             if words[7].find('nfs') != -1:
                 self.__nfs_data['statvers'] = words[8]
+        elif 'nfs' in words or 'nfs4' in words:
+            self.__nfs_data['export'] = words[0]
+            self.__nfs_data['mountpoint'] = words[3]
+            self.__nfs_data['fstype'] = words[6]
+            if words[6].find('nfs') != -1:
+                self.__nfs_data['statvers'] = words[7]
         elif words[0] == 'age:':
             self.__nfs_data['age'] = long(words[1])
         elif words[0] == 'opts:':

@@ -371,6 +377,9 @@ def parse_stats_file(filename):
         if words[0] == 'device':
             key = words[4]
             new = [ line.strip() ]
+        elif 'nfs' in words or 'nfs4' in words:
+            key = words[3]
+            new = [ line.strip() ]
         else:
             new += [ line.strip() ]
         ms_dict[key] = new

Signature · 1 line

1.7.6.2

Bryan Schumaker

2011-09-21 18:25 UTC #3 lore raw

In reply to Steve Dickson

Quoted 7 lines · Steve Dickson

Both parser of nfsiostat and mountstats depend on the the first word in /proc/self/mountstats to be 'device' for them to parse through all the devices.
NFS mounts entries, in 3.1, no longer have start with the word 'device'. They start with the actual device, which confused both set of parsing routines.

I sent in a patch to fix this on the kernel end almost a month ago, but it must have gotten lost with the kernel.org confusion. I included what I came up with below.

- Bryan

[PATCH] VFS: Add "device" tag to /proc/self/mountstats

nfsiostat was failing to find mounted filesystems on kernels after 2.6.38 because of changes to show_vfsstat() by commit c7f404b40a3665d9f4e9a927cc5c1ee0479ed8f9. This patch adds back the "device" tag before the nfs server entry so scripts can parse the mountstats file correctly.

Signed-off-by Bryan Schumaker <[email protected]>

---
 fs/namespace.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

fs/namespace.c +1 -0

index 22bfe82..1429114 100644
@@ -1109,6 +1109,7 @@ static int show_vfsstat(struct seq_file *m, void *v)
 
 	/* device */
 	if (mnt->mnt_sb->s_op->show_devname) {
+		seq_puts(m, "device ");
 		err = mnt->mnt_sb->s_op->show_devname(m, mnt);
 	} else {
 		if (mnt->mnt_devname) {

Quoted 9 lines

Steve Dickson (2):
nfsiostat: Breaks on 3.1 kernels
mountstats: Breaks on 3.1 kernels
 tools/mountstats/mountstats.py |    9 +++++++++
 tools/nfs-iostat/nfs-iostat.py |   11 +++++++++--
 2 files changed, 18 insertions(+), 2 deletions(-)

Steve Dickson

author

2011-09-21 18:31 UTC #4 lore raw

Quoted 10 lines · Bryan Schumaker

Quoted 7 lines · Steve Dickson
Both parser of nfsiostat and mountstats depend on the the first word in /proc/self/mountstats to be 'device' for them to parse through all the devices.
NFS mounts entries, in 3.1, no longer have start with the word 'device'. They start with the actual device, which confused both set of parsing routines.
I sent in a patch to fix this on the kernel end almost a month ago, but it must have gotten lost with the kernel.org confusion. I included what I came up with below

Interesting... Maybe we should still apply these patches until your patch shows up? They are definitely broken...

steved.
.

Quoted 40 lines

- Bryan
[PATCH] VFS: Add "device" tag to /proc/self/mountstats
nfsiostat was failing to find mounted filesystems on kernels after 2.6.38 because of changes to show_vfsstat() by commit c7f404b40a3665d9f4e9a927cc5c1ee0479ed8f9. This patch adds back the "device" tag before the nfs server entry so scripts can parse the mountstats file correctly.
Signed-off-by Bryan Schumaker <[email protected]>
---
 fs/namespace.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
fs/namespace.c +1 -0
index 22bfe82..1429114 100644
@@ -1109,6 +1109,7 @@ static int show_vfsstat(struct seq_file *m, void *v)
 
 	/* device */
 	if (mnt->mnt_sb->s_op->show_devname) {
+		seq_puts(m, "device ");
 		err = mnt->mnt_sb->s_op->show_devname(m, mnt);
 	} else {
 		if (mnt->mnt_devname) {
Quoted 11 lines
Steve Dickson (2):
nfsiostat: Breaks on 3.1 kernels
mountstats: Breaks on 3.1 kernels
 tools/mountstats/mountstats.py |    9 +++++++++
 tools/nfs-iostat/nfs-iostat.py |   11 +++++++++--
 2 files changed, 18 insertions(+), 2 deletions(-)

Chuck Lever

2011-09-21 18:51 UTC #5 lore raw

Quoted 8 lines · Steve Dickson

Quoted 5 lines · Bryan Schumaker
Quoted 4 lines · Steve Dickson
Both parser of nfsiostat and mountstats depend on the the first word in /proc/self/mountstats to be 'device' for them to parse through all the devices.

Quoted 7 lines

Quoted 5 lines
Quoted 3 lines
NFS mounts entries, in 3.1, no longer have start with the word 'device'. They start with the actual device, which confused both set of parsing routines.
I sent in a patch to fix this on the kernel end almost a month ago, but it must have gotten lost with the kernel.org confusion. I included what I came up with below
Interesting... Maybe we should still apply these patches until your patch shows up? They are definitely broken...

I think we should wait until we get some judgement from the VFS guys. Either:

o This is deemed a regression in a formal kernel API, in which case it's a kernel bug and should be fixed there, or

o This is deemed to be a permanent change to the kernel API, in which case we should address the problem in the python tools, as Steve has done

I think all the broken kernels still have stable releases, so it's possible that a kernel fix could address the whole problem. Bryan, if you are asked to repost, can you add a Cc: [email protected] below your SOB?

Quoted 46 lines

steved.
.
Quoted 31 lines
- Bryan
[PATCH] VFS: Add "device" tag to /proc/self/mountstats
nfsiostat was failing to find mounted filesystems on kernels after 2.6.38 because of changes to show_vfsstat() by commit c7f404b40a3665d9f4e9a927cc5c1ee0479ed8f9. This patch adds back the "device" tag before the nfs server entry so scripts can parse the mountstats file correctly.
Signed-off-by Bryan Schumaker <[email protected]>
---
fs/namespace.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
fs/namespace.c +1 -0
index 22bfe82..1429114 100644
@@ -1109,6 +1109,7 @@ static int show_vfsstat(struct seq_file *m, void *v)
 
 	/* device */
 	if (mnt->mnt_sb->s_op->show_devname) {
+		seq_puts(m, "device ");
 		err = mnt->mnt_sb->s_op->show_devname(m, mnt);
 	} else {
 		if (mnt->mnt_devname) {
Quoted 3 lines
Quoted 9 lines
Quoted 8 lines
Steve Dickson (2):
nfsiostat: Breaks on 3.1 kernels
mountstats: Breaks on 3.1 kernels
tools/mountstats/mountstats.py | 9 +++++++++
tools/nfs-iostat/nfs-iostat.py | 11 +++++++++--
2 files changed, 18 insertions(+), 2 deletions(-)
--
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

Signature · 2 lines

Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Bryan Schumaker

2011-09-21 19:03 UTC #6 lore raw

Quoted 19 lines · Chuck Lever

Quoted 8 lines · Steve Dickson
Quoted 5 lines · Bryan Schumaker
Quoted 4 lines · Steve Dickson
Both parser of nfsiostat and mountstats depend on the the first word in /proc/self/mountstats to be 'device' for them to parse through all the devices.
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
NFS mounts entries, in 3.1, no longer have start with the word 'device'. They start with the actual device, which confused both set of parsing routines.
I sent in a patch to fix this on the kernel end almost a month ago, but it must have gotten lost with the kernel.org confusion. I included what I came up with below
Interesting... Maybe we should still apply these patches until your patch shows up? They are definitely broken...

Quoted 7 lines

I think we should wait until we get some judgement from the VFS guys. Either:
o This is deemed a regression in a formal kernel API, in which case it's a kernel bug and should be fixed there, or
o This is deemed to be a permanent change to the kernel API, in which case we should address the problem in the python tools, as Steve has done
I think all the broken kernels still have stable releases, so it's possible that a kernel fix could address the whole problem. Bryan, if you are asked to repost, can you add a Cc: [email protected] below your SOB?

Yeah, no problem. Should I resend it with that line anyway?

- Bryan

Quoted 48 lines

Quoted 47 lines
steved.
.
Quoted 31 lines
- Bryan
[PATCH] VFS: Add "device" tag to /proc/self/mountstats
nfsiostat was failing to find mounted filesystems on kernels after 2.6.38 because of changes to show_vfsstat() by commit c7f404b40a3665d9f4e9a927cc5c1ee0479ed8f9. This patch adds back the "device" tag before the nfs server entry so scripts can parse the mountstats file correctly.
Signed-off-by Bryan Schumaker <[email protected]>
---
fs/namespace.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
fs/namespace.c +1 -0
index 22bfe82..1429114 100644
@@ -1109,6 +1109,7 @@ static int show_vfsstat(struct seq_file *m, void *v)
 
 	/* device */
 	if (mnt->mnt_sb->s_op->show_devname) {
+		seq_puts(m, "device ");
 		err = mnt->mnt_sb->s_op->show_devname(m, mnt);
 	} else {
 		if (mnt->mnt_devname) {
Quoted 3 lines
Quoted 9 lines
Quoted 8 lines
Steve Dickson (2):
nfsiostat: Breaks on 3.1 kernels
mountstats: Breaks on 3.1 kernels
tools/mountstats/mountstats.py | 9 +++++++++
tools/nfs-iostat/nfs-iostat.py | 11 +++++++++--
2 files changed, 18 insertions(+), 2 deletions(-)
--
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

A message that is not in the archive (sent off-list or not yet received): [email protected]

J. Bruce Fields

2011-09-21 18:51 UTC #8 lore raw

Quoted 8 lines · Steve Dickson

Quoted 5 lines · Bryan Schumaker
Quoted 4 lines · Steve Dickson
Both parser of nfsiostat and mountstats depend on the the first word in /proc/self/mountstats to be 'device' for them to parse through all the devices.

Quoted 7 lines

Quoted 5 lines
Quoted 3 lines
NFS mounts entries, in 3.1, no longer have start with the word 'device'. They start with the actual device, which confused both set of parsing routines.
I sent in a patch to fix this on the kernel end almost a month ago, but it must have gotten lost with the kernel.org confusion. I included what I came up with below
Interesting... Maybe we should still apply these patches until your patch shows up? They are definitely broken...

May as well fix the problem on both sides if possible.

--b.

Quoted 47 lines

steved.
.
Quoted 31 lines
- Bryan
[PATCH] VFS: Add "device" tag to /proc/self/mountstats
nfsiostat was failing to find mounted filesystems on kernels after 2.6.38 because of changes to show_vfsstat() by commit c7f404b40a3665d9f4e9a927cc5c1ee0479ed8f9. This patch adds back the "device" tag before the nfs server entry so scripts can parse the mountstats file correctly.
Signed-off-by Bryan Schumaker <[email protected]>
---
 fs/namespace.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
fs/namespace.c +1 -0
index 22bfe82..1429114 100644
@@ -1109,6 +1109,7 @@ static int show_vfsstat(struct seq_file *m, void *v)
 
 	/* device */
 	if (mnt->mnt_sb->s_op->show_devname) {
+		seq_puts(m, "device ");
 		err = mnt->mnt_sb->s_op->show_devname(m, mnt);
 	} else {
 		if (mnt->mnt_devname) {
Quoted 3 lines
Quoted 9 lines
Quoted 8 lines
Steve Dickson (2):
nfsiostat: Breaks on 3.1 kernels
mountstats: Breaks on 3.1 kernels
 tools/mountstats/mountstats.py |    9 +++++++++
 tools/nfs-iostat/nfs-iostat.py |   11 +++++++++--
 2 files changed, 18 insertions(+), 2 deletions(-)
--
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

Steve Dickson

author

2011-09-21 19:31 UTC #9 lore raw

Quoted 13 lines · J. Bruce Fields

Quoted 12 lines · Steve Dickson
Quoted 9 lines · Bryan Schumaker
Quoted 7 lines · Steve Dickson
Both parser of nfsiostat and mountstats depend on the the first word in /proc/self/mountstats to be 'device' for them to parse through all the devices.
NFS mounts entries, in 3.1, no longer have start with the word 'device'. They start with the actual device, which confused both set of parsing routines.

Quoted 5 lines

Quoted 3 lines
Quoted 1 line
I sent in a patch to fix this on the kernel end almost a month ago, but it must have gotten lost with the kernel.org confusion. I included what I came up with below
Interesting... Maybe we should still apply these patches until your patch shows up? They are definitely broken...
May as well fix the problem on both sides if possible.

I agree this.. I can always pull these back out if/when Bryan's patch shows up...

steved.

Quoted 50 lines

--b.
Quoted 47 lines
steved.
.
Quoted 31 lines
- Bryan
[PATCH] VFS: Add "device" tag to /proc/self/mountstats
nfsiostat was failing to find mounted filesystems on kernels after 2.6.38 because of changes to show_vfsstat() by commit c7f404b40a3665d9f4e9a927cc5c1ee0479ed8f9. This patch adds back the "device" tag before the nfs server entry so scripts can parse the mountstats file correctly.
Signed-off-by Bryan Schumaker <[email protected]>
---
 fs/namespace.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
fs/namespace.c +1 -0
index 22bfe82..1429114 100644
@@ -1109,6 +1109,7 @@ static int show_vfsstat(struct seq_file *m, void *v)
 
 	/* device */
 	if (mnt->mnt_sb->s_op->show_devname) {
+		seq_puts(m, "device ");
 		err = mnt->mnt_sb->s_op->show_devname(m, mnt);
 	} else {
 		if (mnt->mnt_devname) {
Quoted 3 lines
Quoted 9 lines
Quoted 8 lines
Steve Dickson (2):
nfsiostat: Breaks on 3.1 kernels
mountstats: Breaks on 3.1 kernels
 tools/mountstats/mountstats.py |    9 +++++++++
 tools/nfs-iostat/nfs-iostat.py |   11 +++++++++--
 2 files changed, 18 insertions(+), 2 deletions(-)
--
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

Bryan Schumaker

2011-09-21 19:34 UTC #10 lore raw

Quoted 16 lines · Steve Dickson

Quoted 13 lines · J. Bruce Fields
Quoted 12 lines · Steve Dickson
Quoted 9 lines · Bryan Schumaker
Quoted 7 lines · Steve Dickson
Both parser of nfsiostat and mountstats depend on the the first word in /proc/self/mountstats to be 'device' for them to parse through all the devices.
NFS mounts entries, in 3.1, no longer have start with the word 'device'. They start with the actual device, which confused both set of parsing routines.

Quoted 7 lines

Quoted 5 lines
Quoted 3 lines
Quoted 1 line
I sent in a patch to fix this on the kernel end almost a month ago, but it must have gotten lost with the kernel.org confusion. I included what I came up with below
Interesting... Maybe we should still apply these patches until your patch shows up? They are definitely broken...
May as well fix the problem on both sides if possible.
I agree this.. I can always pull these back out if/when Bryan's patch shows up...

Sounds good to me. It makes sense to have tools that work with current kernels.

Quoted 53 lines

steved.
Quoted 50 lines
--b.
Quoted 47 lines
steved.
.
Quoted 31 lines
- Bryan
[PATCH] VFS: Add "device" tag to /proc/self/mountstats
nfsiostat was failing to find mounted filesystems on kernels after 2.6.38 because of changes to show_vfsstat() by commit c7f404b40a3665d9f4e9a927cc5c1ee0479ed8f9. This patch adds back the "device" tag before the nfs server entry so scripts can parse the mountstats file correctly.
Signed-off-by Bryan Schumaker <[email protected]>
---
 fs/namespace.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
fs/namespace.c +1 -0
index 22bfe82..1429114 100644
@@ -1109,6 +1109,7 @@ static int show_vfsstat(struct seq_file *m, void *v)
 
 	/* device */
 	if (mnt->mnt_sb->s_op->show_devname) {
+		seq_puts(m, "device ");
 		err = mnt->mnt_sb->s_op->show_devname(m, mnt);
 	} else {
 		if (mnt->mnt_devname) {
Quoted 3 lines
Quoted 9 lines
Quoted 8 lines
Steve Dickson (2):
nfsiostat: Breaks on 3.1 kernels
mountstats: Breaks on 3.1 kernels
 tools/mountstats/mountstats.py |    9 +++++++++
 tools/nfs-iostat/nfs-iostat.py |   11 +++++++++--
 2 files changed, 18 insertions(+), 2 deletions(-)
--
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