From 893793a8c493338be04fc78a2dd0cf9f719e9779 Mon Sep 17 00:00:00 2001 From: Jim Warner Date: Fri, 11 Jan 2019 00:00:00 -0600 Subject: [PATCH] top: improve logic surrounding 'smp_num_cpus' variable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I thank Guido Jäkel for raising the issue cited in the merge request referenced below. While restoring 1 line of code would produce the desired results, it does not address the root cause of that problem he experienced. The variable 'smp_num_cpus' was set by libprocps via a sysconf(_SC_NPROCESSORS_ONLN) call. It was supposed to represent total number of processors currently online. It also served as the position in the Cpu_tics[] array where the /proc/stat line #1 (cpu summary) was stored. The variable 'Cpu_faux_tot' was valued by top based on total individual cpus parsed from the /proc/stat file. It serves as a fence post for Cpu_tics[] array access. The problem Guido experienced results from a disparity between those 2 variables, plus one instance where the wrong variable was used in the summary_show() routine. . Here is the real culprit, the actual incorrect code: . summary_hlp(&Cpu_tics[Cpu_faux_tot], N_txt(WORD_a... Which always should have been represented in this way: . summary_hlp(&Cpu_tics[smp_num_cpus], N_txt(WORD_a... ------------------------------------------------------ The above 'disparity' might arise in any system when a cpu is taken offline since there's a 3 second delay in cpu and memory refreshes in an effort to reduce costs. Usually this particular condition will be short lived. However, there is a more persistent problem under lxc. If a host cpu is taken offline and then brought online again, within the container sysconf returns the proper number of online processors. But, /proc/stat does not! Sadly, I've yet to find a way to coax a container into refreshing its /proc/stat, short of reboting the host. [ might that represent a potential bug in lxc logic? ] Reference(s): https://gitlab.com/procps-ng/procps/merge_requests/82 Signed-off-by: Jim Warner With-thanks-to: Guido Jäkel --- top/top.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/top/top.c b/top/top.c index b031188e..1e8ace09 100644 --- a/top/top.c +++ b/top/top.c @@ -2817,7 +2817,6 @@ static void sysinfo_refresh (int forced) { meminfo(); #ifndef PRETEND8CPUS cpuinfo(); - Cpu_faux_tot = smp_num_cpus; #endif Numa_node_tot = numa_max_node() + 1; sav_secs = cur_secs; @@ -3637,10 +3636,10 @@ static void before (char *me) { xalloc_err_handler = xalloc_our_handler; // establish cpu particulars + cpuinfo(); #ifdef PRETEND8CPUS smp_num_cpus = 8; #endif - Cpu_faux_tot = smp_num_cpus; Cpu_States_fmts = N_unq(STATE_lin2x4_fmt); if (linux_version_code > LINUX_VERSION(2, 5, 41)) Cpu_States_fmts = N_unq(STATE_lin2x5_fmt); @@ -5124,7 +5123,7 @@ static void keys_global (int ch) { Pseudo_row = PROC_XTRA; break; case 'I': - if (Cpu_faux_tot > 1) { + if (smp_num_cpus > 1) { Rc.mode_irixps = !Rc.mode_irixps; show_msg(fmtmk(N_fmt(IRIX_curmode_fmt) , Rc.mode_irixps ? N_txt(ON_word_only_txt) : N_txt(OFF_one_word_txt))); @@ -5855,7 +5854,7 @@ static void summary_show (void) { numa_nope: if (CHKw(w, View_CPUSUM)) { // display just the 1st /proc/stat line - summary_hlp(&Cpu_tics[Cpu_faux_tot], N_txt(WORD_allcpus_txt)); + summary_hlp(&Cpu_tics[smp_num_cpus], N_txt(WORD_allcpus_txt)); Msg_row += 1; } else {