]> granicus.if.org Git - procps-ng/commitdiff
top: improve logic surrounding 'smp_num_cpus' variable
authorJim Warner <james.warner@comcast.net>
Fri, 11 Jan 2019 06:00:00 +0000 (00:00 -0600)
committerCraig Small <csmall@enc.com.au>
Tue, 22 Jan 2019 11:46:53 +0000 (22:46 +1100)
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>
top/top.c

index b031188e801d38be8ca7be65cd3a23f8f5273e75..1e8ace0993dfd168fc093d117231bb3eb3179275 100644 (file)
--- 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 {