Date: Sat, 9 Jul 2011 08:30:47 +0000 From: Alexander Best <arundel@freebsd.org> To: John Baldwin <jhb@freebsd.org> Cc: current@freebsd.org, edwin@freebsd.org Subject: Re: [PATCH] Make top -P an interactive toggle Message-ID: <20110709083047.GA86927@freebsd.org> In-Reply-To: <20110708215051.GA14098@freebsd.org> References: <201107081511.43417.jhb@freebsd.org> <20110708215051.GA14098@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20110709083047.GA86927>