]> granicus.if.org Git - sysstat/commitdiff
Fixed several bufs with nfsiostat (and cifsiostat).
authorSebastien Godard <sysstat@orange.fr>
Fri, 11 Mar 2011 13:45:05 +0000 (14:45 +0100)
committerSebastien Godard <sysstat@orange.fr>
Fri, 11 Mar 2011 13:45:05 +0000 (14:45 +0100)
Mail from Masanari Iida (masanari.iida@hp.com) 08/02/2011:

Sebastien,

Thanks for the fix.
Before you release the core on end of Feb,  I would ask you to test the
code with following 2 scenario.

(1)  Mount / Umount while nfsiostat running.
Check points
(a)  nfsiostat detect new nfs mount points after nfsiostat started.
The mounted NFS share have to be reported by nfsiostat.

(b)  nfsiostat detect nfs mount points which is umounted after nfsiostat started.
No lines reported by nfsiostat  after umount NFS share.

This is an original bug scenario that I reported to you in the first e-mail.

(2) nfsiostat not showing incorrect value when NFS mount point re-mount happen.
Following symptom was seen on sysstat 7.0.2 (on RHEL5).

Step to reproduce.
(1)  Mount an NFS share
(2)  Run iostat  (or nfsiostat)
(3)  Umount the NFS share
(4)  Mount the same NFS share that umounted in step 3.
(5)  Check the iostat result for the NFS share.

Actual Result on sysstat 7.0.2.
Very large value for rops/s and wops/s display one time.
These are incorrect values.

avg-cpu:  %user   %nice %system %iowait  %steal   %idle
           7.25    0.00    2.25    0.00    0.00   90.50

Device:            tps    kB_read/s    kB_wrtn/s    kB_read    kB_wrtn

Device:                   rkB_nor/s    wkB_nor/s    rkB_dir/s    wkB_dir/s    rkB_svr/s    wkB_svr/s    rops/s    wops/s
abc123.jpn.hp.com:/nfs-test         0.00         0.00         0.00         0.00         0.00         0.00 18446744073709551616.00 18446744073709551616.00

avg-cpu:  %user   %nice %system %iowait  %steal   %idle
           0.25    0.00    0.00    0.00    0.00   99.75

Device:            tps    kB_read/s    kB_wrtn/s    kB_read    kB_wrtn

Device:                   rkB_nor/s    wkB_nor/s    rkB_dir/s    wkB_dir/s    rkB_svr/s    wkB_svr/s    rops/s    wops/s
abc123.jpn.hp.com:/nfs-test         0.00         0.00         0.00         0.00         0.00         0.00      1.00      1.00

Expected result.
The nfsiostat reported correct value for rops/s and  wops/s
even when the NFS mount point re-mounted while nfsiostat running.

Regards,
Masanari Iida

Mail from Masanari Iida (masanari.iida@hp.com) 24/02/2011:

Hello,

Thank you for your support.

I have tested your new code.
The bugs that I have reported in previous e-mail are fixed on this version.
So I don't get no more coredump, and nfsiostat detect all mounted and
umounted filesystems while running.

One minor issue still remain here.
On 2nd result of the nfsiostat always include some unknown value.

swift.jpn.hp.com:/src          0.00         0.00         0.00         0.00         0.00         0.00      0.00      0.00      0.00
swift.jpn.hp.com:/src          0.00         0.00         0.00         0.00         0.00         0.00 516000.00      0.00      0.00   <<
swift.jpn.hp.com:/src          0.00         0.00         0.00         0.00         0.00         0.00      0.00      0.00      0.00
swift.jpn.hp.com:/src          0.00         0.00         0.00         0.00         0.00         0.00      0.00      0.00      0.00
swift.jpn.hp.com:/src          0.00         0.00         0.00         0.00         0.00         0.00      0.00      0.00      0.00

blxc:/hptc_cluster         0.00         0.00         0.00         0.00         0.00         0.00      0.00      0.00      0.00
blxc:/hptc_cluster  31130270.51         0.00         0.00         0.00   9139775.59         0.00 1280500.00 603100.00      0.00 <<
blxc:/hptc_cluster         0.00         0.00         0.00         0.00         0.00         0.00      0.00      0.00      0.00
blxc:/hptc_cluster         0.00         0.00         0.00         0.00         0.00         0.00      0.00      0.00      0.00
blxc:/hptc_cluster         0.00         0.00         0.00         0.00         0.00         0.00      0.00      0.00      0.00

