From owner-freebsd-current@FreeBSD.ORG Sat Jul 9 09:44:16 2011 Return-Path: Delivered-To: current@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 1233) id 6DC3F1065672; Sat, 9 Jul 2011 09:44:16 +0000 (UTC) Date: Sat, 9 Jul 2011 09:44:16 +0000 From: Alexander Best To: John Baldwin Message-ID: <20110709094416.GA95093@freebsd.org> References: <201107081511.43417.jhb@freebsd.org> <20110708215051.GA14098@freebsd.org> <20110709083047.GA86927@freebsd.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20110709083047.GA86927@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 09:44:16 -0000 On Sat Jul 9 11, Alexander Best wrote: > 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. would it be possible to display a note when using 'P'? E.g. when pressing 'z' top informs the user with one of these messages: "Displaying system idle process." or "Not displaying system idle process." would be nice to have something similar when pressing the 'P' key. Maybe: "Display per-cpu CPU usage statistics." and "Not display per-cpu CPU usage statistics." cheers. alex > > > > > 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