Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

atop.daily: removed dependency from procps #7

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

atop.daily: removed dependency from procps #7

wants to merge 1 commit into from

Conversation

fcolista
Copy link

@fcolista fcolista commented Sep 6, 2017

This patch allows atop to not work only with procps, but with busybox ps too.
Distribution with small amount of packages and embedded systems can benefit from this.
Thanks for considering.
.:Francesco Colista

@Atoptool
Copy link
Owner

Development is certainly not frozen.

@Atoptool
Copy link
Owner

In my opinion, the proposed modification does not work properly any more with procps ps since no flags are passed (ps does not show the daemon processes, like atop).
I propose another modification that does not depend on ps any more:

  if  [ -e $PIDFILE ]  &&  grep  -q  'atop$'  /proc/`cat $PIDFILE`/comm

Can you confirm that this works with busybox?

@fcolista
Copy link
Author

It works, but after thinking a little bit more, I think that it's better to use "pidof". This is implemented both in procps and in busybox too:

if [ -e $PIDFILE ] && pidof atop|awk '{print $1}';

Francesco

@Atoptool
Copy link
Owner

If more atop processes are active (e.g. the daemon running in the background and a couple of interactive atop processes in the foreground), pidof shows more that one PID. You still don't know which PID is the one related to the background atop (the PID that should be compared to the PID found in the pid file).

Gerlof

zlandau added a commit to zlandau/atop that referenced this pull request Feb 16, 2020
The load average reporting functions in showsys.c use static buffer
sizes. When the load averages on a machine are very large, this causes
the writes to extend past the buffer. With this commit, if a number is
too large then we just show '>NNNNNN'. I'm not sure if this is the best
choice, so I'm open to other ideas.

This is what the output looks like when we exceed the maximums:

  CPL | avg1 >999999 | avg5 >999999 | avg15 >99999 | csw 103117e3 | intr 88296e3 |

Note that this was triggered from a kernel that is reporting clearly
inaccurate numbers:

  $ cat /proc/loadavg
  1.25 2.40 368567045.47 1/589 53576

But regardless, crashing is no fun.

For future reference, I narrowed down the issue by building with
-fsanitize=address. For example:

  ==55396==ERROR: AddressSanitizer: global-buffer-overflow on address 0x55c527d6f14f at pc 0x7f4729942df9 bp 0x7ffe1fd30710 sp 0x7ffe1fd2fea0
  WRITE of size 10 at 0x55c527d6f14f thread T0
      #0 0x7f4729942df8 in __interceptor_vsprintf /build/gcc/src/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1627
      Atoptool#1 0x7f47299432cf in __interceptor_sprintf /build/gcc/src/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1670
      Atoptool#2 0x55c527d1c167 in sysprt_CPLAVG15 (/home/kapheine/projects/atop/atop+0x74167)
      Atoptool#3 0x55c527d2087a in showsysline (/home/kapheine/projects/atop/atop+0x7887a)
      Atoptool#4 0x55c527d1471f in prisyst (/home/kapheine/projects/atop/atop+0x6c71f)
      Atoptool#5 0x55c527d090ff in generic_samp (/home/kapheine/projects/atop/atop+0x610ff)
      Atoptool#6 0x55c527ce17c9 in main (/home/kapheine/projects/atop/atop+0x397c9)
      Atoptool#7 0x7f472952a022 in __libc_start_main (/usr/lib/libc.so.6+0x27022)
      Atoptool#8 0x55c527ce266d in _start (/home/kapheine/projects/atop/atop+0x3a66d)

  0x55c527d6f14f is located 49 bytes to the left of global variable 'buf' defined in 'showsys.c:981:54' (0x55c527d6f180) of size 15
  0x55c527d6f14f is located 0 bytes to the right of global variable 'buf' defined in 'showsys.c:1000:54' (0x55c527d6f140) of size 15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants