iostat and more than one core

2010-11-17 Thread Luis Useche
Hi Guys,

Doing a small disk benchmark in my laptop with dd, I found that dd and
iostats were reporting different numbers. To be precise, iostat was
returning half of the MB/sec than dd (24.5 vs 49 MB/sec).

Digging a bit on the iostat code, I realized that the struct _disk cpu
time was returning 200 timer ticks even though it was read every second. I
tested by booting my machine with bsd.sp (to use one core instead of two)
and now it was returning the right number.

It seems like the number of ticks are incrementing by a factor proportional
to the number of CPUs (I think this makes sense). This makes iostat to
report the wrong bandwidth disk utilization.

Here is a patch I implemented to fix the problem:

diff --git a/usr.sbin/iostat/iostat.c b/usr.sbin/iostat/iostat.c
index 7da45b5..ffcd43a 100644
--- a/usr.sbin/iostat/iostat.c
+++ b/usr.sbin/iostat/iostat.c
@@ -64,6 +64,8 @@

 #include sys/dkstat.h
 #include sys/time.h
+#include sys/param.h
+#include sys/sysctl.h

 #include err.h
 #include ctype.h
@@ -84,7 +86,7 @@ extern intdk_ndrive;
 kvm_t *kd;
 char*nlistf, *memf;

-inthz, reps, interval;
+inthz, reps, interval, ncpu;
 static inttodo = 0;

 volatile sig_atomic_t wantheader;
@@ -112,8 +114,9 @@ int dkinit(int);
 int
 main(int argc, char *argv[])
 {
-int ch, hdrcnt;
+int ch, hdrcnt, mib[2];
 struct timevaltv;
+size_t size;

 while ((ch = getopt(argc, argv, Cc:dDIM:N:Tw:)) != -1)
 switch(ch) {
@@ -156,6 +159,11 @@ main(int argc, char *argv[])
 if (!ISSET(todo, SHOW_CPU | SHOW_TTY | SHOW_STATS_1 | SHOW_STATS_2))
 todo |= SHOW_CPU | SHOW_TTY | SHOW_STATS_1;

+mib[0] = CTL_HW;
+mib[1] = HW_NCPU;
+size = sizeof(ncpu);
+(void) sysctl(mib, 2, ncpu, size, NULL, 0);
+
 dkinit(0);
 dkreadstats();
 selectdrives(argv);
@@ -342,7 +350,7 @@ display(void)
 if (etime == 0.0)
 etime = 1.0;
 /* Convert to seconds. */
-etime /= (float)hz;
+etime /= (float)hz*ncpu;

 /* If we're showing totals only, then don't divide by the
  * system time.


Any thoughts?
Luis.



Re: iostat and more than one core

2010-11-17 Thread Benny Löfgren

On 2010-11-17 18.23, Luis Useche wrote:

Doing a small disk benchmark in my laptop with dd, I found that dd and
iostats were reporting different numbers. To be precise, iostat was
returning half of the MB/sec than dd (24.5 vs 49 MB/sec).

Digging a bit on the iostat code, I realized that the struct _disk cpu
time was returning 200 timer ticks even though it was read every second. I
tested by booting my machine with bsd.sp (to use one core instead of two)
and now it was returning the right number.


I've noticed the same phenomenon with iostat but haven't had time to 
investigate. So now I did some iostat runs on a few of my servers with 
1, 2, 4 and 8 cores and can definitely verify your findings.


I also noted that the tty columns are affected by the same cpu scaling 
problem, and your patch neatly fixes that as well.



It seems like the number of ticks are incrementing by a factor proportional
to the number of CPUs (I think this makes sense). This makes iostat to
report the wrong bandwidth disk utilization.



Here is a patch I implemented to fix the problem:

diff --git a/usr.sbin/iostat/iostat.c b/usr.sbin/iostat/iostat.c
index 7da45b5..ffcd43a 100644
--- a/usr.sbin/iostat/iostat.c
+++ b/usr.sbin/iostat/iostat.c
@@ -64,6 +64,8 @@

  #includesys/dkstat.h
  #includesys/time.h
+#includesys/param.h
+#includesys/sysctl.h

  #includeerr.h
  #includectype.h
@@ -84,7 +86,7 @@ extern intdk_ndrive;
  kvm_t *kd;
  char*nlistf, *memf;

-inthz, reps, interval;
+inthz, reps, interval, ncpu;
  static inttodo = 0;

  volatile sig_atomic_t wantheader;
@@ -112,8 +114,9 @@ int dkinit(int);
  int
  main(int argc, char *argv[])
  {
-int ch, hdrcnt;
+int ch, hdrcnt, mib[2];
  struct timevaltv;
+size_t size;

  while ((ch = getopt(argc, argv, Cc:dDIM:N:Tw:)) != -1)
  switch(ch) {
@@ -156,6 +159,11 @@ main(int argc, char *argv[])
  if (!ISSET(todo, SHOW_CPU | SHOW_TTY | SHOW_STATS_1 | SHOW_STATS_2))
  todo |= SHOW_CPU | SHOW_TTY | SHOW_STATS_1;

+mib[0] = CTL_HW;
+mib[1] = HW_NCPU;
+size = sizeof(ncpu);
+(void) sysctl(mib, 2,ncpu,size, NULL, 0);


You really should check this for errors, however unlikely it is to fail.


  dkinit(0);
  dkreadstats();
  selectdrives(argv);
@@ -342,7 +350,7 @@ display(void)
  if (etime == 0.0)
  etime = 1.0;
  /* Convert to seconds. */
-etime /= (float)hz;
+etime /= (float)hz*ncpu;

  /* If we're showing totals only, then don't divide by the
   * system time.


Any thoughts?
Luis.




Regards,
/Benny

--
internetlabbet.se / work:   +46 8 551 124 80  / Words must
Benny Lvfgren/  mobile: +46 70 718 11 90 /   be weighed,
/   fax:+46 8 551 124 89/not counted.
   /email:  benny -at- internetlabbet.se