Re: [PATCH] Make top -P an interactive toggle

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)
> > _at__at_ -196,9 +196,9 _at__at_
> >      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
> > _at__at_ -225,8 +225,9 _at__at_
> >  #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 */
> > _at__at_ -411,7 +412,7 _at__at_
> >  		break;
> >  
> >  	      case 'P':
> > -		pcpu_stats = Yes;
> > +		pcpu_stats = !pcpu_stats;
> >  		break;
> >  
> >  	      case 'z':
> > _at__at_ -1088,6 +1089,12 _at__at_
> >  				    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)
> > _at__at_ -151,16 +151,14 _at__at_
> >      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;
> > _at__at_ -170,6 +168,21 _at__at_
> >      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)
> > _at__at_ -205,6 +205,7 _at__at_
> >  .BR \-H ,
> >  .BR \-I ,
> >  .BR \-j ,
> > +.BR \-P ,
> >  .BR \-S ,
> >  .BR \-t ,
> >  .BR \-u ,
> > _at__at_ -314,6 +315,9 _at__at_
> >  .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)
> > _at__at_ -239,19 +239,48 _at__at_
> >  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) {
> > _at__at_ -299,52 +328,38 _at__at_
> >  	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);
> >  }
> > _at__at_ -398,14 +413,11 _at__at_
> >  	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);
> >  
> > _at__at_ -413,21 +425,17 _at__at_
> >  	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