From owner-freebsd-current@FreeBSD.ORG Sat Jul 9 08:30:47 2011 Return-Path: Delivered-To: current@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 1233) id D0B9A1065670; Sat, 9 Jul 2011 08:30:47 +0000 (UTC) Date: Sat, 9 Jul 2011 08:30:47 +0000 From: Alexander Best To: John Baldwin Message-ID: <20110709083047.GA86927@freebsd.org> References: <201107081511.43417.jhb@freebsd.org> <20110708215051.GA14098@freebsd.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20110708215051.GA14098@freebsd.org> Cc: current@freebsd.org, edwin@freebsd.org Subject: Re: [PATCH] Make top -P an interactive toggle X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 09 Jul 2011 08:30:47 -0000 On Fri Jul 8 11, Alexander Best wrote: > On Fri Jul 8 11, John Baldwin wrote: > > This patch lets you use 'P' while top is running to toggle between per-CPU and > > global CPU stats. > > very cool. i always thought that being able to interactivly enable/disable > per-cpu stats in top would be a useful feature. great to see this being > implemented. oh...and of course i tested your patch. ;) works great without any issues. > > top is nearing perfection. ;) > > > > > Index: contrib/top/top.c > > =================================================================== > > --- contrib/top/top.c (revision 223873) > > +++ contrib/top/top.c (working copy) > > @@ -196,9 +196,9 @@ > > fd_set readfds; > > > > #ifdef ORDER > > - static char command_chars[] = "\f qh?en#sdkriIutHmSCajzo"; > > + static char command_chars[] = "\f qh?en#sdkriIutHmSCajzPo"; > > #else > > - static char command_chars[] = "\f qh?en#sdkriIutHmSCajz"; > > + static char command_chars[] = "\f qh?en#sdkriIutHmSCajzP"; > > #endif > > /* these defines enumerate the "strchr"s of the commands in command_chars */ > > #define CMD_redraw 0 > > @@ -225,8 +225,9 @@ > > #define CMD_showargs 20 > > #define CMD_jidtog 21 > > #define CMD_kidletog 22 > > +#define CMD_pcputog 23 > > #ifdef ORDER > > -#define CMD_order 23 > > +#define CMD_order 24 > > #endif > > > > /* set the buffer for stdout */ > > @@ -411,7 +412,7 @@ > > break; > > > > case 'P': > > - pcpu_stats = Yes; > > + pcpu_stats = !pcpu_stats; > > break; > > > > case 'z': > > @@ -1088,6 +1089,12 @@ > > ps.kidle ? "D" : "Not d"); > > putchar('\r'); > > break; > > + case CMD_pcputog: > > + pcpu_stats = !pcpu_stats; > > + toggle_pcpustats(&statics); > > + max_topn = display_updatecpus(&statics); > > + reset_display(); > > + break; > > default: > > new_message(MT_standout, " BAD CASE IN SWITCH!"); > > putchar('\r'); > > Index: contrib/top/display.c > > =================================================================== > > --- contrib/top/display.c (revision 223873) > > +++ contrib/top/display.c (working copy) > > @@ -151,16 +151,14 @@ > > return(smart_terminal ? lines : Largest); > > } > > > > -int display_init(statics) > > +int display_updatecpus(statics) > > > > struct statics *statics; > > > > { > > register int lines; > > - register char **pp; > > - register int *ip; > > register int i; > > - > > + > > /* call resize to do the dirty work */ > > lines = display_resize(); > > num_cpus = statics->ncpus; > > @@ -170,6 +168,21 @@ > > for (i = num_cpus; i > 9; i /= 10) > > cpustates_column++; > > > > + return(lines); > > +} > > + > > +int display_init(statics) > > + > > +struct statics *statics; > > + > > +{ > > + register int lines; > > + register char **pp; > > + register int *ip; > > + register int i; > > + > > + lines = display_updatecpus(statics); > > + > > /* only do the rest if we need to */ > > if (lines > -1) > > { > > Index: contrib/top/top.X > > =================================================================== > > --- contrib/top/top.X (revision 223873) > > +++ contrib/top/top.X (working copy) > > @@ -205,6 +205,7 @@ > > .BR \-H , > > .BR \-I , > > .BR \-j , > > +.BR \-P , > > .BR \-S , > > .BR \-t , > > .BR \-u , > > @@ -314,6 +315,9 @@ > > .IR jail (8) > > ID. > > .TP > > +.B P > > +Toggle the display of per-CPU statistics. > > +.TP > > .B t > > Toggle the display of the > > .I top > > Index: usr.bin/top/machine.c > > =================================================================== > > --- usr.bin/top/machine.c (revision 223873) > > +++ usr.bin/top/machine.c (working copy) > > @@ -239,19 +239,48 @@ > > static void getsysctl(const char *name, void *ptr, size_t len); > > static int swapmode(int *retavail, int *retfree); > > > > +void > > +toggle_pcpustats(struct statics *statics) > > +{ > > + > > + if (ncpus == 1) > > + return; > > + > > + /* Adjust display based on ncpus */ > > + if (pcpu_stats) { > > + y_mem += ncpus - 1; /* 3 */ > > + y_swap += ncpus - 1; /* 4 */ > > + y_idlecursor += ncpus - 1; /* 5 */ > > + y_message += ncpus - 1; /* 5 */ > > + y_header += ncpus - 1; /* 6 */ > > + y_procs += ncpus - 1; /* 7 */ > > + Header_lines += ncpus - 1; /* 7 */ > > + statics->ncpus = ncpus; > > + } else { > > + y_mem = 3; > > + y_swap = 4; > > + y_idlecursor = 5; > > + y_message = 5; > > + y_header = 6; > > + y_procs = 7; > > + Header_lines = 7; > > + statics->ncpus = 1; > > + } > > +} > > + > > int > > machine_init(struct statics *statics, char do_unames) > > { > > - int pagesize; > > - size_t modelen; > > + int i, j, empty, pagesize; > > + size_t size; > > struct passwd *pw; > > > > - modelen = sizeof(smpmode); > > - if ((sysctlbyname("machdep.smp_active", &smpmode, &modelen, > > + size = sizeof(smpmode); > > + if ((sysctlbyname("machdep.smp_active", &smpmode, &size, > > NULL, 0) != 0 && > > - sysctlbyname("kern.smp.active", &smpmode, &modelen, > > + sysctlbyname("kern.smp.active", &smpmode, &size, > > NULL, 0) != 0) || > > - modelen != sizeof(smpmode)) > > + size != sizeof(smpmode)) > > smpmode = 0; > > > > if (do_unames) { > > @@ -299,52 +328,38 @@ > > statics->order_names = ordernames; > > #endif > > > > - /* Adjust display based on ncpus */ > > - if (pcpu_stats) { > > - int i, j, empty; > > - size_t size; > > - > > - cpumask = 0; > > - ncpus = 0; > > - GETSYSCTL("kern.smp.maxcpus", maxcpu); > > - size = sizeof(long) * maxcpu * CPUSTATES; > > - times = malloc(size); > > - if (times == NULL) > > - err(1, "malloc %zd bytes", size); > > - if (sysctlbyname("kern.cp_times", times, &size, NULL, 0) == -1) > > - err(1, "sysctlbyname kern.cp_times"); > > - pcpu_cp_time = calloc(1, size); > > - maxid = (size / CPUSTATES / sizeof(long)) - 1; > > - for (i = 0; i <= maxid; i++) { > > - empty = 1; > > - for (j = 0; empty && j < CPUSTATES; j++) { > > - if (times[i * CPUSTATES + j] != 0) > > - empty = 0; > > - } > > - if (!empty) { > > - cpumask |= (1ul << i); > > - ncpus++; > > - } > > + /* Allocate state for per-CPU stats. */ > > + cpumask = 0; > > + ncpus = 0; > > + GETSYSCTL("kern.smp.maxcpus", maxcpu); > > + size = sizeof(long) * maxcpu * CPUSTATES; > > + times = malloc(size); > > + if (times == NULL) > > + err(1, "malloc %zd bytes", size); > > + if (sysctlbyname("kern.cp_times", times, &size, NULL, 0) == -1) > > + err(1, "sysctlbyname kern.cp_times"); > > + pcpu_cp_time = calloc(1, size); > > + maxid = (size / CPUSTATES / sizeof(long)) - 1; > > + for (i = 0; i <= maxid; i++) { > > + empty = 1; > > + for (j = 0; empty && j < CPUSTATES; j++) { > > + if (times[i * CPUSTATES + j] != 0) > > + empty = 0; > > } > > - > > - if (ncpus > 1) { > > - y_mem += ncpus - 1; /* 3 */ > > - y_swap += ncpus - 1; /* 4 */ > > - y_idlecursor += ncpus - 1; /* 5 */ > > - y_message += ncpus - 1; /* 5 */ > > - y_header += ncpus - 1; /* 6 */ > > - y_procs += ncpus - 1; /* 7 */ > > - Header_lines += ncpus - 1; /* 7 */ > > + if (!empty) { > > + cpumask |= (1ul << i); > > + ncpus++; > > } > > - size = sizeof(long) * ncpus * CPUSTATES; > > - pcpu_cp_old = calloc(1, size); > > - pcpu_cp_diff = calloc(1, size); > > - pcpu_cpu_states = calloc(1, size); > > - statics->ncpus = ncpus; > > - } else { > > - statics->ncpus = 1; > > } > > + size = sizeof(long) * ncpus * CPUSTATES; > > + pcpu_cp_old = calloc(1, size); > > + pcpu_cp_diff = calloc(1, size); > > + pcpu_cpu_states = calloc(1, size); > > + statics->ncpus = 1; > > > > + if (pcpu_stats) > > + toggle_pcpustats(statics); > > + > > /* all done! */ > > return (0); > > } > > @@ -398,14 +413,11 @@ > > int i, j; > > size_t size; > > > > - /* get the cp_time array */ > > - if (pcpu_stats) { > > - size = (maxid + 1) * CPUSTATES * sizeof(long); > > - if (sysctlbyname("kern.cp_times", pcpu_cp_time, &size, NULL, 0) == -1) > > - err(1, "sysctlbyname kern.cp_times"); > > - } else { > > - GETSYSCTL("kern.cp_time", cp_time); > > - } > > + /* get the CPU stats */ > > + size = (maxid + 1) * CPUSTATES * sizeof(long); > > + if (sysctlbyname("kern.cp_times", pcpu_cp_time, &size, NULL, 0) == -1) > > + err(1, "sysctlbyname kern.cp_times"); > > + GETSYSCTL("kern.cp_time", cp_time); > > GETSYSCTL("vm.loadavg", sysload); > > GETSYSCTL("kern.lastpid", lastpid); > > > > @@ -413,21 +425,17 @@ > > for (i = 0; i < 3; i++) > > si->load_avg[i] = (double)sysload.ldavg[i] / sysload.fscale; > > > > - if (pcpu_stats) { > > - for (i = j = 0; i <= maxid; i++) { > > - if ((cpumask & (1ul << i)) == 0) > > - continue; > > - /* convert cp_time counts to percentages */ > > - percentages(CPUSTATES, &pcpu_cpu_states[j * CPUSTATES], > > - &pcpu_cp_time[j * CPUSTATES], > > - &pcpu_cp_old[j * CPUSTATES], > > - &pcpu_cp_diff[j * CPUSTATES]); > > - j++; > > - } > > - } else { > > - /* convert cp_time counts to percentages */ > > - percentages(CPUSTATES, cpu_states, cp_time, cp_old, cp_diff); > > + /* convert cp_time counts to percentages */ > > + for (i = j = 0; i <= maxid; i++) { > > + if ((cpumask & (1ul << i)) == 0) > > + continue; > > + percentages(CPUSTATES, &pcpu_cpu_states[j * CPUSTATES], > > + &pcpu_cp_time[j * CPUSTATES], > > + &pcpu_cp_old[j * CPUSTATES], > > + &pcpu_cp_diff[j * CPUSTATES]); > > + j++; > > } > > + percentages(CPUSTATES, cpu_states, cp_time, cp_old, cp_diff); > > > > /* sum memory & swap statistics */ > > { > > > > -- > > John Baldwin