This is part of !118 where @tt.rantala found a memory leak.
The other part of !118 may come later if the performance change
is significant.
References:
procps-ng/procps!118
With glibc, each time the strftime() function is used (twice per process
in a typical ps -fe run), a stat("/etc/localtime") system call is used
to determine the timezone. Not only does this add extra system call
overhead, but when multiple ps processes are trying to access this
file (or multiple glibc programs using strftime) in parallel, this can
trigger significant lock contention within the OS kernel.
Since ps is not intended to run for long periods of time as a
daemon (during which the system timezone could be altered and PS might
reasonably be expected to adapt its output), there is no benefit to
repeatedly doing this stat(). To stop this behavior, explicitly set the
TZ variable to its default value (:/etc/localtime) whenever it is unset.
glibc will then cache the stat() result.
While sysctl did change the order of /run and /etc to match
systemd in the referenced commit, the Debian bug report that
brought it to light was not documented.
References:
commit 24a1574f0ahttps://bugs.debian.org/950788
So as to not obscure the results from this new target,
we'll redirect that final 'make clean' output to null.
Signed-off-by: Jim Warner <james.warner@comcast.net>
If a hash results report is output (via UNREF_RPTHASH)
a portion is devoted to occupied table entries ordered
by depth. There is a possibility that some depths will
not be found among existing occupied table entries and
to avoid any confusion probably should not be printed.
[ to illustrate the potential for confusion prior to ]
[ this patch, force a very small table size (like 8) ]
[ & then trigger the procps_pids_unref() eoj report. ]
So this patch ensures only 'in use' entries are shown.
[ admittedly, all of the remaining logic in the loop ]
[ could/should be subordinate to this new 'if' test, ]
[ but we will keep the change to a minimum. besides, ]
[ there's no harm subtracting/adding a zero numdepth ]
[ especially since the chance of a zero is very low. ]
Signed-off-by: Jim Warner <james.warner@comcast.net>
free, slabtop and uptime would happily take extra command line
arguments and doing nothing about them. The programs now check
optind after option processing and will give you usage screen
if there is anything extra.
References:
procps-ng/procps#181
For long lines from a process, watch would wrap them around to the
next. While this default option has it uses, sometimes you want to
just cut those long lines down.
watch has a -w flag which will truncate the lines to the number
of columns. A few simple lines to do this new trick.
I think I caught all the ANSI state correctly but there might be
a chance it bleeds to the next row.
References:
procps-ng/procps#182
noinst_PROGRAMS are built with "make" even though we had the
test programs in there and only needed them for "make check".
In theory the check target should depend on check_PROGRAMS as
check-am target does and the document states it should, but for
reasons understood by the automake whisperers only, it doesn't
build them.
check only depends on BUILT_SOURCES for some reason.
check-am: all-am
$(MAKE) $(AM_MAKEFLAGS) $(check_PROGRAMS)
$(MAKE) $(AM_MAKEFLAGS) check-TESTS
check: $(BUILT_SOURCES)
$(MAKE) $(AM_MAKEFLAGS) check-recursive
References:
https://www.gnu.org/software/automake/manual/html_node/Scripts_002dbased-Testsuites.html
The referenced commits created the library infrastructure and test
program to validate that the structures and macros line up with
each other.
The library needs to be (re)built with -DITEMTABLE_DEBUG and then
the test program ran. We clean before and after so we are not
testing a non-debug library or having a debug library hanging around
to cause future problems.
Due to test_Itemtables depending on the library, we don't need to
explicitly build the library.
To validate the library structure/header corrospondence run:
make check-lib
References:
commit e616409aa4
commit 92d0297e1ehttps://www.freelists.org/post/procps/keep-on-patchin,19
This patch just raises the size of the hash table used
to calculate elapsed task stuff. The net result should
be less need for 'chaining' under pid hash collisions.
[ the hash scheme is intentionally kept as primitive ]
[ and, therefore, as fast as possible. it employs an ]
[ 'and' approach versus a 'mod' operation since both ]
[ yield similar distribution but the former approach ]
[ was 4 fewer cpu instructions in terms of overhead. ]
[ additionally, for hash collisions, 'chaining' uses ]
[ an array index rather than the usual pointer since ]
[ the HST_t guys may move when they are reallocated. ]
Signed-off-by: Jim Warner <james.warner@comcast.net>
This patch separates the memory allocations into those
used initially from those used in later reallocations.
Thus, we can reduce that iterative realloc() overhead.
Additionally, we'll correct a long standing oops where
multiple history_info structures were created at 'new'
time when only one should have been allocated (jeeze).
[ originally the allocation was strangely based upon ]
[ number of 'items' (???) & later a #define constant ]
Reference(s):
. May, 2016 - subsequent bad history_info logic
commit 9ebadc1438
. Aug, 2015 - original faulty history_info code
commit 7e6a371d8a
Signed-off-by: Jim Warner <james.warner@comcast.net>
The referenced commit the comm length was increased from 16 to 64
characters to handle the larger command names for things like kernel
threads.
However most user processes are limited to 15 characters which means
if you try something like ps -C myprogramisbiggerthansixteen this would
fail to match because /proc/<PID>/comm would only be myprogramisbigg
ps now checks the comm length and if it is 15 and if the given match
is 15 or more, it will only match the first 15 characters.
This is also how killall has worked for about a year.
Thanks to Jean Delvare <jdelvare@suse.de> for the note.
Copy of commit from master.
References:
commit 14005a371e
commit psmisc/psmisc@1188315cd0
commit 3e1c00d051
Signed-off-by: Craig Small <csmall@dropbear.xyz>
This patch is an outgrowth of that commit shown below.
Many additional potential segmentation faults might be
encountered if interactive commands are opened up to a
user when a '-p' switch has a single non-existent pid.
[ always the 'k', 'L', 'r', 'Y' keys & maybe 'v' too ]
So, this patch will restrict such a loser (oops, user)
to a reduced subset of normal commands until he/she/it
quits then restarts top with something to be displayed
or issues the '=' command overriding that '-p' switch.
Reference(s):
commit d3203d99dd
Signed-off-by: Jim Warner <james.warner@comcast.net>
I'm not sure why, but the make check will now fail for vmstat
Running ./vmstat.test/vmstat.exp ...
FAIL: vmstat disk information (-d option)
With the _new function returning the error.
In vmstat all other structures are set to NULL before calling _new
except the diskstat ones. This has been corrected.
This patch fixes a nearly decade old bug discovered by
Frederik Deweerdt. His merge request shown below would
be an adequate solution except for iterative overhead.
This alternate patch will represent substantially less
overhead for an admittedly extremely rare possibility.
Reference(s):
https://gitlab.com/procps-ng/procps/-/merge_requests/114
And-thanks-to: Frederik Deweerdt <fdeweerdt@fastly.com>
Signed-off-by: Jim Warner <james.warner@comcast.net>
What had been stated as the contents of the cpu graphs
was never really inclusive enough. Those recent newlib
tweaks highlighted the need for these man doc changes.
Signed-off-by: Jim Warner <james.warner@comcast.net>
This patch may be a little misleading in terms of size
since most of the changes just reorder a little logic.
The most significant changes involve two GUEST values.
My original implementation excluded such tics from the
TOTAL calculation and, therefore, the BUSY figure too.
That decision was erroneously based on some code found
in ./kernel/sched/cputime.c which in hindsight applies
only to processes, not those system level cpu figures.
[ another likely oops classified STOLEN tics as IDLE ]
So, this patch attempts to bring those SUM values into
better agreement with the calculations performed for a
root cgroup (see ./kernel/cgroup/rstat.c source file).
[ we differ from those above in that we also include ]
[ the IDLE plus IOWAIT tics in our TOTAL calculation ]
Signed-off-by: Jim Warner <james.warner@comcast.net>
There's a huge toe-stubber awaiting future maintenance
as reflected in that commit below which deals with the
addition of new enumerators to the Item_table. Namely,
whenever the table is grown, one must remember to also
change that existing 'logical_end' enumerator's value.
Well, not anymore! Since that MAXTABLE macro was added
to the procps-private.h header we can now also exploit
it so a 'logical_end' automatically tracks table size.
This change also renders some code associated with the
ITEMTABLE_DEBUG #define unnecessary. So it's gone too.
Reference(s):
. 08/2016, add new enumerators
commit 09e1886c9e
. 08/2020, added MAXTABLE macro
commit c865b06c30
. 08/2020, introduced ITEMTABLE_DEBUG
commit 92d0297e1e
Signed-off-by: Jim Warner <james.warner@comcast.net>
[ this is what happens when i've run out of bugs and ]
[ issues and am now left with way too much free time ]
Signed-off-by: Jim Warner <james.warner@comcast.net>
All of these changes are self-explanatory, so I'll not
provide any more comments thus avoiding further shame.
[ everything's perfectly justified and right margins ]
[ are filled entirely, but of course it must be luck ]
Signed-off-by: Jim Warner <james.warner@comcast.net>
At 'new' time, the major API modules each ensure their
'info' parameter isn't NULL but what it pointed to was
except this single straggler, for some unknown reason.
So, this patch brings him into line with those others.
[ And, without going into the ugly details, this was ]
[ the reason I never experienced an abend originally ]
[ but Craig did. And, though related to stacks mgmt, ]
[ zero initialization was not a factor. Anyway, with ]
[ this patch, everybody would have experienced abend ]
[ under the original (faulty) test_Itemtables logic! ]
Reference(s):
https://www.freelists.org/post/procps/keep-on-patchin,13
Signed-off-by: Jim Warner <james.warner@comcast.net>
Me thinks Craig had the right idea but perhaps not the
most correct solution. As currently structured, all of
the tests now go way too far by checking every 'unref'
and 'new' call when what we're trying for is survival.
In the final analysis, it doesn't matter who issues an
EXIT_FAILURE - that run_tests guy or an early Exit out
of a procps_new() function. They both will produce the
same end result of the desired "FAIL" test diagnostic.
[ and this patch once again allows the slabinfo test ]
Reference(s):
commit 4eeed6dcff
Signed-off-by: Jim Warner <james.warner@comcast.net>
The referenced commit introduced a test program for the API
but it would often fail due to:
The given pointer for _new() not being NULL
The return value for _new not checked, so the subsequent _unref()
would free() random memory
slabinfo checks failing due to permission denied errors.
The first two are fixed, as well as returning a fail to the test
if they don't return correctly, with slabinfo waiting to see if there
is a way of initialising the structure without reading the slabinfo.
References:
commit e616409aa4
Signed-off-by: Craig Small <csmall@dropbear.xyz>