(These 0 data are correct, since I just mount the NFS share and not doing
any I/O during the test. )

How to reproduce 1
(1) Mount one NFS filesystem.
(2) Run nfsiostat with interval and count options.  The count must be 3 or more.
(3) Check the 2nd result from nfsiostat.

How to reproduce 2
(1) Run nfsiostat with interval.
(2) Mount the NFS filesystem while running the nfsiostat.
(3) Check the 2nd result of the just mounted NFS filesystem.

I know that vmstat or iostat case,  the first result is a history of data since OS boot.
So I usually ignore the first data and use after the 2nd result.
In NFS automount enviroment,  NFS mount/umount happen at random timing.
So it is hard to remove these sudden large value from data.

Impact.
When a script draw a graph, these sudden large values expand Y scale.
So the normal value may looks smaller than expected in the graph.

Regards,
Masanari Iida

cifsiostat.c
nfsiostat.c
nls/sysstat.pot

index 0db7ea229cc0d430d76504d29c9ce185cba9b2b1..e03e839563a78a187976a42c96cd9d21b8a7fa6c 100644 (file)
@@ -230,11 +230,12 @@ void save_stats(char *name, int curr, struct cifs_stats *st_io)
        /* Look for CIFS directory in data table */
        for (i = 0; i < cifs_nr; i++) {
                st_hdr_cifs_i = st_hdr_cifs + i;
-               if (!strcmp(st_hdr_cifs_i->name, name)) {
+               if ((st_hdr_cifs_i->used == TRUE) &&
+                   (!strcmp(st_hdr_cifs_i->name, name))) {
                        break;
                }
        }
-       
+
        if (i == cifs_nr) {
                /*
                 * This is a new filesystem: Look for an unused entry to store it.
@@ -244,13 +245,19 @@ void save_stats(char *name, int curr, struct cifs_stats *st_io)
                        if (!st_hdr_cifs_i->used) {
                                /* Unused entry found... */
                                st_hdr_cifs_i->used = TRUE; /* Indicate it is now used */
+                               st_hdr_cifs_i->active = TRUE;
                                strcpy(st_hdr_cifs_i->name, name);
-                               st_cifs_i = st_cifs[!curr] + i;
-                               memset(st_cifs_i, 0, CIFS_STATS_SIZE);
+                               st_cifs_i = st_cifs[curr] + i;
+                               *st_cifs_i = *((struct cifs_stats *) st_io);
                                break;
                        }
                }
                if (i == cifs_nr) {
+                       /*
+                        * It is a new CIFS directory
+                        * but there is no free structure to store it.
+                        */
+
                        /* All entries are used: The number has to be increased */
                        cifs_nr = cifs_nr + 5;
 
@@ -263,8 +270,9 @@ void save_stats(char *name, int curr, struct cifs_stats *st_io)
 
                        /* Set the new entries inactive */
                        for (j = 0; j < 5; j++) {
-                               st_hdr_cifs_i = st_hdr_cifs + j;
+                               st_hdr_cifs_i = st_hdr_cifs + i + j;
                                st_hdr_cifs_i->used = FALSE;
+                               st_hdr_cifs_i->active = FALSE;
                        }
 
                        /* Increase the size of st_hdr_ionfs buffer */
@@ -274,19 +282,20 @@ void save_stats(char *name, int curr, struct cifs_stats *st_io)
                                        perror("malloc");
                                        exit(4);
                                }
+                               memset(st_cifs[j] + i, 0, 5 * CIFS_STATS_SIZE);
                        }
-
                        /* Now i shows the first unused entry of the new block */
                        st_hdr_cifs_i = st_hdr_cifs + i;
                        st_hdr_cifs_i->used = TRUE; /* Indicate it is now used */
+                       st_hdr_cifs_i->active = TRUE;
                        strcpy(st_hdr_cifs_i->name, name);
-                       st_cifs_i = st_cifs[!curr] + i;
-                       memset(st_cifs_i, 0, CIFS_STATS_SIZE);
+                       st_cifs_i = st_cifs[curr] + i;
+                       *st_cifs_i = *st_io;
                }
-       }
-       if (i < cifs_nr) {
+       } else {
                st_hdr_cifs_i = st_hdr_cifs + i;
                st_hdr_cifs_i->active = TRUE;
+               st_hdr_cifs_i->used = TRUE;
                st_cifs_i = st_cifs[curr] + i;
                *st_cifs_i = *st_io;
        }
