top: improve logic surrounding 'smp_num_cpus' variable

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 <james.warner@comcast.net>
With-thanks-to: Guido Jäkel <G.Jaekel@DNB.DE>
This commit is contained in:
Jim Warner 2019-01-11 00:00:00 -06:00 committed by Craig Small
parent f67127e728
commit 893793a8c4

View File

@ -2817,7 +2817,6 @@ static void sysinfo_refresh (int forced) {
meminfo(); meminfo();
#ifndef PRETEND8CPUS #ifndef PRETEND8CPUS
cpuinfo(); cpuinfo();
Cpu_faux_tot = smp_num_cpus;
#endif #endif
Numa_node_tot = numa_max_node() + 1; Numa_node_tot = numa_max_node() + 1;
sav_secs = cur_secs; sav_secs = cur_secs;
@ -3637,10 +3636,10 @@ static void before (char *me) {
xalloc_err_handler = xalloc_our_handler; xalloc_err_handler = xalloc_our_handler;
// establish cpu particulars // establish cpu particulars
cpuinfo();
#ifdef PRETEND8CPUS #ifdef PRETEND8CPUS
smp_num_cpus = 8; smp_num_cpus = 8;
#endif #endif
Cpu_faux_tot = smp_num_cpus;
Cpu_States_fmts = N_unq(STATE_lin2x4_fmt); Cpu_States_fmts = N_unq(STATE_lin2x4_fmt);
if (linux_version_code > LINUX_VERSION(2, 5, 41)) if (linux_version_code > LINUX_VERSION(2, 5, 41))
Cpu_States_fmts = N_unq(STATE_lin2x5_fmt); Cpu_States_fmts = N_unq(STATE_lin2x5_fmt);
@ -5124,7 +5123,7 @@ static void keys_global (int ch) {
Pseudo_row = PROC_XTRA; Pseudo_row = PROC_XTRA;
break; break;
case 'I': case 'I':
if (Cpu_faux_tot > 1) { if (smp_num_cpus > 1) {
Rc.mode_irixps = !Rc.mode_irixps; Rc.mode_irixps = !Rc.mode_irixps;
show_msg(fmtmk(N_fmt(IRIX_curmode_fmt) show_msg(fmtmk(N_fmt(IRIX_curmode_fmt)
, Rc.mode_irixps ? N_txt(ON_word_only_txt) : N_txt(OFF_one_word_txt))); , 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: numa_nope:
if (CHKw(w, View_CPUSUM)) { if (CHKw(w, View_CPUSUM)) {
// display just the 1st /proc/stat line // 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; Msg_row += 1;
} else { } else {