Re: SMP per-CPU statistics under FreeBSD.

On Mon, Apr 17, 2006 at 10:46:56PM +0000, Marco van Tol wrote:
> On Mon, Apr 17, 2006 at 10:43:14AM -0500, Bill WIlson wrote:
> > On Mon, 17 Apr 2006 14:03:58 +0000
> > Marco van Tol <[email protected]> wrote:
> > 
> > > I asked on the FreeBSD hackers mailing list whether per CPU statistics were
> > > available under FreeBSD, with the intention to modify gkrellm 2.2.9 to
> > > support it.  I had seen per CPU statistics with gkrellm under linux, and
> > > really liked it.  I'm running an AMD Athlon64 dual-core.
> > > 
> > > John Baldwin has been very helpful and supplied me a patch to get per-CPU
> > > statistics under FreeBSD as that's not available in the main trunk yet.
> > > http://lists.freebsd.org/pipermail/freebsd-hackers/2006-April/016093.html
> > > 
> > > I will try to modify gkrellm 2.2.9 to support what this patch does, and mail
> > > a patch for gkrellm over here if I succeed.
> > > 
> > > Is there anything else I should be aware of while making this attempt?
> > 
> > The cpu interface is fairly simple and is briefly documented in gkrellm-sysdeps.h.
> > The things to know are:
> > 
> > 1) Give the gkrellm_cpu_set_number_of_cpus() the number of real CPUs (ie
> > don't include composite even if you report data for a composite CPU).
> > 
> > 2) You have the choice of reporting composite CPU data yourself or letting gkrellm
> > calculate the composite data.  Just never call gkrellm_cpu_assign_composite_data()
> > and gkrellm will do it for you.
> > 
> > 3) You probably don't need to use the gkrellm_cpu_add_instance() call as it
> > is meant for architectures where non-contiguous CPU numbers will be reported.
> > This can happen on Solaris where for example the reporting can be for CPU
> > numbers 0,1 and 3 with 2 missing.
> > 
> > 3) You should probably test that your implementation works in client/server mode.
> > You can do this on a single machine.  Just start gkrellmd and then run
> > gkrellm -s localhost or whatever is needed on FreeBSD.
> 
> Thank you very much for these pointers.  Much appreciated! :)
> I hope to have something to play with soon.

The attached patch makes it work only in non-client/server mode.  It's
really cool, I like it a lot. :)

In client/server mode it only shows one CPU.  I'll look into why that is,
but I wanted to send you this patch already so you can see whether you like
the way I wrote it or not.  It'd be nice if this ended up in your CVS tree
at some point when you're satisfied with what I did. ;)

I tested it both on FreeBSD 6.1-RC1 without the pcpu_time patch, which makes
it work the way it does now, and on FreeBSD 7-CURRENT with the pcpu_time
patch which makes it show the number of CPU's in my system with specific
data for each of them.

I used the netbsd.c in sysdeps/ as sample code while writing this so thanks
to Bill Wilson who is listed as its author for that.

For now I'm off to bed (it's slightly past 3am in NL), but I'll look into if
I can get it to work in client/server mode as well.

Ow, and a little side note, I changed the 'int len' variables, which were to
go as argument 4 into sysctl into size_t's, which is what they should be
these days under FreeBSD.  They generated warnings during compilation of
freebsd.c.  There were only 2 of them outside of the code that I was
changing in freebsd.c.  I only changed them in freebsd.c.

Marco

-- 
...the only way to maintain a moderate sum of happiness in this life,
is not to worry about the future or regret the past too much.
- Mel Gibson

_______________________________________________
Gkrellm mailing list
[email protected]
http://ninja.linux-phreak.biz/mailman/listinfo/gkrellm

--- freebsd.c.orig	Sat Apr 22 02:16:26 2006
+++ freebsd.c	Sun Apr 23 02:33:46 2006
@@ -86,25 +86,49 @@
 #endif
 #include <kvm.h>
 
-
 extern	kvm_t	*kvmd;
 
 static int	oid_cp_time[CTL_MAXNAME + 2];
 static size_t	oid_cp_time_len = sizeof(oid_cp_time);
 static gint	have_cp_time;
 
+static int	oid_pcpu_time[CTL_MAXNAME + 2];
+static size_t	oid_pcpu_time_len = sizeof(oid_cp_time);
+static gint	have_pcpu_time;
+
+static gint	ncpus;
+
+static gint
+get_ncpus(void)
+{
+	int ncpu;
+	size_t len = sizeof(ncpu);
+
+	if (sysctlbyname("hw.ncpu", &ncpu, &len, NULL, 0) < 0)
+		return 1;
+	else
+		return ncpu;
+}
+
 void
 gkrellm_sys_cpu_read_data(void)
 	{
-	long		cp_time[CPUSTATES];
-	int		len = sizeof(cp_time);
+	long		cp_time[ncpus][CPUSTATES];
+	size_t		len = sizeof(cp_time);
+	int		i;
 	static struct nlist nl[] = {
 #define N_CP_TIME	0
 		{ "_cp_time" },
 		{ "" }
 	};
 
-	if (have_cp_time)
+	if (have_pcpu_time)
+		{
+		if (sysctl(oid_pcpu_time, oid_pcpu_time_len,
+			   cp_time, &len, NULL, 0) < 0)
+			return;
+		}
+	else if (have_cp_time)
 		{
 		if (sysctl(oid_cp_time, oid_cp_time_len,
 			   cp_time, &len, 0, 0) < 0)
@@ -122,18 +146,28 @@
 			return;
 		}
 
-	/* Currently, SMP is not supported */
-	gkrellm_cpu_assign_data(0, cp_time[CP_USER], cp_time[CP_NICE],
-				cp_time[CP_SYS], cp_time[CP_IDLE]);
+	for (i = 0 ; i < ncpus ; i++)
+		gkrellm_cpu_assign_data(i, cp_time[i][CP_USER],
+					cp_time[i][CP_NICE], cp_time[i][CP_SYS],
+					cp_time[i][CP_IDLE]);
 	}
 
 gboolean
 gkrellm_sys_cpu_init(void)
 	{
-	static char	*name = "kern.cp_time";
+	static char	*name = "kern.pcpu_time";
 
-	gkrellm_cpu_set_number_of_cpus(1);
+	if (gk_sysctlnametomib(name, oid_pcpu_time, &oid_pcpu_time_len) == 0)
+		{
+		++have_pcpu_time;
+		ncpus = get_ncpus();
+		gkrellm_cpu_set_number_of_cpus(ncpus);
+		return TRUE;
+		}
 
+	ncpus = 1;
+	gkrellm_cpu_set_number_of_cpus(ncpus);
+	name = "kern.cp_time";
 	if (gk_sysctlnametomib(name, oid_cp_time, &oid_cp_time_len) < 0)
 		return TRUE;
 	++have_cp_time;
@@ -206,7 +240,7 @@
 	static u_int	n_processes, n_forks = 0, curpid = -1;
 	u_int		n_vforks, n_rforks;
 	gint		r_forks, r_vforks, r_rforks;
-	gint		len;
+	size_t		len;
 	gint		nextpid, nforked;
 	static struct nlist nl[] = {
 #define N_NEXTPID	0
@@ -570,7 +604,7 @@
 	gint		tcp_status;
 	struct xinpgen	*xig, *oxig;
 	gchar		*buf;
-	gint		len = 0;
+	size_t		len = 0;
 
 	if (!initialized)
 		{