@@ -408,6 +417,7 @@ void write_cifs_stat(int curr, unsigned long long itv, int fctr,
        else {
                printf("%-22s ", shi->name);
        }
+
        /*       rB/s   wB/s   fo/s   fc/s   fd/s*/
        printf("%12.2f %12.2f %9.2f %9.2f %12.2f %12.2f %12.2f \n",
               S_VALUE(ionj->rd_bytes, ioni->rd_bytes, itv) / fctr,
@@ -529,6 +539,7 @@ void rw_io_stat_loop(long int count, struct tm *rectime)
                if (count > 0) {
                        count--;
                }
+
                if (count) {
                        curr ^= 1;
                        pause();
index c657490118bf74d9dbb3c3d892aa29431c3d3a0a..bf0ba17e5bf7f99c51b5fa3620e224b0483977b3 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * nfsiostat: report NFS I/O statistics
+ * nfsiostat: Report NFS I/O statistics
  * Copyright (C) 2010 Red Hat, Inc. All Rights Reserved
  * Written by Ivana Varekova <varekova@redhat.com>
  *
@@ -138,7 +138,7 @@ int get_nfs_mount_nr(void)
                if ((strstr(line, "mounted")) && (strstr(line, "on")) &&
                    (strstr(line, "with")) && (strstr(line, "fstype"))) {
 
-                       sscanf(strstr(line, "fstype") + 6, "%10s", type_name);
+                       sscanf(strstr(line, "fstype") + 6, "%9s", type_name);
                        if ((!strncmp(type_name, "nfs", 3)) && (strncmp(type_name, "nfsd", 4))) {
                                nfs ++;
                        }
@@ -250,8 +250,7 @@ void io_sys_free(void)
  * @st_hdr_ionfs       Pointer on structures describing an NFS filesystem.
  ***************************************************************************
  */
-void save_stats(char *name, int curr, void *st_io, int ionfs_nr,
-               struct io_hdr_stats *st_hdr_ionfs)
+void save_stats(char *name, int curr, void *st_io)
 {
        int i, j;
        struct io_hdr_stats *st_hdr_ionfs_i;
@@ -260,11 +259,12 @@ void save_stats(char *name, int curr, void *st_io, int ionfs_nr,
        /* Look for NFS directory in data table */
        for (i = 0; i < ionfs_nr; i++) {
                st_hdr_ionfs_i = st_hdr_ionfs + i;
-               if (!strcmp(st_hdr_ionfs_i->name, name)) {
+               if ((st_hdr_ionfs_i->used) &&
+                   (!strcmp(st_hdr_ionfs_i->name, name))) {
                        break;
                }
        }
-       
+
        if (i == ionfs_nr) {
                /*
                 * This is a new filesystem: Look for an unused entry to store it.
@@ -274,9 +274,12 @@ void save_stats(char *name, int curr, void *st_io, int ionfs_nr,
                        if (!st_hdr_ionfs_i->used) {
                                /* Unused entry found... */
                                st_hdr_ionfs_i->used = TRUE; /* Indicate it is now used */
+                               st_hdr_ionfs_i->active = TRUE;
+
                                strcpy(st_hdr_ionfs_i->name, name);
-                               st_ionfs_i = st_ionfs[!curr] + i;
+                               st_ionfs_i = st_ionfs[curr] + i;
                                memset(st_ionfs_i, 0, IO_NFS_STATS_SIZE);
+                               *st_ionfs_i = *((struct io_nfs_stats *) st_io);
                                break;
                        }
                }
@@ -293,8 +296,9 @@ void save_stats(char *name, int curr, void *st_io, int ionfs_nr,
 
                        /* Set the new entries inactive */
                        for (j = 0; j < 5; j++) {
-                               st_hdr_ionfs_i = st_hdr_ionfs + j;
+                               st_hdr_ionfs_i = st_hdr_ionfs + i + j;
                                st_hdr_ionfs_i->used = FALSE;
+                               st_hdr_ionfs_i->active = FALSE;
                        }
 
                        /* Increase the size of st_hdr_ionfs buffer */
@@ -304,18 +308,19 @@ void save_stats(char *name, int curr, void *st_io, int ionfs_nr,
                                        perror("malloc");
                                        exit(4);
                                }
+                               memset(st_ionfs[j] + i, 0, 5 * IO_NFS_STATS_SIZE);
                        }
 
                        /* Now i shows the first unused entry of the new block */
                        st_hdr_ionfs_i = st_hdr_ionfs + i;
                        st_hdr_ionfs_i->used = TRUE; /* Indicate it is now used */
                        strcpy(st_hdr_ionfs_i->name, name);
-                       st_ionfs_i = st_ionfs[!curr] + i;
+                       st_ionfs_i = st_ionfs[curr] + i;
                        memset(st_ionfs_i, 0, IO_NFS_STATS_SIZE);
                }
-       }
-       if (i < ionfs_nr) {
+       } else  {
                st_hdr_ionfs_i = st_hdr_ionfs + i;
+               st_hdr_ionfs_i->used = TRUE;
                st_hdr_ionfs_i->active = TRUE;
                st_ionfs_i = st_ionfs[curr] + i;
                *st_ionfs_i = *((struct io_nfs_stats *) st_io);
@@ -354,24 +359,23 @@ void read_nfs_stat(int curr)
                return;
 
        sprintf(aux, "%%%ds",
-               MAX_NAME_LEN < 200 ? MAX_NAME_LEN : 200);
+               MAX_NAME_LEN < 200 ? MAX_NAME_LEN-1 : 200);
 
        while (fgets(line, 256, fp) != NULL) {
-
                /* Read NFS directory name */
                if (!strncmp(line, "device", 6)) {
                        sw = 0;
                        sscanf(line + 6, aux, nfs_name);
                        mount_part = strchr(line + 7, ' ');
                        if (mount_part != NULL) {
-                               sscanf(mount_part, "%10s %10s", mount, on);
+                               sscanf(mount_part, "%9s %9s", mount, on);
                                if ((!strncmp(mount, "mounted", 7)) && (!strncmp(on, "on", 2))) {
                                        sw = 1;
                                }
                        }
                }
 
-               sscanf(line, "%10s", prefix);
+               sscanf(line, "%9s", prefix);
                if (sw && (!strncmp(prefix, "bytes:", 6))) {
                        /* Read the stats for the last NFS-mounted directory */
                        sscanf(strstr(line, "bytes:") + 6, "%llu %llu %llu %llu %llu %llu",
@@ -424,7 +428,8 @@ void read_nfs_stat(int curr)
                                }
                                else if (!strncmp(operation, "WRITE:", 6)) {
                                        snfs.nfs_wops = v1;
-                                       save_stats(nfs_name, curr, &snfs, ionfs_nr, st_hdr_ionfs);
+
+                                       save_stats(nfs_name, curr, &snfs);
                                        sw = 0;
                                }
                        }
@@ -599,7 +604,6 @@ void rw_io_stat_loop(long int count, struct tm *rectime)
                        uptime0[curr] = 0;
                        read_uptime(&(uptime0[curr]));
                }
-
                /* Read NFS directories stats */
                read_nfs_stat(curr);
 
index b3c1052505d90110536046deda6fca6081941448..8edbe331866d4e32d915d13ce198567118d24e4b 100644 (file)
@@ -8,7 +8,7 @@ msgid ""
 msgstr ""
 "Project-Id-Version: PACKAGE VERSION\n"
 "Report-Msgid-Bugs-To: sysstat <at> orange.fr\n"
-"POT-Creation-Date: 2011-01-07 14:45+0100\n"
+"POT-Creation-Date: 2011-03-06 20:22+0100\n"
 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <LL@li.org>\n"
@@ -41,7 +41,7 @@ msgstr ""
 msgid "sysstat version %s\n"
 msgstr ""
 
-#: ioconf.c:490 rd_stats.c:72 rd_stats.c:2179 sa_common.c:1061 sadc.c:623
+#: ioconf.c:490 rd_stats.c:68 rd_stats.c:2003 sa_common.c:1061 sadc.c:623
 #: sadc.c:632 sadc.c:692
 #, c-format
 msgid "Cannot open %s: %s\n"
@@ -76,7 +76,7 @@ msgid ""
 "ALL } ] [ -V ]\n"
 msgstr ""
 
-#: mpstat.c:535 pidstat.c:1819 sar.c:382
+#: mpstat.c:535 pidstat.c:1819 sar.c:380
 msgid "Average:"
 msgstr ""
 
@@ -94,12 +94,12 @@ msgid ""
 "[ -p { <pid> [,...] | SELF | ALL } ] [ -T { TASK | CHILD | ALL } ]\n"
 msgstr ""
 
-#: pidstat.c:199 sar.c:1017
+#: pidstat.c:199 sar.c:1015
 #, c-format
 msgid "Requested activities not available\n"
 msgstr ""
 
-#: rd_stats.c:2225
+#: rd_stats.c:2049
 #, c-format
 msgid "Cannot handle so many processors!\n"
 msgstr ""
@@ -202,7 +202,7 @@ msgstr ""
 msgid "\t[Unknown activity format]"
 msgstr ""
 
-#: sar.c:106
+#: sar.c:105
 #, c-format
 msgid ""
 "Options are:\n"
@@ -214,39 +214,39 @@ msgid ""
 "[ -i <interval> ] [ -s [ <hh:mm:ss> ] ] [ -e [ <hh:mm:ss> ] ]\n"
 msgstr ""
 
-#: sar.c:128
+#: sar.c:126
 #, c-format
 msgid "Main options and reports:\n"
 msgstr ""
 
-#: sar.c:129
+#: sar.c:127
 #, c-format
 msgid "\t-b\tI/O and transfer rate statistics\n"
 msgstr ""
 
-#: sar.c:130
+#: sar.c:128
 #, c-format
 msgid "\t-B\tPaging statistics\n"
 msgstr ""
 
-#: sar.c:131
+#: sar.c:129
 #, c-format
 msgid "\t-d\tBlock device statistics\n"
 msgstr ""
 
-#: sar.c:132
+#: sar.c:130
 #, c-format
 msgid "\t-H\tHugepages utilization statistics\n"
 msgstr ""
 
-#: sar.c:133
+#: sar.c:131
 #, c-format
 msgid ""
 "\t-I { <int> | SUM | ALL | XALL }\n"
 "\t\tInterrupts statistics\n"
 msgstr ""
 
-#: sar.c:135
+#: sar.c:133
 #, c-format
 msgid ""
 "\t-m { <keyword> [,...] | ALL }\n"
@@ -259,7 +259,7 @@ msgid ""
 "\t\tTEMP\tDevices temperature\n"
 msgstr ""
 
-#: sar.c:143
+#: sar.c:141
 #, c-format
 msgid ""
 "\t-n { <keyword> [,...] | ALL }\n"
@@ -285,84 +285,84 @@ msgid ""
 "\t\tUDP6\tUDP traffic\t(v6)\n"
 msgstr ""
 
-#: sar.c:164
+#: sar.c:162
 #, c-format
 msgid "\t-q\tQueue length and load average statistics\n"
 msgstr ""
 
-#: sar.c:165
+#: sar.c:163
 #, c-format
 msgid "\t-r\tMemory utilization statistics\n"
 msgstr ""
 
-#: sar.c:166
+#: sar.c:164
 #, c-format
 msgid "\t-R\tMemory statistics\n"
 msgstr ""
 
-#: sar.c:167
+#: sar.c:165
 #, c-format
 msgid "\t-S\tSwap space utilization statistics\n"
 msgstr ""
 
-#: sar.c:168
+#: sar.c:166
 #, c-format
 msgid ""
 "\t-u [ ALL ]\n"
 "\t\tCPU utilization statistics\n"
 msgstr ""
 
-#: sar.c:170
+#: sar.c:168
 #, c-format
 msgid "\t-v\tKernel table statistics\n"
 msgstr ""
 
-#: sar.c:171
+#: sar.c:169
 #, c-format
 msgid "\t-w\tTask creation and system switching statistics\n"
 msgstr ""
 
-#: sar.c:172
+#: sar.c:170
 #, c-format
 msgid "\t-W\tSwapping statistics\n"
 msgstr ""
 
-#: sar.c:173
+#: sar.c:171
 #, c-format
 msgid "\t-y\tTTY device statistics\n"
 msgstr ""
 
-#: sar.c:216
+#: sar.c:214
 #, c-format
 msgid "End of data collecting unexpected\n"
 msgstr ""
 
-#: sar.c:806
+#: sar.c:804
 #, c-format
 msgid "Invalid data format\n"
 msgstr ""
 
-#: sar.c:810
+#: sar.c:808
 #, c-format
 msgid "Using a wrong data collector from a different sysstat version\n"
 msgstr ""
 
-#: sar.c:834
+#: sar.c:832
 #, c-format
 msgid "Inconsistent input data\n"
 msgstr ""
 
-#: sar.c:1264
+#: sar.c:1262
 #, c-format
 msgid "-f and -o options are mutually exclusive\n"
 msgstr ""
 
-#: sar.c:1270
+#: sar.c:1268
 #, c-format
 msgid "Not reading from a system activity file (use -f option)\n"
 msgstr ""
 
-#: sar.c:1397
+#: sar.c:1395
 #, c-format
 msgid "Cannot find the data collector (%s)\n"
 msgstr ""