[procps] [PATCH] 'vmstat -S M 1' and 'vmstat -S m 1' output of the 'si and 'so' fields is always zero

  • From: Jaromir Capik <jcapik@xxxxxxxxxx>
  • To: procps-ng <procps@xxxxxxxxxxxxx>
  • Date: Mon, 23 Jul 2012 14:09:27 -0400 (EDT)

Hello guys.

One of our engineers discovered a bug in the vmstat unit conversions.

I analysed the code and found the following ...

'si' and 'so' values depend on the result of the unitConvert
function where the output is a fixed-point size of kb_per_page
after the conversion. It gives 4 for kB units and 0 for MB units.
This also causes problems when switching between 'K' and 'k'
since the output value is 4 in both cases and the result for
'k' and 'K' then doesn't differ ... I swapped the conversion with
multiplication in order to make the number higher so it doesn't
lose precision. Since the unitConvert now accepts long instead
of int, I had to change the input type from int to long.
Maybe we could switch from float to double in the unitConvert
function one day ... 

The patch is attached ...

Btw ... There might be more bugs like that, but I haven't searched
for them yet ...

Regards,
Jaromir.

--
Jaromir Capik
Red Hat Czech, s.r.o.
Software Engineer / BaseOS

Email: jcapik@xxxxxxxxxx
Web: www.cz.redhat.com
Red Hat Czech s.r.o., Purkynova 99/71, 612 45, Brno, Czech Republic
IC: 27690016 


diff --git a/vmstat.c b/vmstat.c
index 669a263..93f620d 100644
--- a/vmstat.c
+++ b/vmstat.c
@@ -220,7 +220,7 @@ static void new_header(void)
             _("wa"));
 }
 
-static unsigned long unitConvert(unsigned int size)
+static unsigned long unitConvert(unsigned long size)
 {
        float cvSize;
        cvSize = (float)size / dataUnit * ((statMode == SLABSTAT) ? 1 : 1024);
@@ -264,8 +264,8 @@ static void new_format(void)
               unitConvert(kb_swap_used), unitConvert(kb_main_free),
               unitConvert(a_option?kb_inactive:kb_main_buffers),
               unitConvert(a_option?kb_active:kb_main_cached),
-              (unsigned)( (*pswpin  * unitConvert(kb_per_page) * hz + divo2) / 
Div ),
-              (unsigned)( (*pswpout * unitConvert(kb_per_page) * hz + divo2) / 
Div ),
+              (unsigned)( (unitConvert(*pswpin  * kb_per_page) * hz + divo2) / 
Div ),
+              (unsigned)( (unitConvert(*pswpout * kb_per_page) * hz + divo2) / 
Div ),
               (unsigned)( (*pgpgin                * hz + divo2) / Div ),
               (unsigned)( (*pgpgout               * hz + divo2) / Div ),
               (unsigned)( (*intr                  * hz + divo2) / Div ),
@@ -320,9 +320,9 @@ static void new_format(void)
                       unitConvert(a_option?kb_inactive:kb_main_buffers),
                       unitConvert(a_option?kb_active:kb_main_cached),
                       /*si */
-                      (unsigned)( ( (pswpin [tog] - pswpin 
[!tog])*unitConvert(kb_per_page)+sleep_half )/sleep_time ),
+                      (unsigned)( ( unitConvert((pswpin [tog] - pswpin 
[!tog])*kb_per_page)+sleep_half )/sleep_time ),
                       /* so */
-                      (unsigned)( ( (pswpout[tog] - 
pswpout[!tog])*unitConvert(kb_per_page)+sleep_half )/sleep_time ),
+                      (unsigned)( ( unitConvert((pswpout[tog] - 
pswpout[!tog])*kb_per_page)+sleep_half )/sleep_time ),
                       /* bi */
                       (unsigned)( (  pgpgin [tog] - pgpgin [!tog]             
+sleep_half )/sleep_time ),
                       /* bo */

Other related posts: