Hello *,

I made a fix by sorting the read lines from /proc/diskstats.

Diff:

703c703
<       char line[256], dev_name[MAX_NAME_LEN], dev_name2[MAX_NAME_LEN];
---
>       char line[256], dev_name[MAX_NAME_LEN];
713,717d712
<         char *lines[256]; /* Array of 256 pointers to line-strings */
<         int line_len;
<         int line_nb=0; /* will contain the line (device) count */
<         int l_c, l_c2;
<         char *tmp;
724,725c719
<         /* /proc/diskstats contains an unordered list of devices */
<       if ((fp = fopen(DISKSTATS, "r")) == NULL) 
---
>       if ((fp = fopen(DISKSTATS, "r")) == NULL)
727,733d720
<       while (fgets(line, sizeof(line), fp) != NULL && line_nb < 256) {
<               line_len=strlen(line);
<               lines[line_nb]=malloc(line_len+1); /* Make room for the 
terminating \0 */
<               memcpy(lines[line_nb], line, line_len+1); /* memcpy is faster 
than strcpy */
<               line_nb++; 
<         }
<       fclose(fp);
735,754c722,723
<         /* Sort device list */
<         for (l_c=0; l_c < line_nb-1; l_c++)
<         {
<              sscanf(lines[l_c], "%u %u %s", &major, &minor, dev_name);
<              for (l_c2=l_c+1; l_c2 < line_nb; l_c2++)
<              {
<                  sscanf(lines[l_c2], "%u %u %s", &major, &minor, dev_name2);
<                  if (strncmp(dev_name, dev_name2, sizeof(line)) > 0) { // 
swap places
<                     tmp=lines[l_c]; 
<                     lines[l_c]=lines[l_c2];   
<                     lines[l_c2]=tmp;
<                     l_c--; /* walk thru it again */
<                    break;
<                  }
<              }
<         }
< 
<         /* Walk through device list */
<         for (l_c=0; l_c < line_nb; l_c++)
<         {
---
>       while (fgets(line, sizeof(line), fp) != NULL) {
> 
756c725
<               i = sscanf(lines[l_c], "%u %u %s %lu %lu %lu %lu %lu %lu %lu %u 
%u %u %u %lu %lu %lu %lu",
---
>               i = sscanf(line, "%u %u %s %lu %lu %lu %lu %lu %lu %lu %u %u %u 
> %u %lu %lu %lu %lu",
828,831c797
< 
<        /* Free the lines array */
<         for (l_c=0; l_c < line_nb; l_c++)
<             free(lines[l_c]);
---
>       fclose(fp);

Maybe more readable: the whole function…

/*
 ***************************************************************************
 * Read stats from /proc/diskstats.
 *
 * IN:
 * @curr        Index in array for current sample statistics.
 ***************************************************************************
 */
void read_diskstats_stat(int curr)
{
        FILE *fp;
        char line[256], dev_name[MAX_NAME_LEN], dev_name2[MAX_NAME_LEN];
        char *dm_name;
        struct io_stats sdev;
        int i;
        unsigned int ios_pgr, tot_ticks, rq_ticks, wr_ticks;
        unsigned long rd_ios, rd_merges_or_rd_sec, rd_ticks_or_wr_sec, wr_ios;
        unsigned long wr_merges, rd_sec_or_wr_ios, wr_sec;
        unsigned long dc_ios, dc_merges, dc_sec, dc_ticks;
        char *ioc_dname;
        unsigned int major, minor;
        char *lines[256]; /* Array of 256 pointers to line-strings */
        int line_len;
        int line_nb=0; /* will contain the line (device) count */
        int l_c, l_c2;
        char *tmp;

        memset(&sdev, 0, sizeof(struct io_stats));

        /* Every I/O device entry is potentially unregistered */
        set_entries_unregistered(iodev_nr, st_hdr_iodev);

        /* /proc/diskstats contains an unordered list of devices */
        if ((fp = fopen(DISKSTATS, "r")) == NULL) 
                return;
        while (fgets(line, sizeof(line), fp) != NULL && line_nb < 256) {
              line_len=strlen(line);
              lines[line_nb]=malloc(line_len+1); /* Make room for the 
terminating \0 */
              memcpy(lines[line_nb], line, line_len+1); /* memcpy is faster 
than strcpy */
              line_nb++; 
        }
        fclose(fp);

        /* Sort device list */
        for (l_c=0; l_c < line_nb-1; l_c++)
        {
             sscanf(lines[l_c], "%u %u %s", &major, &minor, dev_name);
             for (l_c2=l_c+1; l_c2 < line_nb; l_c2++)
             {
                 sscanf(lines[l_c2], "%u %u %s", &major, &minor, dev_name2);
                 if (strncmp(dev_name, dev_name2, sizeof(line)) > 0) { // swap 
places
                    tmp=lines[l_c]; 
                    lines[l_c]=lines[l_c2];   
                    lines[l_c2]=tmp;
                    l_c--; /* walk thru it again */
                   break;
                 }
             }
        }

        /* Walk through device list */
        for (l_c=0; l_c < line_nb; l_c++)
        {
                /* major minor name rio rmerge rsect ruse wio wmerge wsect wuse 
running use aveq dcio dcmerge dcsect dcuse*/
                i = sscanf(lines[l_c], "%u %u %s %lu %lu %lu %lu %lu %lu %lu %u 
%u %u %u %lu %lu %lu %lu",
                           &major, &minor, dev_name,
                           &rd_ios, &rd_merges_or_rd_sec, &rd_sec_or_wr_ios, 
&rd_ticks_or_wr_sec,
                           &wr_ios, &wr_merges, &wr_sec, &wr_ticks, &ios_pgr, 
&tot_ticks, &rq_ticks,
                           &dc_ios, &dc_merges, &dc_sec, &dc_ticks);

                if (i >= 14) {
                        /* Device or partition */
                        if (!dlist_idx && !DISPLAY_PARTITIONS(flags) &&
                            !is_device(dev_name, ACCEPT_VIRTUAL_DEVICES))
                                continue;
                        sdev.rd_ios     = rd_ios;
                        sdev.rd_merges  = rd_merges_or_rd_sec;
                        sdev.rd_sectors = rd_sec_or_wr_ios;
                        sdev.rd_ticks   = (unsigned int) rd_ticks_or_wr_sec;
                        sdev.wr_ios     = wr_ios;
                        sdev.wr_merges  = wr_merges;
                        sdev.wr_sectors = wr_sec;
                        sdev.wr_ticks   = wr_ticks;
                        sdev.ios_pgr    = ios_pgr;
                        sdev.tot_ticks  = tot_ticks;
                        sdev.rq_ticks   = rq_ticks;

                        if (i == 18) {
                                /* Discard I/O */
                                sdev.dc_ios     = dc_ios;
                                sdev.dc_merges  = dc_merges;
                                sdev.dc_sectors = dc_sec;
                                sdev.dc_ticks   = dc_ticks;
                        }
                }
                else if (i == 7) {
                        /* Partition without extended statistics */
                        if (DISPLAY_EXTENDED(flags) ||
                            (!dlist_idx && !DISPLAY_PARTITIONS(flags)))
                                continue;

                        sdev.rd_ios     = rd_ios;
                        sdev.rd_sectors = rd_merges_or_rd_sec;
                        sdev.wr_ios     = rd_sec_or_wr_ios;
                        sdev.wr_sectors = rd_ticks_or_wr_sec;
                }
                else
                        /* Unknown entry: Ignore it */
                        continue;

                if ((ioc_dname = ioc_name(major, minor)) != NULL) {
                        if (strcmp(dev_name, ioc_dname) && strcmp(ioc_dname, 
K_NODEV)) {
                                /*
                                 * No match: Use name generated from 
sysstat.ioconf data
                                 * (if different from "nodev") works around 
known issues
                                 * with EMC PowerPath.
                                 */
                                strncpy(dev_name, ioc_dname, MAX_NAME_LEN - 1);
                                dev_name[MAX_NAME_LEN - 1] = '\0';
                        }
                }

                if ((DISPLAY_DEVMAP_NAME(flags)) && (major == dm_major)) {
                        /*
                         * If the device is a device mapper device, try to get 
its
                         * assigned name of its logical device.
                         */
                        dm_name = transform_devmapname(major, minor);
                        if (dm_name) {
                                strncpy(dev_name, dm_name, MAX_NAME_LEN - 1);
                                dev_name[MAX_NAME_LEN - 1] = '\0';
                        }
                }

                save_stats(dev_name, curr, &sdev, iodev_nr, st_hdr_iodev);
        }

       /* Free the lines array */
        for (l_c=0; l_c < line_nb; l_c++)
            free(lines[l_c]);

        /* Free structures corresponding to unregistered devices */
        free_unregistered_entries(iodev_nr, st_hdr_iodev);
}

It would be great if this could be merged into the release so we all can enjoy 
the sorted devices :)

Thanks!

Heiko

PS: Maybe someone cares to check my sorting algorithm, I have a history of 
making off-by-ones and frankly, I don’t know if this is a good solution fort 
sorting either.

Reply via email to