Original report:
When trying kill a process with insufficient privileges (see blow),
pkill displays the error message “... failed: Operation not permitted”,
but returns 0. Surely it should return 3?
$ pkill syslogd ; echo $?
pkill: killing pid 373 failed: Operation not permitted
0
Return value 0 means one of more things matched. For a pgrep (which
shares code with pkill) this makes sense, there was a match. It seems
wrong for pkill to return 0 when it in fact could not do what you told
it to. However return value 3 means a fatal error and it's not fatal.
Looking at other programs when trying to kill things it cannot kill.
shell kill returns 1, procps kill returns 1, killall returns 1, skill
returns 0 (and says it was successful!, ah well poor old skill)
The consensus seems to be that you return 1 if you cannot kill it, even
if you found it. In other words the return value for both not found and
not able to kill it is the same.
pkill only returns 0 if something was killed. This means we found a
match AND the kill() system call worked too.
References:
https://bugs.debian.org/852758
Signed-off-by: Craig Small <csmall@enc.com.au>
This patch adapts a master branch commit to our newlib
branch. Shown below was that original commit msg text.
------------------------------------------------------
top: Add unobtrusive XDG support
By default the file HOME/.toprc will be prefered. This ensures there
should be minimal breakage even if this file is later created by some
other means. Otherwise we will follow the new behaviour described by
the XDG Base Directory Specification:
If the XDG_CONFIG_HOME environment variable is available we will attempt
to use this as XDG_CONFIG_HOME/procps/toprc otherwise we will fall-back
to HOME/.config/procps/toprc instead.
Signed-off-by: Earnestly <zibeon@gmail.com>
------------------------------------------------------
Reference(s):
. master branch original
commit 0a0f7d60e309c13c8a399bc2187bed6e3e156b43
. discussion thread
https://gitlab.com/procps-ng/procps/merge_requests/38
Signed-off-by: Jim Warner <james.warner@comcast.net>
This avoids situations where longer regex which matches short-named proc
is used. Test for pgrep updated.
This is the newlib update of 5d12be1b7e8cc690a4d8778754aae5db4c07db2b
Signed-off-by: Craig Small <csmall@enc.com.au>
Since its introduction, our evolved /proc/stat API has
relied on a static buffer of 8192 bytes. This approach
is probably Ok for other /proc files but it would only
accommodate around 100 processors. If such a threshold
were exceeded then this interface could never succeed.
Now days 100 processors doesn't seem at all excessive.
So this commit trades that static buffer for a dynamic
self-tuning one. And since so much former top CPU code
was already rolled into this module, we just stole the
already proven top dynamic buffer management code too.
[ this also meant switching low level unbuffered I/O ]
[ calls to standard library buffered I/O calls. that ]
[ is exactly what <slabinfo> and <diskstats> employ. ]
Reference(s):
. 1st gen readstat introduction
commit a410e236ab
Signed-off-by: Jim Warner <james.warner@comcast.net>
With the addition of those new derived SUM values, any
CPUs taken offline or brought online would distort the
historical (delta) results. So this patch just forces
a history reset when such transitions are encountered.
Reference(s):
. derived SUM provisions introduced
commit 2c86c4984a
Signed-off-by: Jim Warner <james.warner@comcast.net>
For each of those interfaces employing a priming read,
all the other 'read' functions begin with the module's
name except this guy which began with 'read_slabinfo'.
Now, they'll all begin with their module name then end
the same with a '_read_failed' boolean hinting suffix.
Signed-off-by: Jim Warner <james.warner@comcast.net>
Upon reflection, at the point where the 'priming read'
was introduced, any possibility of history distortions
was also eliminated. This was true because all of the
'old' (zeroed) data will have been replaced with 'new'
data whenever a user finally calls get, select & reap.
Thus, any DELTA values will automatically reflect that
interval between 'new' and subsequent retrieval calls.
[ diskstats didn't actually employ a 1st time switch ]
[ like the others so we have changed a comment only. ]
[ but that module will retain something similar used ]
[ inside node_update whenever a new node is created. ]
Reference(s):
. priming read added to slabinfo
commit 5d5a52a380
. priming read added to diskstats
commit ecd64f4445
. priming read added to meminfo, stat, vmstat
commit 1a2b62c779
Signed-off-by: Jim Warner <james.warner@comcast.net>
This commit just exploits those new library provisions
for tic categories, introduced in the preceding patch,
which had been prompted by the issue referenced below.
[ ok it also corrects the top graph for system usage ]
[ since this turkey failed to include tics for these ]
[ two interrupts: STAT_TIC_IRQ and STAT_TIC_SOFTIRQ. ]
Reference(s):
https://gitlab.com/procps-ng/procps/issues/48
Signed-off-by: Jim Warner <james.warner@comcast.net>
This commit arose out of the discussion (and research)
surrounding the issue cited below. It is an attempt to
consolidate and standardize the calculation of jiffies
categories (e.g. 'idle', 'busy', etc.) once & for all.
Also included is the enum STAT_TIC_NUM_CONTRIBUTORS in
case anyone, in the future, decides to calculate usage
based upon elapsed time * Hz (like top does in process
level %CPU stats). In such an event, a total number of
CPUs or NUMA Nodes would be needed for proper scaling.
Reference(s):
https://gitlab.com/procps-ng/procps/issues/48
Signed-off-by: Jim Warner <james.warner@comcast.net>
Unlike the ps kludge under the master branch to ensure
that namespaces appear the same under both 32 & 64-bit
models, this newlib branch already used a proper type.
However source data still carried the original type as
'signed long' versus that more proper 'unsigned long'.
So, this patch makes sources & destinations identical.
Reference(s):
. master branch ps kludge
commit c41c614b0c
Signed-off-by: Jim Warner <james.warner@comcast.net>
Aw shucks, not all support for this defunct enumerator
was removed via the commits shown below (but, is now).
[ what remained were just variables named after that ]
[ deprecated/deleted enumerator, but still & all ... ]
[ plus, i have left the doc/libproc.3 file untouched ]
[ since it already appears badly out of date anyway! ]
Reference(s):
. ps references partially purged
commit 66c4024d75
. enumerator purged from library
commit 912075605b
Signed-off-by: Jim Warner <james.warner@comcast.net>
This commit will resolve the RedHat Bugzilla #1322111.
[ import from identical commit against master branch ]
[ but without trailing whitespace, thank you so much ]
Imported by: Jim Warner <james.warner@comcast.net>
While a Debian bug report referenced below was limited
to the 'eip' and 'esp' fields, this patch also extends
address width adaptations to some other addresses too.
[ and, we do so in a far less invasive manner than a ]
[ redhat approach shown below adding two new fields! ]
Reference(s):
. new debian bug report
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=846361
. old redhat solution
https://bugzilla.redhat.com/show_bug.cgi?id=244152
Signed-off-by: Jim Warner <james.warner@comcast.net>
In each module employing a priming read at 'new' time,
should that read fail, a call to 'unref' will be made.
However, there is a hidden dependency that these calls
must never occur before the context 'refcount' was set
due to the way an 'unref' conditional was constructed.
So this commit just ensures that 'unref' will function
as expected, even if called with a 'refcount' of zero.
Signed-off-by: Jim Warner <james.warner@comcast.net>
In the commit referenced below, in addition to several
tweaks to comments, 3 fields were no longer assured of
being present in the results stacks. However, 2 of the
3 fields might, in fact, be required even if they were
not currently being displayed in any of the 4 windows.
The PIDS_CMD is used in two separate 'Inspect' headers
('Y' command) and the PIDS_ID_EUID is required if that
'User Filter' ('u' or 'U' command) was being employed.
That latter field's inclusion will be made conditional
but the former field must be unconditionally included.
( for old top, PIDS_CMD would have always been there )
Reference(s):
commit 4e4debda9b
Signed-off-by: Jim Warner <james.warner@comcast.net>
No longer will ps print nwchan as 'ffffff', '-' or '1'
since the proc/PID/stat wchan field didn't represent a
real address anyway. Rather, the field will henceforth
output a dash ('-'), the ps customary 'not available'.
That man document was also tweaked to better represent
actual behavior. An asterisk ('*') was never shown for
threaded tasks and that dash ('-') usually didn't mean
running tasks (sometimes associated with permissions).
Signed-off-by: Jim Warner <james.warner@comcast.net>
Removing the Item_table 'stat' oldflags for WCHAN_ADDR
was wrong since that 'stat' field is not a constant 0.
Rather, it could assume these 3 values: -1, 0, and +1.
I have not been able to pin down a '-1' result, but it
probably means some sort of permission error (-EPERM).
The '1' or '0' values were supposed to distinguish the
tasks that were or were not blocked (whether there was
a wchan address). However, in practice there is little
correlation between those values and availability of a
kernel symbol in /proc/$$/wchan (perhaps due to race).
Anyway, the real point is that a 'stat' wchan does not
now intentionally contain an address. Thus, outputting
'ffffff', '-' or '1' in programs like ps is senseless.
So this patch just eliminates PIDS_WCHAN_ADDR from our
item enumerators leaving only the PIDS_WCHAN_NAME guy.
Now the new library can't be blamed for bad addresses!
Reference(s):
. removed Item_table 'oldflags'
commit c4aa6c0ab4
. linux removal of wchan addresses
commit b2f73922d119686323f14fbbe46587f863852328
Signed-off-by: Jim Warner <james.warner@comcast.net>
It makes no sense to begin our tracked nested level at
'1' then later require a '1' to be subtracted from the
level as artwork and indentation is added for display.
By beginning such tracked levels at zero, we can avoid
the need to adjust it & use it directly in a snprintf.
Signed-off-by: Jim Warner <james.warner@comcast.net>
It would have been nice to remove this 'nDRT' guy from
the fields management screen and man document as well.
But, while this version of top could be made to handle
an older rcfile, the reverse would not have been true.
Besides, it's been zero for so long already we'll just
include a 'deprecated' note in top's man page for now.
[ the 'nTH ' field number was corrected in there too ]
Signed-off-by: Jim Warner <james.warner@comcast.net>
It seems inappropriate to blindly include fields known
to always be zero in our brand new library. Therefore,
this patch removes support for three such enumerators.
[ that stat 'it_real_value' (PIDS_ALARM) field could ]
[ have been made obsolete before a linux 2.6 release ]
Signed-off-by: Jim Warner <james.warner@comcast.net>
This commit just contains some tweaks to comments plus
a few adjustments to whitespace for alignment purposes
and a normalization of the header inclusion #define's.
[ plus a spelling error in one header file was fixed ]
Signed-off-by: Jim Warner <james.warner@comcast.net>
When this module was upgraded to 3rd generation in the
patch referenced below, numa node support was migrated
from the top program into newlib. The 'guest_nice' and
'guest' tics were overlooked as top did not need them.
So, this commit corrects that oversight and achieves a
proper symmetry between the cpu & numa jiffies counts.
Reference(s):
. 3rd gen redesign, numa support imported
commit abc71a46ad
Signed-off-by: Jim Warner <james.warner@comcast.net>
Maybe some folks still need a few .gitconfig tweaks to
catch the trailing whitespace errors a little earlier.
Or, at the least, after a local commit they should do:
$ git diff HEAD~1
[ and then check if git marks any with his red blobs ]
Signed-off-by: Jim Warner <james.warner@comcast.net>
A priming read at 'new' time in that <slabinfo> module
was important so that permission problems are detected
early. Plus, it also had the potential of making delta
values valid when 'get' or 'select' were first called.
It is for that latter reason that such a read was also
incorporated in the <diskstats> module 'new' function.
No other module, however, employed such priming reads.
This patch just brings those potential benefits to all
of our other newlib modules with the exception of that
<pids> guy. That module is, of necessity, sufficiently
different from those others to justify such exclusion.
Not only are there precious few DELTA enums in <pids>,
but the costs of a priming read would be much greater.
[ otherwise, these newly added priming reads have no ]
[ measurable negative impact on performance/timings. ]
Signed-off-by: Jim Warner <james.warner@comcast.net>
The <slabinfo> header provides 3 groups of enumerators
with prefixes of SLABINFO, SLABS & SLABNODE. The first
is strictly user oriented & isn't supported internally
by any structure. The other two, however, have structs
associated with 'em but, unfortunately, 1 is misnamed.
The 'struct slabs_node' is associated with 'nodes' and
supports the enumerators with the SLABNODE prefix. But
the 'struct slabs_hist' was associated with 'hist' yet
supports those enumerators with just the SLABS prefix.
We do not care very much what some structure is called
but we do care about an identifier used manipulate it.
This patch will trade the 'hist' identifier associated
with 'struct slabs_hist' for a more congruous 'slabs'.
[ it's awful when the author can't remember what the ]
[ true meaning of an identifier is after creating it ]
Signed-off-by: Jim Warner <james.warner@comcast.net>
If, in fact, numa nodes are dynamic (like that current
total of on-line cpus) the existing logic was lacking.
It included an early return before checking the total.
So, this commit ensures that the nodes total is always
set or updated consistently in only a single function.
There's no need to set it at the time 'new' is called.
[ and since under our existing code this nodes total ]
[ could never possibly have been zero, the erroneous ]
[ test (with the early return) has now been whacked! ]
Signed-off-by: Jim Warner <james.warner@comcast.net>
Since our library is responsible for NUMA support, and
since the top program already accommodates the lack of
NUMA data, there's no reason that #define NUMA_DISABLE
need be explicitly referenced in the top source files.
Ergo, this commit just eliminates all such references.
Now, top will rely only on procps_stat_reap() results.
Signed-off-by: Jim Warner <james.warner@comcast.net>
This just brings the newlib branch NEWS into line with
the current version from our master branch since those
changes have already been incorporated in this branch.
Signed-off-by: Jim Warner <james.warner@comcast.net>
Still unhappy with a minor memory leak associated with
libnuma, I experimented with omitting the dlclose that
was issued at module's end. For some reason which will
remain a mystery, the valgrind leak then went bye-bye.
So this patch just omits one use of dlclose and relies
on whatever kernel magic is at work to free the memory
when each process ends. We kept, however, the original
code (now commented-out) to serve as a future caution.
There remains one potential (but unlikely) dlclose use
near the original dlopen. But there will be no leak as
that 'numa_node_of_cpu' will not yet have been called.
This seems to be the culprit that triggers such leaks.
None of this libnuma shit would likely have come close
to hitting our fan had the numa developers provided us
with 'new' and 'unref' functions like our newlib does.
Signed-off-by: Jim Warner <james.warner@comcast.net>
Add a warning if you specify a command over 15 characters and don't
use the -f command.
This is a pick of two patches from master:
24fd260 pgrep: Fix off by one error in line check
4a7f9fc pgrep - adds warning that pattern exceeds 15 chars without
References:
!25
kill -l SIGHUP (or any other signal-name prefixed with "SIG")
would cause free() to be called with a bad pointer instead of
a pointer to what was allocated. Fix this and add test-case.
Ever since its introduction, the 'x' (extended format)
option has employed strncmp to parse those smaps keys.
Such an approach worked well as long as those prefixes
were guaranteed to be unique. But, with the 4.3 kernel
a new 'SwapPss' field was added to those within smaps.
That triggered a 2nd match for the 'Swap' logic which,
in turn, resulted in a duplicate output line of zeros.
So this patch just trades strncmp for strcmp, avoiding
potential future problems when /proc/$$/smaps evolves.
Reference(s):
. recent bug report
https://bugzilla.redhat.com/show_bug.cgi?id=1374061
. linux 4.3 kernel introduces SwapPss
commit 8334b96221ff0dcbde4873d31eb4d84774ed8ed4
. original pmap -x option introduction
commit 380cc1e908
Signed-off-by: Jim Warner <james.warner@comcast.net>
Recent profiling and timings have resulted in improved
newlib performance. This patch completes that process.
It just normalizes naming conventions employed for all
allocations involving reaped stacks & history support.
The modules offering a 'reap' function will also offer
the now standardized corresponding STACKS_INCR define.
The modules which provide dynamic history support will
now have a separate #define called NEWOLD_INCR used in
allocations/reallocations. And, while values currently
are set equal to that STACKS_INCR value, in the future
some reason for divorcing those two may be discovered.
----------------------------- for future reference ---
In those modules which contain the STACKS_INCR #define
it is tempting to specify a large value so as to avoid
repeated calls to malloc/realloc. However, in doing so
an extra runtime price will be paid in 'cleanup_stack'
calls with any iterative programs like top or slabtop.
So, with the current values a balance has been sought.
Signed-off-by: Jim Warner <james.warner@comcast.net>
This commit just brings some comments plus identifiers
into agreement with the current newlib implementation.
Signed-off-by: Jim Warner <james.warner@comcast.net>
FreeBSD doesn't have SIGPWR so makes no sense in warning and assuming
its 29.
Picked from master, but the file has moved from proc/sig.c to
library/signals.c
References:
commit 8abd0c92abhttps://bugs.debian/org/832148
Signed-off-by: Craig Small <csmall@enc.com.au>
After discovering those terrible costs associated with
/proc/status vs. /proc/stat, our library changed so as
to favor the latter if a field could be met by either.
Well, low-and-behold, this top program had chosen some
item enumerators that needlessly caused 'status' to be
accessed when 'statm' could be used instead. And while
top's needs require conversion from pages to KiB, that
is still far less costly than that damn 'status' file.
[ this was found when comparing newlib top against a ]
[ 3.2.8 ancient top since the current master top was ]
[ suffering from the exact same 'status' deficiency. ]
[ and, no way was top-3.2.8 going to beat newlib top ]
[ by 50% - we'll allow only a 1-10% occasional loss! ]
Signed-off-by: Jim Warner <james.warner@comcast.net>
Profiling revealed a large amount of time spent in the
'escape_str_utf8' function (escape.c) with both of our
NLS branches (newlib and master). That same result was
not seen under an ancient top-3.2.8 program & library.
Well, the 3.2.8 result was ultimately explained by the
absence of a 'setlocale', necessary under NLS support.
Thus, when that ancient library tested for locale, all
it got was 'ANSI_...' & assumed 'UTF-8' wasn't active.
But after a hack to that ancient code to place it on a
par with newlib/master, I still found cost differences
that led me to revisit an old change referenced below.
It turns out that 'iswprint' costs far more than would
a call of 'isprint', even with the extra support code.
So this commit just reverts that five year old change.
Reference(s):
commit 7b0fc19e9d
Signed-off-by: Jim Warner <james.warner@comcast.net>
This patch contains the following miscellaneous stuff:
. The pids_stacks_fetch() routine might call for newly
allocated stacks to be itemized. However, that job was
already tended to by the pids_stacks_alloc() function.
So, this patch just eliminates a redundant invocation.
------------------------------------------------------
. The concept of 'dirty_stacks' has not kept pace with
the evolving stacks implementation. Originally, stacks
were considered dirty only if free() of dynamic memory
was needed before refreshing any single result struct.
Later, with the introduction of the 'extra' item and a
promise to reset it to zero, 'dirty' was much broader.
So, this patch just treats the dirty flg as others do.
------------------------------------------------------
. Lastly, a word or three about performance & timings.
Tuning efforts concentrated on the <pids> API and top.
And unless an oldlib equivalent to the preceding patch
is applied (favoring stat vs. status), newlib top will
often outperform the oldlib version (obviously wrong).
So assuming /proc/stat is preferred in both libraries,
generally speaking, a cpu and elapsed time increase of
1-5% was found for this new stacks oriented interface.
Of course, there's also an increased memory footprint.
There are some occasions, however, when the newlib top
is at a substantial disadvantage. For example if WCHAN
or TTY is displayed, such items will be present in all
newlib reaped stacks (i.e. every process). But old top
would only incur such overhead with displayable tasks.
Thus, oldlib top could outperform newlib by up to 25%,
for example, if only fields requiring NO library flags
were displayed. However, such a scenario is not likely
since only GID, UID, PID, TGID & WCHAN would be shown.
In the usual case, that overhead associated with WCHAN
and/or TTY is overshadowed by other top runtime costs.
All in all a pleasing outcome I deem quite acceptable.
------------------------------------------------------
Signed-off-by: Jim Warner <james.warner@comcast.net>
Long ago, in a galaxy far away (when top was in charge
of library FILL flgs) /proc/status was to be preferred
over /proc/stat if a field could be satisfied by both.
This was done to avoid costly 64-bit math emulation in
a 32-bit application due to 'unsigned long long' data.
Well it's time to acknowledge the prevalence of 64-bit
platforms. And in such an environment the cost picture
has shifted significantly. It now costs 14 times (wow)
as much to access /proc/status compared to /proc/stat.
In other words, even with '%llu' variables, a sscanf()
call in stat2proc() beats the pants off that home brew
gperf based hashing employed by the status2proc() guy.
In fact, status2proc incurs higher costs than found in
the most expensive aspect of top's forest view option.
Here's a gprof extract to illustrate the costs. It was
produced with an rcfile requiring fields from both the
/proc/stat & /proc/status pseudo files (among others).
There were 5000 iterations in each of 4 separate gprof
runs subsequently merged into 1 gmon.sum for analysis.
% self self
time seconds calls us/call name
----- ------- ------- ------- -----------
28.65 4.10 4689423 0.87 status2proc
26.14 3.74 40000 93.50 forest_adds
...
01.96 0.28 4689427 0.06 stat2proc
[ since forest_adds is recursive, the calls value is ]
[ the non-recursive #, its 'call graph' shows totals ]
Anyway, now that such cost is known this patch becomes
what is euphemistically known as the usual no-brainer.
[ jeeze, was it really this long between profilings? ]
Signed-off-by: Jim Warner <james.warner@comcast.net>
After doing some profiling and timings, then comparing
newlib top to the existing 3.3.12 version, I concluded
top should avoid stack results unless actually needed.
Not only will stack depth be kept to minimums, but the
new library can save some otherwise wasted pathlength.
Signed-off-by: Jim Warner <james.warner@comcast.net>
